Commit graph

29 commits

Author SHA1 Message Date
Bryan Helmkamp
80de5ca616 refactor(static): centralize env var names
Add fabro-static::EnvVars as the shared registry for fixed environment variable names and migrate env reads, clap env bindings, and subprocess/test allowlists to use it.

Add clippy bans for raw std::env lookup APIs so future dynamic env facades must be documented explicitly.
2026-04-24 12:29:51 -04:00
Bryan Helmkamp
4585f9874c
fix(test): wait for chrome process exit instead of polling for screenshot
The headless Chrome screenshot test polled for the screenshot file every
100 ms for 20 seconds, then panicked. Slow CI runners (Ubuntu 24.04 GHA)
sometimes took longer than the polling deadline, surfacing as a flake.

Chrome with `--screenshot` exits when the file is written, so waiting on
the process is the deterministic completion signal — no polling, no
arbitrary deadline that might be too short.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
2026-04-23 17:49:04 -04:00
Bryan Helmkamp
e6e091fe8e
refactor(server): lock down server secrets 2026-04-23 07:15:27 -04:00
Bryan Helmkamp
323c797e0f
refactor(auth): simplify CLI auth plumbing after code review
Consolidate three copies of `normalized_http_base_url` and
`build_public_http_client` into shared helpers in `user_config`,
add `Display for ServerTarget`, drop stale `#[allow(dead_code)]`
markers now that login/logout/JWT are wired, remove dead
`LOGIN_SUCCESSFUL` and `_error_description` field, and gate
test-only helpers behind `#[cfg(test)]`.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
2026-04-20 11:13:37 -04:00
Bryan Helmkamp
9c68c57bcc
feat(auth): add CLI GitHub login and logout flow
Add the server-side CLI OAuth endpoints and token persistence needed to
mint JWT access tokens and rotating refresh tokens from the existing
GitHub web auth flow.

Add CLI auth storage plus `fabro auth login`, `logout`, and `status`, and
prefer stored OAuth access tokens when building target clients.
2026-04-20 07:12:52 -04:00
Bryan Helmkamp
0b91c71514
test(unwrap): clean shared support and workflow fixtures 2026-04-19 21:01:25 -04:00
Bryan Helmkamp
72924ba611
Merge remote-tracking branch 'origin/main'
# Conflicts:
#	lib/crates/fabro-agent/src/cli.rs
#	lib/crates/fabro-cli/src/commands/run/run_progress/mod.rs
#	lib/crates/fabro-cli/tests/it/cmd/exec.rs
#	lib/crates/fabro-cli/tests/it/scenario/lifecycle.rs
#	lib/crates/fabro-telemetry/src/spawn.rs
#	lib/crates/fabro-workflow/tests/it/attractor_compat.rs
#	lib/crates/fabro-workflow/tests/it/cp_integration.rs
2026-04-19 20:32:02 -04:00
Bryan Helmkamp
ad0d532691
chore(clippy): require reasons on allow attributes
Enable clippy::allow_attributes_without_reason at the workspace level.
Add concise, callsite-specific reasons to existing allow attributes, including generated code paths.
2026-04-19 20:24:24 -04:00
Bryan Helmkamp
19939c5f07
lint(clippy): disallow blocking std::fs on Tokio paths
Phase 2/3 of the std::fs lint initiative (Phase 1 refactors landed in
commit 9d1c0d98c).

clippy.toml additions (appended to disallowed-methods):
  std::fs::read, read_to_string, write, read_dir, copy, canonicalize
  std::fs::File::open, File::create, File::create_new
  std::fs::OpenOptions::open

File::options was deliberately excluded — it returns an OpenOptions
builder with no syscall. OpenOptions::open is where the block happens.
Non-blocking std::fs items (metadata, exists, create_dir_all, remove_*,
rename, and all std::fs types) remain legal.

