Skip to content

fix(cli): restore terminal from cbreak mode after run exits #294

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
hertznsk wants to merge 1 commit into
base: main
Choose a base branch
from
Draft

fix(cli): restore terminal from cbreak mode after run exits #294

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 23 additions & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,29 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
failure is reported as a new `agent_compaction_skipped` event
(`reason: "estimate_unavailable"`) rather than vanishing into stderr. See
[Workflow Syntax → Context Compaction](docs/workflow-syntax.md#context-compaction).
- **Terminal left in cbreak mode (no echo/ICANON) after a run exited** — a
`KeyboardListener` started while the terminal was already in cbreak mode
(after an Esc pause/resume cycle, or a second listener in the same process)
captured that cbreak state as its "original" settings and restored it on
`stop()`, leaving the user's interactive shell without echo or canonical
mode. The tty baseline is now captured once per process, before the first
`tty.setcbreak()`, and reused by every later listener; `start()` is
idempotent while active so a duplicate call can't overwrite the baseline;
restore uses `TCSANOW` so a blocked output drain can't delay it; and the
saved baseline is cleared only after a successful `tcsetattr` so a
transient failure can be retried by `atexit`/`SIGTERM`/`stop()`. The
SIGTERM cleanup handler also no longer swallows the signal: after
restoring the terminal it delegates to the previously-installed
disposition (reset-and-re-raise for `SIG_DFL`, ignore for `SIG_IGN`,
invoke a callable previous handler), and re-registration captures the
previous disposition in the handler closure so the listener can no
longer recurse into itself. The run and resume commands now also reapply
and retire the baseline at their outermost cleanup boundary, after provider
shutdown and every other teardown step. This closes a later race where
normal completion or Ctrl+C could restore the terminal correctly and then
a provider's cleanup could put it back into cbreak; SIGTERM during that same
late-cleanup window restores the process baseline before terminating.
([#290](https://github.com/microsoft/conductor/issues/290))

## [0.1.36](https://github.com/microsoft/conductor/compare/v0.1.35...v0.1.36) - 2026-09-02

Expand Down
157 changes: 86 additions & 71 deletions src/conductor/cli/run.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2531,46 +2531,55 @@ async def run_workflow_async(
terminal_error_message = str(exc)
raise
finally:
# Write the terminal run record (MCP server plan E2) before
# removing the live one below, so a completed run remains
# resolvable by run_id after this process exits. Never raises --
# see the helper's own docstring.
_write_terminal_record_for_current_process(
event_log_subscriber=event_log_subscriber,
workflow_path=workflow_path,
started_at=started_at_iso,
status=terminal_status,
output=terminal_output,
error_type=terminal_error_type,
error_message=terminal_error_message,
engine=engine,
)
try:
# Write the terminal run record (MCP server plan E2) before
# removing the live one below, so a completed run remains
# resolvable by run_id after this process exits. Never raises --
# see the helper's own docstring.
_write_terminal_record_for_current_process(
event_log_subscriber=event_log_subscriber,
workflow_path=workflow_path,
started_at=started_at_iso,
status=terminal_status,
output=terminal_output,
error_type=terminal_error_type,
error_message=terminal_error_message,
engine=engine,
)

# Clean up the Fleet Manager run record on every exit path (E2 —
# normal completion, an explicit WorkflowTerminated re-raise, or an
# unexpected exception all funnel through this finally). Unlike the
# legacy PID file (removed only by a background child), this runs
# unconditionally: foreground and foreground-with-dashboard runs now
# write a record too and must remove it on exit just the same.
# Guarded (never raises) so a failure here cannot prevent the
# dashboard/event-log/file-logging cleanup below from running.
_remove_run_record_for_current_process_safe()

# Stop dashboard if it was started
if dashboard is not None:
await dashboard.stop()
# Clean up the Fleet Manager run record on every exit path (E2 —
# normal completion, an explicit WorkflowTerminated re-raise, or an
# unexpected exception all funnel through this finally). Unlike the
# legacy PID file (removed only by a background child), this runs
# unconditionally: foreground and foreground-with-dashboard runs now
# write a record too and must remove it on exit just the same.
# Guarded (never raises) so a failure here cannot prevent the
# dashboard/event-log/file-logging cleanup below from running.
_remove_run_record_for_current_process_safe()

# Stop dashboard if it was started
if dashboard is not None:
await dashboard.stop()

# Close JSONL event log and report path
if event_log_subscriber is not None:
event_log_subscriber.close()
_verbose_console.print(
styled("[dim]Event log written to: {}[/dim]", event_log_subscriber.path)
)
# Close JSONL event log and report path
if event_log_subscriber is not None:
event_log_subscriber.close()
_verbose_console.print(
styled("[dim]Event log written to: {}[/dim]", event_log_subscriber.path)
)

# Report log file path to stderr and close file logging
if log_file is not None and _file_console is not None:
_verbose_console.print(styled("[dim]Log written to: {}[/dim]", log_file))
close_file_logging()
finally:
# Provider shutdown occurs after the listener's inner ``finally``
# and may itself touch the controlling TTY. Reapply the process
# baseline at the outermost boundary so normal exit, Ctrl+C, and a
# later cleanup failure all return a sane terminal to the shell.
from conductor.interrupt.listener import restore_terminal_baseline

# Report log file path to stderr and close file logging
if log_file is not None and _file_console is not None:
_verbose_console.print(styled("[dim]Log written to: {}[/dim]", log_file))
close_file_logging()
restore_terminal_baseline(clear=True)


def format_routes(routes: list[dict[str, Any]]) -> Text:
Expand Down Expand Up @@ -3334,44 +3343,50 @@ async def resume_workflow_async(
terminal_error_message = str(exc)
raise
finally:
# Write the terminal run record (MCP server plan E2) before
# removing the live one below -- mirrors run_workflow_async. A
# resumed run reuses its predecessor's run_id, so this call
# replaces the earlier terminal record rather than duplicating it.
# Never raises -- see the helper's own docstring.
_write_terminal_record_for_current_process(
event_log_subscriber=event_log_subscriber,
workflow_path=resolved_workflow_path,
started_at=started_at_iso,
status=terminal_status,
output=terminal_output,
error_type=terminal_error_type,
error_message=terminal_error_message,
engine=engine,
)
try:
# Write the terminal run record (MCP server plan E2) before
# removing the live one below -- mirrors run_workflow_async. A
# resumed run reuses its predecessor's run_id, so this call
# replaces the earlier terminal record rather than duplicating it.
# Never raises -- see the helper's own docstring.
_write_terminal_record_for_current_process(
event_log_subscriber=event_log_subscriber,
workflow_path=resolved_workflow_path,
started_at=started_at_iso,
status=terminal_status,
output=terminal_output,
error_type=terminal_error_type,
error_message=terminal_error_message,
engine=engine,
)

# Clean up the Fleet Manager run record on every exit path (E2 —
# mirrors run_workflow_async so a resumed run's record is removed
# the same way a fresh run's is). Guarded (never raises) so a
# failure here cannot prevent the dashboard/event-log/file-logging
# cleanup below from running.
_remove_run_record_for_current_process_safe()
# Clean up the Fleet Manager run record on every exit path (E2 —
# mirrors run_workflow_async so a resumed run's record is removed
# the same way a fresh run's is). Guarded (never raises) so a
# failure here cannot prevent the dashboard/event-log/file-logging
# cleanup below from running.
_remove_run_record_for_current_process_safe()

# Stop dashboard if it was started
if dashboard is not None:
await dashboard.stop()
# Stop dashboard if it was started
if dashboard is not None:
await dashboard.stop()

# Close JSONL event log and report path
if event_log_subscriber is not None:
event_log_subscriber.close()
_verbose_console.print(
styled("[dim]Event log written to: {}[/dim]", event_log_subscriber.path)
)
# Close JSONL event log and report path
if event_log_subscriber is not None:
event_log_subscriber.close()
_verbose_console.print(
styled("[dim]Event log written to: {}[/dim]", event_log_subscriber.path)
)

# Report log file path to stderr and close file logging
if log_file is not None and _file_console is not None:
_verbose_console.print(styled("[dim]Log written to: {}[/dim]", log_file))
close_file_logging()
finally:
# Keep resume teardown parity with ``run_workflow_async``.
from conductor.interrupt.listener import restore_terminal_baseline

# Report log file path to stderr and close file logging
if log_file is not None and _file_console is not None:
_verbose_console.print(styled("[dim]Log written to: {}[/dim]", log_file))
close_file_logging()
restore_terminal_baseline(clear=True)


async def _prefetch_plugin_sources(config: Any, workflow_path: Path) -> dict[str, Any]:
Expand Down
Loading