Conversation
Greptile SummaryThis PR adds a Key changes:
Issues found:
Confidence Score: 3/5
Important Files Changed
Sequence DiagramsequenceDiagram
participant User
participant CLI as dimos graph (dimos.py)
participant GraphMain as graph.main()
participant BuildHTML as _build_html()
participant Loader as importlib
participant ToSVG as to_svg()
participant DotRender as dot.render_svg()
participant Graphviz as graphviz (dot)
participant HTTPServer
participant Browser
User->>CLI: dimos graph <file.py> [--no-disconnected] [--port N]
CLI->>GraphMain: main(python_file, show_disconnected, port)
GraphMain->>BuildHTML: _build_html(python_file, show_disconnected)
BuildHTML->>Loader: exec_module(file.py)
Loader-->>BuildHTML: module with Blueprint globals
loop For each Blueprint
BuildHTML->>ToSVG: to_svg(bp, tmp_svg, show_disconnected)
ToSVG->>DotRender: render_svg(bp, path, show_disconnected)
DotRender->>Graphviz: dot -Tsvg (subprocess)
Graphviz-->>DotRender: SVG bytes
DotRender-->>ToSVG: writes SVG file
ToSVG-->>BuildHTML: done
BuildHTML->>BuildHTML: read SVG, unlink tmp file
end
BuildHTML-->>GraphMain: HTML string
GraphMain->>HTTPServer: HTTPServer("0.0.0.0", port)
GraphMain->>Browser: webbrowser.open(url)
Browser->>HTTPServer: GET /
HTTPServer-->>Browser: text/html response
GraphMain->>GraphMain: server.handle_request() returns
GraphMain-->>User: "Served. Exiting."
Last reviewed commit: d31d6a8 |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| def log_message(self, format: str, *args: object) -> None: | ||
| pass | ||
|
|
||
| server = HTTPServer(("0.0.0.0", port), Handler) |
There was a problem hiding this comment.
HTTP server exposed on all network interfaces
The server binds to "0.0.0.0", which makes it reachable from any machine on the same network, not just the local developer's browser. This exposes the rendered blueprint diagrams (which may include internal architecture details) to anyone on the LAN.
Change the bind address to "127.0.0.1" (loopback only):
| server = HTTPServer(("0.0.0.0", port), Handler) | |
| server = HTTPServer(("127.0.0.1", port), Handler) |
| fd, svg_path = tempfile.mkstemp(suffix=".svg", prefix=f"dimos_{name}_") | ||
| os.close(fd) | ||
| to_svg(bp, svg_path, show_disconnected=show_disconnected) | ||
| with open(svg_path) as f: | ||
| svg_content = f.read() | ||
| os.unlink(svg_path) |
There was a problem hiding this comment.
Temp file leaked if to_svg raises an exception
If to_svg(...) raises (e.g. graphviz fails, a bad blueprint, etc.) the temporary file created by tempfile.mkstemp is never deleted — the os.unlink on line 67 is only reached on the happy path. Wrap this in a try/finally:
fd, svg_path = tempfile.mkstemp(suffix=".svg", prefix=f"dimos_{name}_")
os.close(fd)
try:
to_svg(bp, svg_path, show_disconnected=show_disconnected)
with open(svg_path) as f:
svg_content = f.read()
finally:
os.unlink(svg_path)
sections.append(f'<h2>{name}</h2>\n<div class="diagram">{svg_content}</div>')| with open(svg_path) as f: | ||
| svg_content = f.read() | ||
| os.unlink(svg_path) | ||
| sections.append(f'<h2>{name}</h2>\n<div class="diagram">{svg_content}</div>') |
There was a problem hiding this comment.
Blueprint name is embedded into HTML without escaping
name comes from Python module attribute names (so angle-bracket injection isn't possible today), but embedding it raw is a fragile pattern. If any future code path produces a name string with HTML special characters, this will produce broken or malicious HTML. Consider using html.escape(name) here:
| sections.append(f'<h2>{name}</h2>\n<div class="diagram">{svg_content}</div>') | |
| sections.append(f'<h2>{__import__("html").escape(name)}</h2>\n<div class="diagram">{svg_content}</div>') |
Or import html at the top of the file and use html.escape(name).
|
|
||
| def test_no_blueprints(tmp_path: object) -> None: | ||
| import pathlib | ||
|
|
There was a problem hiding this comment.
Incorrect type annotation for pytest tmp_path fixture
The pytest tmp_path fixture injects a pathlib.Path, but the parameter is typed as object. This makes the pathlib.Path(str(tmp_path)) on the next line an unnecessary round-trip. The same pattern appears at line 37.
| def test_no_blueprints(tmp_path: "pathlib.Path") -> None: |
Or import pathlib at the top and use pathlib.Path directly, eliminating the Path(str(tmp_path)) workaround.
…nnected stream indicators Replace graphviz SVG rendering with client-side Mermaid diagrams for blueprint visualization. Adds pan/zoom, two color themes (vivid and tailwind), colored edge labels, and dashed borders on dangling streams.
1 participant
DRAFT: just making this so I don't forget about it later
Problem
Closes DIM-XXX
Solution
Breaking Changes
How to Test
Contributor License Agreement