Skip to content

fix(tui): emit subagent completion before terminal update#2120

Open
cyq1017 wants to merge 1 commit into
Hmbown:mainfrom
cyq1017:codex/fix-1961-subagent-completion-order
Open

fix(tui): emit subagent completion before terminal update#2120
cyq1017 wants to merge 1 commit into
Hmbown:mainfrom
cyq1017:codex/fix-1961-subagent-completion-order

Conversation

@cyq1017
Copy link
Copy Markdown
Contributor

@cyq1017 cyq1017 commented May 25, 2026

Closes #1961

Summary

Fixes a race where <codewhale:subagent.done> completion events could be emitted after the parent turn had already proceeded to terminal update/summarization, making the assistant appear to remain active after it should have finished.

This change reorders emission so parent sub-agent completion is sent before the terminal update path executes, so completion is recorded in transcript in the expected sequence.

Changed files

  • crates/tui/src/tools/subagent/mod.rs
  • crates/tui/src/tools/subagent/tests.rs

Verification

  • cargo fmt --all -- --check
  • cargo test -p deepseek-tui run_subagent_task_emits_parent_completion_before_terminal_update
  • cargo test -p deepseek-tui emit_parent_completion_fires_for_direct_child -- --nocapture
  • cargo test -p deepseek-tui emit_parent_completion_skips_grandchildren -- --nocapture
  • cargo test -p deepseek-tui turn_holds_open_for_running_or_completed_subagents -- --nocapture
  • git diff --check

Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request introduces support for a base_url configuration setting, allowing users to specify custom DeepSeek-compatible endpoints through the TUI and CLI. It also addresses a race condition in subagent task completion (Issue #1961) by reordering logic to emit parent completions before updating the manager's terminal state. Review feedback indicates that persist_status_items and network policy commands are currently hardcoded to use the default configuration path, suggesting they should be updated to respect custom configuration paths provided via the application state.

Comment thread crates/tui/src/commands/config.rs Outdated
use std::fs;

let path = config_toml_path()?;
let path = config_toml_path(None)?;
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

medium

persist_status_items is hardcoded to use the default configuration path (None). This ignores any custom configuration path provided via the --config CLI flag or the DEEPSEEK_CONFIG_PATH environment variable. This function should be updated to accept an optional path parameter, and its callers (such as in config_ui.rs) should pass app.config_path.as_deref() to ensure consistency.

Comment thread crates/tui/src/commands/network.rs Outdated

fn list_policy() -> anyhow::Result<String> {
let path = super::config::config_toml_path()?;
let path = super::config::config_toml_path(None)?;
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

medium

The network policy commands are hardcoded to use the default configuration path by passing None to config_toml_path. This causes the application to read from and write to the default ~/.deepseek/config.toml even if a custom configuration file was specified at startup. These functions should be updated to respect the active configuration path from the App state.

@cyq1017 cyq1017 force-pushed the codex/fix-1961-subagent-completion-order branch from e75c1b9 to d4e75b3 Compare May 25, 2026 10:48
@cyq1017 cyq1017 force-pushed the codex/fix-1961-subagent-completion-order branch from d4e75b3 to 134d89f Compare May 25, 2026 11:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

fix: delayed child-agent/internal events after final summary

1 participant