Skip to content

fix TUI tools on MacOS/Linux when network not configured#1512

Merged
jeff-hykin merged 1 commit intodevfrom
jeff/fix/interactive
Mar 10, 2026
Merged

fix TUI tools on MacOS/Linux when network not configured#1512
jeff-hykin merged 1 commit intodevfrom
jeff/fix/interactive

Conversation

@jeff-hykin
Copy link
Member

Problem

Stuff like lcmspy doesn't work when the system isn't configured (Linux and MacOS)

Why?

  • TUI takes over STDIN
  • then configure_system runs, and tries to ask for yes/no input
  • deadlock because user can't confirm yes/no

Solution

  • patch so that system conf runs before starting the TUI system

Breaking Changes

None

How to Test

de-configure your multicast or network settings (maybe reboot) then run lcmspy

Contributor License Agreement

  • I have read and approved the CLA.

@greptile-apps
Copy link
Contributor

greptile-apps bot commented Mar 10, 2026

Greptile Summary

This PR fixes a deadlock in TUI tools (lcmspy, dtop, agentspy) on Linux and macOS when the system isn't configured. The root cause was that Textual would take over stdin before configure_system had a chance to prompt the user for yes/no input, causing a deadlock. The fix eagerly calls autoconf() at the main() entry point (before app.run()), then passes autoconf=False into each TUI class so the inner LCMService.start() only performs a non-interactive check_only=True pass inside the TUI.

  • The fix is logically correct and is applied consistently across all three affected entry points.
  • autoconf=False is properly threaded through AgentSpyAppAgentMessageMonitorPickleLCM, ResourceSpyAppPickleLCM, and LCMSpyAppGraphLCMSpyLCMService.
  • The web mode branches in agentspy and lcmspy are unaffected: the Server re-executes the file without the "web" argument, so the else branch (with the new autoconf() call) is hit.
  • One minor concern: the new top-level autoconf() calls in all three main() functions have no try/except, whereas the existing call in LCMService.start() does. An exception from configure_system will now crash the tool before it even starts, rather than printing an error and continuing.

Confidence Score: 4/5

  • Safe to merge; the fix correctly resolves the deadlock without introducing new logic errors.
  • The change is minimal and well-targeted. The core fix — hoisting autoconf() before app.run() and disabling it inside the TUI — is correct and consistent across all three entry points. The only concern is missing error handling around the new top-level autoconf() calls, which is a regression from the existing try/except in LCMService.start(). This doesn't affect correctness on a properly functioning system but could surface noisily if configure_system throws.
  • No files require special attention; the error-handling concern applies equally to all three changed files.

Important Files Changed

Filename Overview
dimos/utils/cli/agentspy/agentspy.py Adds autoconf parameter to AgentMessageMonitor and AgentSpyApp, and calls autoconf() before app.run() in main(). Logic is correct; autoconf=False ensures no interactive prompt fires inside Textual. Minor: autoconf() lacks a try/except (see dtop.py comment).
dimos/utils/cli/dtop.py Adds autoconf parameter to ResourceSpyApp.__init__ and calls autoconf() before ResourceSpyApp.run() in main(). Correct fix; PickleLCM(autoconf=False) means start() only calls autoconf(check_only=True) inside the TUI. Missing try/except around top-level autoconf() call.
dimos/utils/cli/lcmspy/run_lcmspy.py Adds autoconf parameter to LCMSpyApp.__init__ and calls autoconf() before LCMSpyApp.run() in main(). Consistent with the other two files. Same minor concern with missing error handling on the top-level autoconf() call.

Sequence Diagram

sequenceDiagram
    participant User
    participant main()
    participant autoconf
    participant TUI App
    participant LCMService

    Note over User, LCMService: Fixed flow (this PR)
    User->>main(): run lcmspy / dtop / agentspy
    main()->>autoconf: autoconf() [full configure, may prompt user]
    autoconf-->>User: interactive prompt (typer.confirm) if needed
    User-->>autoconf: yes/no response
    autoconf-->>main(): configured
    main()->>TUI App: App(autoconf=False).run()
    Note over TUI App: Textual now owns stdin
    TUI App->>LCMService: start() [autoconf=False → check_only=True]
    LCMService->>autoconf: autoconf(check_only=True) [no prompt]
    autoconf-->>LCMService: ok
    LCMService-->>TUI App: LCM loop running
Loading

Last reviewed commit: 20a944f

@jeff-hykin jeff-hykin force-pushed the jeff/fix/interactive branch from 20a944f to fca37ff Compare March 10, 2026 16:47
# start LCM before .run() takes over the terminal (raw mode),
# because autoconf uses typer.confirm() which deadlocks inside a TUI.
self.spy = GraphLCMSpy(autoconf=True, graph_log_window=0.5)
self.spy.start()
Copy link
Member Author

@jeff-hykin jeff-hykin Mar 10, 2026

Choose a reason for hiding this comment

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

yeah calling start inside of init is cringe or whatever, but all other solutions are even more messy. We need LCM to start and ask the user about config stuff before the textual app starts, otherwise were going to get a deadlock

Copy link
Contributor

Choose a reason for hiding this comment

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

I checked on an Ubuntu VM, and indeed lcmspy is broken without configuring on Linux as well.

But the fix could be a lot simpler. I think you just need to call autoconf. We're doing at the top of dimos.py. We should probably do it in every other command entrypoint.

Image

Copy link
Member Author

@jeff-hykin jeff-hykin Mar 10, 2026

Choose a reason for hiding this comment

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

I think double-calling autoconf (once on the outside again on the inside) is more of a hack. Claude started with that and created a flag for turning autoconf off on the inside, but IMO thats even more of a mess.

After looking at it more, I think the better fix is for autoconf to be called in __init__ instead of in start. I'll make a PR for that

@jeff-hykin jeff-hykin changed the title fix TUI tools on MacOS fix TUI tools on MacOS/Linux Mar 10, 2026
@jeff-hykin jeff-hykin changed the title fix TUI tools on MacOS/Linux fix TUI tools on MacOS/Linux when network not configured Mar 10, 2026
@jeff-hykin jeff-hykin merged commit 0537fcb into dev Mar 10, 2026
12 checks passed
jeff-hykin added a commit that referenced this pull request Mar 10, 2026
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.

3 participants