Commit graph

5 commits

Author SHA1 Message Date
Bryan Helmkamp
32015b2226
fix(graph): support dotted Fabro graph attributes (#324)
## Summary

Graph rendering now accepts documented Fabro dotted DOT attributes end
to end while keeping raw Graphviz calls behind `fabro-graphviz`. The new
`RenderableDot` boundary applies Fabro render styling and normalization
before raw SVG rendering, and both the CLI subprocess and server path
now route through that typed boundary instead of calling `graphviz_sys`
directly outside the graphviz crate.

The branch also adds a small curated DOT compatibility corpus covering
ACP agent attributes, human default choices, and subworkflow manager
attributes. Those fixtures are exercised by both render and validation
tests, and the run overview now shows graph render errors directly
instead of falling through to the empty graph state.

## Verification

- `cargo nextest run -p fabro-graphviz`
- `cargo nextest run -p fabro-validate`
- `cargo nextest run -p fabro-cli render_graph`
- `cargo nextest run -p fabro-server
render_graph_from_manifest_accepts_fabro_dotted_attributes`
- `cargo +nightly-2026-04-14 fmt --check --all`
- `cargo +nightly-2026-04-14 clippy -p fabro-graphviz -p fabro-cli -p
fabro-server -p fabro-validate --all-targets -- -D warnings`
- `rg -n "graphviz_sys" lib/crates/fabro-cli lib/crates/fabro-server`
- `cd apps/fabro-web && bun test`
- `cd apps/fabro-web && bun run typecheck`

---

[![Compound
Engineering](https://img.shields.io/badge/Compound_Engineering-6366f1)](https://github.com/EveryInc/compound-engineering-plugin)
Generated with GPT-5 via [Codex](https://openai.com/codex)

---------

Co-authored-by: Jess Martin <27258+jessmartin@users.noreply.github.com>
2026-05-20 09:31:08 -04:00
Bryan Helmkamp
95b101a26f
lint(clippy): disallow blocking std::io and std::net on Tokio paths
Extends the workspace clippy.toml — which already bans std:🧵:sleep,
std:🧵:spawn, and std::process::Command::new on Tokio paths — with:

- disallowed-types: std::io::{Read, Write, BufRead, BufReader, BufWriter}
  and std::net::{TcpStream, TcpListener, UdpSocket}
- disallowed-methods: std::io::{stdin, stdout, stderr}

Non-blocking std::io items (Error, ErrorKind, Result, IsTerminal, Cursor)
remain allowed. std::fs is intentionally deferred.

Annotates ~24 pre-existing sync call sites with #[expect(..., reason = "...")]
matching the established pattern. All annotations describe why blocking I/O
is intentional in that context (sync CLI command, test helper, pre-fork
flush, etc.), so a future conversion to async will surface as an unfulfilled
lint expectation instead of silently drifting.

Fixes one real Tokio-path issue surfaced by the new lint:
fabro-cli's server-start daemon-health poller (try_connect) was a sync fn
called from async execute_daemon; std::net::TcpStream::connect_timeout
blocked a Tokio worker for up to 100ms per poll iteration. Converted to
tokio::net::{TcpStream, UnixStream} with tokio::time::timeout.

One follow-up flagged in-code: fabro-agent/src/cli.rs's JSON event writer
uses std::io::stdout() inside tokio::spawn. Annotated with a FOLLOW-UP
reason pointing at tokio::io::stdout; left unchanged since volume is low
and scope exceeded this pass.

Verified: clippy clean, cargo +nightly fmt --check clean, full nextest
workspace run (4131 passed, 182 skipped).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
2026-04-19 16:06:02 -04:00
Bryan Helmkamp
cf80fe567a
test(harness): scrub ambient creds from spawned fabro CLI
CLI integration tests spawned the real fabro binary while letting the
parent process's env pass through. The pr_view "no credentials" snapshot
failed in CI because the Nightly workflow's minted GITHUB_TOKEN was
inherited by the child and turned the expected "credentials required"
error into a real GitHub API call (404 / 401). On developer laptops the
same leak occurs whenever gh auth login is active.

Introduce apply_test_isolation(cmd, home) in fabro-test: env_clear() +
re-populate PATH, HOME, NO_COLOR, and the FABRO_* test overrides. Route
TestContext::command(), the internal server bootstrap, and the four
ad-hoc spawners in tests/it/cmd/{attach,render_graph,runner,server_start}
through the same helper so the isolation is systemic instead of
per-callsite. Tests that deliberately need a credential (OPENAI_API_KEY,
GITHUB_APP_PRIVATE_KEY, etc.) continue to set it explicitly on the
returned Command; those survive the clear.

Add a regression test that sets sentinel GITHUB_TOKEN and
ANTHROPIC_API_KEY in the parent, spawns /usr/bin/env through the helper,
and asserts the child sees neither credential while still seeing PATH
and the harness's FABRO_NO_UPGRADE_CHECK override.

Verified: the full workspace (4022 tests) passes with GITHUB_TOKEN and
ANTHROPIC_API_KEY set in the parent, which previously broke the
pr_view_reads_pull_request_from_store_without_pull_request_json
snapshot. cargo fmt and nightly clippy are clean.
2026-04-18 01:55:01 -04:00
Bryan Helmkamp
1ab1327f96
fix(ci): stabilize graphviz Rust checks
Some checks are pending
Rust / Format (push) Waiting to run
Rust / Clippy (push) Waiting to run
Rust / Test (Linux) (push) Waiting to run
Rust / Test (macOS) (push) Waiting to run
TypeScript / Typecheck (push) Waiting to run
TypeScript / Test (push) Waiting to run
TypeScript / Build (push) Waiting to run
Keep the render-graph CLI integration test explicitly documented for
synchronous stdio subprocess usage, and make the garbage-stdout server
test drain stdin before returning invalid output so the Linux test path
stays deterministic.
2026-04-14 19:31:01 -04:00
Bryan Helmkamp
286eee7efa fix(graphviz): isolate rendering in a subprocess
Run Graphviz through an internal fabro subprocess so renderer failures no
longer share process fate with the server. Keep expected DOT parse failures
on the 400 path via an explicit stdout protocol, and treat child crashes or
protocol violations as 500s.
2026-04-14 18:15:23 -04:00