Annotation policy (per updated plan):
  - Mixed async/sync production source: function- or statement-scoped
    #[expect(...)] so future accidental Tokio-path regressions in the
    same file still fire.
  - Fully-sync production source, test modules, integration tests,
    build.rs: file-level #![expect(...)].
  - Every #[expect] has a specific reason identifying the sync context.

Annotations added in ~90 files across the workspace. Notable narrow
placements: fabro-server server.rs current_server_target,
build_disk_usage_response, create_test_app_state_with_session_key;
fabro-server install.rs read_to_string rollback snapshot;
fabro-sandbox local.rs list_recursive; fabro-agent cli.rs FOLLOW-UP on
the JSON-stdout writer; fabro-llm providers/common.rs FOLLOW-UP for
load_file_as_base64 (7 translator call sites; revisit if file:// URL
usage grows).

build.rs blanket allows: fabro-api/build.rs, fabro-util/build.rs.

Pre-existing unrelated nightly-clippy warnings fixed under scope:
fabro-sandbox sandbox_spec.rs (unused_imports, unused_async),
reconnect.rs (unused_variables, unused_async).

Verified: cargo +nightly-2026-04-14 clippy --workspace --all-targets
-- -D warnings passes; fmt clean; 4129/4131 tests pass (two known
flakes under parallel nextest load, both pass individually and are
unrelated to this change).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
2026-04-19 17:22:21 -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
1048534e2c
ci: switch clippy to pinned nightly, clean up workspace lints
- rust.yml: move clippy to nightly-2026-04-14 (was stable); also pin
  fmt to the same nightly date for consistency. Both jobs now use the
  dated nightly and the run-step uses `cargo +nightly-2026-04-14 ...`.
- AGENTS.md: update developer commands to match CI.
- Duration constructors: replace `Duration::from_secs(N * 60)` /
  `Duration::from_millis(N * 1000)` with `from_mins` / `from_secs` /
  `from_hours` across the workspace to satisfy clippy's new
  `duration_suboptimal_units` lint. std::time::Duration only — custom
  `settings::duration::Duration` sites kept on `from_secs`.
- map/unwrap_or cleanup: `.map(f).unwrap_or(v)` → `.map_or(v, f)`,
  `.map(f).unwrap_or(false)` on Result → `.is_ok_and(f)`, per
  `clippy::map_unwrap_or`.
- Misc lints: collapse nested `if` into match guard in
  handler/llm/api.rs and run_state.rs; replace `columns.len() > 0`
  with `!columns.is_empty()`; switch a pair of `sort_by` calls to
  `sort_by_key`.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
2026-04-16 18:59:11 -04:00
Bryan Helmkamp
ba67ad9cea Stop waiting for headless Chrome to exit 2026-04-12 15:12:36 -04:00
Bryan Helmkamp
e828ce35a5 Disable debug UI auto-refresh for screenshot test 2026-04-12 15:06:04 -04:00
Bryan Helmkamp
7771dc1e41 Fix headless Chrome debug UI CI test 2026-04-12 14:54:28 -04:00
Bryan Helmkamp
206cefcadc Merge remote-tracking branch 'origin/main'
# Conflicts:
#	clippy.toml
2026-04-12 13:43:38 -04:00
Bryan Helmkamp
4d925d5d5d refactor(async): lint std::process::Command across all targets
Move async subprocess paths to Tokio or spawn_blocking, document the
intentional synchronous std::process::Command callsites, and make CI run
Clippy with --all-targets so the guardrail applies to test code too.
2026-04-12 13:35:57 -04:00
Bryan Helmkamp
e8ad12fc30 fix: simplify fabro-http crate and fix correctness issues
- Replace unwrap_or_default() with expect() in hooks/llm HTTP client
  builders — Default silently discards all config (timeouts, TLS, proxy)
- Route fabro-mcp through fabro_http instead of raw reqwest, respecting
  FABRO_HTTP_PROXY_POLICY for MCP HTTP transport connections
- Deduplicate HttpClientBuilder / BlockingHttpClientBuilder via macro
- Extract helpers for repeated http_client error handling in diagnostics
  and web_auth
