Expose active goal in stream JSON

[qqqys]

Summary

• emit active goal updates as stream-json partial events when partial messages are enabled
• add the active goal stream event type to the headless message schema
• cover headless /goal status, active status, and clear output semantics

Validation

• cd packages/cli && npx vitest run src/nonInteractive/io/StreamJsonOutputAdapter.test.ts src/nonInteractiveCliCommands.test.ts src/ui/commands/goalCommand.test.ts src/ui/hooks/useGeminiStream.test.tsx
• npm run build && npm run typecheck

Notes

• Built from the latest upstream main after Support active goal stream events and non-interactive goals #4273 merged.
• The pre-existing vscode companion lint warning remains outside this change.

[github-actions]

📋 Review Summary

This PR adds support for emitting active goal updates as stream JSON events in headless/non-interactive mode, along with improved /goal command status reporting. The implementation is clean and well-tested, with good coverage of the new functionality. The changes are focused and follow existing patterns in the codebase.

🔍 General Feedback

• The implementation correctly follows the existing stream event pattern established for other event types (tool_progress, content deltas)
• Test coverage is comprehensive, including edge cases like null active goal and goal clearing
• The change properly separates concerns: type definitions, adapter logic, and command handling
• Good attention to maintaining backward compatibility (events only emitted when includePartialMessages is enabled)
• The goal command improvements provide consistent UX between interactive and non-interactive modes

🎯 Specific Feedback

🔴 Critical

No critical issues identified.

🟡 High

No high priority issues identified.

🟢 Medium

• File: packages/cli/src/nonInteractive/io/StreamJsonOutputAdapter.ts:133 - Consider adding a type guard or validation for event.value before emitting. While the test shows both object and null values work correctly, an explicit check would make the code more defensive:

  if (event.type === GeminiEventType.ActiveGoal) {
    // event.value is already typed as ActiveGoal | null by ServerGeminiStreamEvent
    this.emitStreamEventIfEnabled(
      {
        type: 'active_goal',
        active_goal: event.value,
      },
      null,
    );
    return;
  }

• File: packages/cli/src/nonInteractive/types.ts:250 - The ActiveGoalStreamEvent interface imports ActiveGoal type from core. Consider verifying this type is exported from @qwen-code/qwen-code-core's public API to avoid potential import issues in downstream consumers.

🔵 Low

• File: packages/cli/src/ui/commands/goalCommand.ts:161 - The early return for non-interactive mode after clearing could be consolidated with the existing return pattern. Currently line 161 returns early, but line 162 falls through. Consider making the pattern consistent:

  if (context.executionMode === 'non_interactive') {
    return infoMessage(`Goal cleared: ${cleared.condition}`);
  }
  // Interactive mode: just add to history without returning message

  This is already correct, but adding a comment explaining why interactive mode returns void (to maintain existing UI behavior) would help future maintainers.

• File: packages/cli/src/nonInteractiveCliCommands.test.ts:251 - The test "should report no active goal for empty non-interactive /goal" could benefit from also testing the interactive mode behavior for comparison, ensuring both modes handle the empty case appropriately.

• File: packages/cli/src/nonActiveCliCommands.test.ts:274 - Consider adding a test case where /goal is called after a goal has naturally completed (achieved/failed/aborted) to verify the getLastGoalTerminal fallback behavior is exercised in non-interactive mode.

✅ Highlights

• Test coverage excellence: The new test in StreamJsonOutputAdapter.test.ts thoroughly covers both setting and clearing active goals, including the null value case
• Type safety: The new ActiveGoalStreamEvent interface is well-defined and integrates cleanly with the existing StreamEvent union
• Consistent patterns: The processEvent override in StreamJsonOutputAdapter follows the exact same pattern as other stream event handlers
• UX improvement: The non-interactive /goal command now provides actionable feedback about goal status, matching the interactive mode experience
• Minimal diff: The changes are surgical - only touching what's needed without unnecessary refactoring

[pomelo-nwu]

Scope is tight (+149/-1, focused on protocol surface + a small UX fix on top of #4273). Reading the code statically — did not run the test suite locally.

What looks good

• Type definition is consistent. ActiveGoalStreamEvent { type: 'active_goal', active_goal: ActiveGoal | null } in types.ts mirrors the snake_case convention used by tool_progress / content_block_*, and ActiveGoal | null accurately reflects ServerGeminiActiveGoalEvent.value in core/turn.ts.
• processEvent override reuses the existing guard. Calling emitStreamEventIfEnabled automatically respects includePartialMessages, so there is no duplicated condition. Other events go through super.processEvent(event) unchanged.
• Nice UX side-fix in goalCommand.ts. Before this PR, non-interactive /goal clear returned void, which handleSlashCommand fell back to as "Command executed successfully." (because createNonInteractiveUI().addItem is a no-op). Returning an explicit Goal cleared: <condition> is a clear improvement.
• Tests cover the meaningful axes. The stream-json test asserts both the populated ActiveGoal and the null (cleared) emission; the CLI tests cover empty /goal, set-then-status, and clear, and properly isolate the module-scoped store via __resetActiveGoalStoreForTests.

Suggestions

1. Consider ACP mode in goalCommand.ts as well. The new branch is gated on executionMode === 'non_interactive', but ACP mode also goes through handleSlashCommand with the same no-op createNonInteractiveUI(), and Session.ts#processSlashCommandResult already handles messageType: 'info' by emitting an agent message chunk to Zed. So ACP users currently get the same "Command executed successfully." fallback. Loosening the check would unify the behavior:

   if (context.executionMode !== 'interactive') {
     return infoMessage(`Goal cleared: ${cleared.condition}`);
   }

2. Optional: add a negative test for the disabled path. The "with partial messages disabled" describe block in StreamJsonOutputAdapter.test.ts doesn't assert that processEvent({ type: ActiveGoal, ... }) is suppressed. emitStreamEventIfEnabled already guards on includePartialMessages, but a single assertion would lock the behavior against future regressions.

3. Minor architectural note (non-blocking). BaseJsonOutputAdapter exposes hooks like onTextBlockCreated / onToolUseBlockCreated and keeps state mutation in the base switch. Overriding the top-level processEvent here is fine because ActiveGoal doesn't touch message state — it's a pure side-channel event. If more side-channel events land later (e.g. hook status), it might be worth introducing an onSideChannelEvent hook in the base class for symmetry. Not needed for this PR.

Risks

• Protocol-level: purely additive change to the stream_event discriminated union. Consumers doing exhaustive type-narrowing will need to handle the new variant, but that is the intended forward-compatible shape.
• state.finalized guard: the base processEvent short-circuits when state.finalized is true. The override doesn't replicate that check, but per the emit sites in core/client.ts all ActiveGoal yields happen inside the turn before Finished, so this is safe today. Worth keeping in mind if the lifecycle ordering ever changes.

Overall looks good — I'd suggest folding in the ACP branch unification before merge, the rest is optional polish.

[wenshao]

No review findings. Downgraded from Approve to Comment: CI failing: Test (ubuntu-latest, Node 22.x). — gpt-5.5 via Qwen Code /review