Conversation
…econnect to logger
Greptile SummaryThis PR introduces Key findings:
Confidence Score: 3/5
Important Files Changed
Sequence DiagramsequenceDiagram
participant CLI as dimos CLI / DIO TUI
participant LB as launch_blueprint()
participant FS as Filesystem (~/.dimos/instances/)
participant SUB as Subprocess (daemon.py __main__)
participant REG as instance_registry
participant COORD as ModuleCoordinator
CLI->>LB: launch_blueprint(robot_types, config_overrides)
LB->>FS: make_run_dir(instance_name)
LB->>FS: write launch_params.json + config.json
LB->>SUB: subprocess.Popen([python, -m, dimos.core.daemon, run_dir], start_new_session=True)
LB-->>CLI: LaunchResult(instance_name, run_dir)
Note over SUB: Double-fork → daemon grandchild
SUB->>FS: redirect stdin→/dev/null, stdout/stderr→stdout.log
SUB->>SUB: OutputTee.start() (pipes fd1+2 → stdout.log + stdout.plain.log)
SUB->>SUB: global_config.update(**config_overrides)
SUB->>COORD: autoconnect + blueprint.build()
COORD-->>SUB: coordinator ready
SUB->>SUB: coordinator.health_check()
SUB->>REG: register(InstanceInfo) → writes current.json
SUB->>SUB: install_signal_handlers()
SUB->>COORD: coordinator.loop() [blocks forever]
Note over CLI: DIO StatusSubApp tails stdout.log in real-time
CLI->>REG: list_running() / get(name) [polls current.json]
REG-->>CLI: InstanceInfo(pid, run_dir, ...)
CLI->>SUB: SIGTERM (stop / force-kill)
SUB->>COORD: coordinator.stop()
SUB->>REG: unregister(name) → deletes current.json
|
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
dimos/utils/cli/theme.py
Outdated
| _self.BACKGROUND = v["dio-bg"] | ||
| _self.BG = v["dio-bg"] | ||
| _self.FOREGROUND = v["dio-fg"] | ||
| _self.ACCENT = v["dio-text"] |
There was a problem hiding this comment.
ACCENT mapped to wrong theme variable
ACCENT is being assigned the dio-text value instead of dio-accent. Immediately below it, both CYAN and BORDER are correctly assigned v["dio-accent"], which confirms this is a copy-paste error.
All UI elements that use theme.ACCENT — including the "thinking…" indicator in the chat tab, syntax highlighting in the human CLI, gradient stops in animations, and more — will render in the plain text color instead of the intended accent color.
| _self.ACCENT = v["dio-text"] | |
| _self.ACCENT = v["dio-accent"] |
| self._log.lines = [l for l in self._log.lines if id(l) not in ids] | ||
| self._strips = [] | ||
| self._log._line_cache.clear() | ||
| self._log.virtual_size = Size(self._log.virtual_size.width, len(self._log.lines)) |
There was a problem hiding this comment.
Direct manipulation of private Textual internals
self._log.lines, self._log._line_cache, and self._log.virtual_size are all private / internal attributes of Textual's RichLog widget. There is no public API contract around them, so any Textual patch release could rename, restructure, or remove them and silently break the "thinking…" indicator at runtime.
If Textual does not expose a public way to remove specific rendered lines, consider a simpler approach: instead of surgically deleting the thinking strip, clear the entire log and re-write the message history when hide() is called (maintaining a _messages list). That removes the dependency on internals entirely.
| info = get(instance_name) | ||
| if info is not None: | ||
| return info.pid | ||
| return None | ||
|
|
||
| def _forward_sigint(signum: int, frame: object) -> None: |
There was a problem hiding this comment.
tee not closed before os._exit(1) on health-check failure
When health check fails, os._exit(1) is called without closing tee. The internal pipe's write-end (fd 1 and 2) remains open, and the reader thread in OutputTee may not flush the last bytes to stdout.log before the process terminates.
Since sys.stderr was opened in line-buffered mode (buffering=1) and the error message ends with \n, the data is flushed in practice. But making the cleanup explicit is safer and mirrors the pattern used in the except block below:
| info = get(instance_name) | |
| if info is not None: | |
| return info.pid | |
| return None | |
| def _forward_sigint(signum: int, frame: object) -> None: | |
| if not coordinator.health_check(): | |
| sys.stderr.write("Error: health check failed — a worker process died.\n") | |
| sys.stderr.flush() | |
| coordinator.stop() | |
| tee.close() | |
| os._exit(1) |
# Conflicts: # dimos/core/daemon.py # dimos/core/test_daemon.py
- Remove section markers (test_no_sections) - Remove __init__.py files, move sub_apps registry to registry.py (test_no_init_files) - Fix test_cli_stop_status to use instance_registry instead of run_registry - Fix test_system_configurator to mock correct confirm function path - Fix onnxruntime type: ignore comments (import-not-found) - Fix theme.py mypy error (list[object] annotation) - Add .venv to .gitignore (symlink in worktrees) Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
1 participant
Draft as a reminder for me later, don't review
Problem
Closes DIM-XXX
Solution
Breaking Changes
How to Test
Contributor License Agreement