- Remove duplicate test_http_client() in fabro-cli and fabro-llm

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
2026-04-12 12:20:55 -04:00
Bryan Helmkamp
3b2cffceaf refactor(http): centralize reqwest behind fabro-http
Add the shared fabro-http transport crate and route hand-written HTTP client construction through it.

Use FABRO_HTTP_PROXY_POLICY for test no-proxy defaults, remove direct reqwest deps from ordinary crates, and add clippy bans for raw reqwest entrypoints.
2026-04-12 11:48:54 -04:00
Bryan Helmkamp
6a87f0a071 fmt: apply nightly rustfmt after merge
Restore a clean nightly rustfmt baseline on the merged main branch so
cargo +nightly fmt --check --all passes again after bringing in
origin/main.
2026-04-11 13:43:30 -04:00
Bryan Helmkamp
007cfed240 refactor: remove backwards-compat error type aliases
No production deployments exist, so there's no need for migration shims.
Remove all six backwards-compat type aliases (AgentError, SdkError,
CoreError, GraphvizError, StoreError, FabroError) and migrate ~880
callsites to use the canonical Error name directly within each crate,
or qualified imports (e.g., `use fabro_llm::Error as LlmError`) for
cross-crate references. Also fix a pre-existing absolute-path clippy
lint in fabro-server error.rs.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
2026-04-11 12:51:42 -04:00
Bryan Helmkamp
5eeacd7864 fmt 2026-04-11 11:27:46 -04:00
Bryan Helmkamp
a1e0762eb0 refactor(workspace): satisfy clippy all-targets warnings 2026-04-05 14:37:32 -04:00
Bryan Helmkamp
8a9d5096e5 test(twin-github): reuse checked-in RSA public key 2026-04-05 12:18:39 -04:00
Bryan Helmkamp
0d9ea168af test: standardize no-proxy localhost HTTP clients 2026-04-05 12:17:29 -04:00
Bryan Helmkamp
093f9c2983 test(twin-github): stop generating rsa keys during tests 2026-04-05 11:02:54 -04:00
Bryan Helmkamp
ddc57d458c Rename SessionConfig to SessionOptions and McpServerConfig to McpServerSettings
Aligns naming with the convention that "Config" is for file-level configuration
while "Options" and "Settings" describe runtime parameters. Also applies
rustfmt formatting fixes in web_auth.rs.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
2026-04-02 06:47:42 -07:00
Bryan Helmkamp
c22845e548 Integrate twin-github for fabro-github tests
Add the stripped twin-github test server to the workspace, wire it through
fabro-test, and cover fabro-github's real HTTP auth and pull-request flows
with twin-backed integration tests. This also refactors the GitHub helper
entry points to take explicit base URLs so tests and callers share the same
request path.
2026-04-01 09:48:33 -04:00
Bryan Helmkamp
55181537c3 Suppress unreachable_pub warnings in twin-openai test helpers
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
2026-03-31 22:41:36 -04:00
Bryan Helmkamp
5b7eabee8f Add twin test mode for OpenAI E2E tests
Integrate twin-openai (fake OpenAI server) into the workspace and wire
it into the e2e_test macro so OpenAI tests can run without real API
credentials. The twin server starts in-process via OnceLock on first use
and provides per-test isolation through bearer-token namespacing.

Changes:
- Add Twin as default TestMode, replacing Off (gating now via #[ignore])
- Extend #[e2e_test] macro with `twin` requirement for twin-only,
  live-only, and dual-mode (twin + live) test gating
- Add e2e_openai!() macro returning (base_url, api_key)
- Convert openai_complete and openai_gpt_5_3_codex_complete to dual-mode
- Add new openai_server_error twin-only test with scripted 500 error
- Standardize axum 0.8 as workspace dependency across all crates
- Relax twin-openai ResponsesRequest to accept unknown fields via flatten

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
2026-03-31 20:36:48 -04:00