Commit graph

22 commits

Author SHA1 Message Date
Bryan Helmkamp
9837a08929
refactor(install): share persistence pipeline (#332)
## Summary

Unifies installer persistence so CLI and web install paths share the
same file/env/vault primitives, while preserving the CLI's server-API
secret persistence and auth bootstrap behavior.

## What Changed

- Added shared `fabro-install` config writers for installer-owned tagged
enum tables, replacing `server.listen` and `cli.target` atomically so
stale variant fields cannot survive.
- Added `InstallPersistencePlan` for disk-backed settings, server env,
and vault writes/removals with the existing rollback semantics for
settings and vault failures.
- Refactored `fabro install`, `fabro install github`, and
`/install/finish` to use the shared persistence plan where their disk
behavior overlaps.
- Preserved full-install ordering: settings/env first, workflow-visible
secrets through the server API second, and CLI `auth.json` only after
API secret persistence succeeds.
- Preserved web installer failure response fields for leftover and
removed env keys, plus the post-success finish hook/shutdown behavior.

## Test Plan

- `cargo nextest run -p fabro-install`
- `cargo nextest run -p fabro-cli commands::install::tests`
- `cargo nextest run -p fabro-cli --test it cmd::install`
- `cargo nextest run -p fabro-server --features test-support --test it
api::install`
- `cargo +nightly-2026-04-14 fmt --check --all`
- `git diff --check`

---

[![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)
2026-05-21 08:04:04 -04:00
Bryan Helmkamp
cd74013d06
refactor(auth): split credential sources and vault schemas (#306)
## Summary

Compared with `origin/main`, this PR splits credential storage and
credential references into explicit types. Vault secrets now distinguish
`token`, `oauth`, and `file` payloads, while runtime/model configuration
points to credentials through explicit `env:<NAME>` and `vault:<NAME>`
source refs.

## Changes

- Replaces the old `environment`/`credential` secret schema vocabulary
with `token`/`oauth`/`file` across OpenAPI, Rust API tests, generated
TypeScript models, CLI/docs references, and the changelog.
- Updates auth resolution, refresh, provider strategies, workflow LLM
handling, server diagnostics, install flows, run manifests, and secret
handlers to consume typed vault entries and explicit credential sources.
- Updates provider catalog TOMLs and config parsing so provider auth and
extra headers use `vault` refs instead of ambiguous `credential` refs.
- Updates CLI install/login/run/secret paths and integration tests to
write and read the new credential shapes.
- Removes the temporary legacy vault migration and empty-vault fallback,
then centralizes provider vault secret-name lookup and Codex API
credential shaping.

## Verification

- `cargo +nightly-2026-04-14 fmt --all`
- `cargo +nightly-2026-04-14 clippy -p fabro-auth -p fabro-model -p
fabro-config -p fabro-vault -p fabro-server -p fabro-cli --all-targets
-- -D warnings`
- `ulimit -n 4096 && cargo nextest run -p fabro-auth -p fabro-model -p
fabro-config -p fabro-vault -p fabro-server -p fabro-cli` (`1938`
passed, `35` skipped)
2026-05-18 11:07:42 -04:00
Bryan Helmkamp
32f100cbe7
feat(install): make LLM setup optional in web installer and CLI (#265)
Some checks failed
Rust / Clippy (push) Waiting to run
Rust / Format (push) Waiting to run
Rust / Generated Docs (push) Waiting to run
Rust / Test (Linux) (push) Waiting to run
Rust / Test (macOS) (push) Waiting to run
TypeScript / Typecheck (push) Has been cancelled
TypeScript / Test (push) Has been cancelled
TypeScript / Build (push) Has been cancelled
## Summary

Makes LLM setup explicitly skippable in both the web installer and
`fabro install`, without making omission accidental. A skipped LLM step
lets install complete with zero LLM credentials; later LLM-dependent
workflows keep using the existing provider-not-configured behavior.
`fabro doctor` is intentionally unchanged.

Plan: `docs/superpowers/plans/2026-05-14-optional-llm-install.md`

## Key changes

**Server + API**
- `PUT /install/llm` now accepts `{"providers":[]}` as "LLM step
completed, skipped" — the empty-list rejection is removed; per-provider
validation for non-empty lists is retained.
- OpenAPI: dropped `minItems: 1` from
`InstallLlmProvidersInput.providers`, updated schema descriptions so
empty = skipped and `llm: null` = incomplete. TypeScript client
regenerated.
- `/install/finish` still requires the LLM step to be present, but
tolerates zero credentials — it writes settings, runtime auth secrets,
and GitHub secrets normally and writes no LLM vault entries.

**Web installer**
- New "Skip LLM setup" secondary action on the LLM step (via a
`secondaryAction` prop on `StepPanel`) that records an empty provider
list and advances to GitHub.
- Review screen shows `LLM providers: Skipped` (step completed, empty)
vs `Not configured` (step never completed), via a new
`describeLlmSummary` helper.
- Continue with no API keys still shows the existing validation error —
skipping is only reachable through the explicit skip action.

**CLI**
- Interactive `fabro install` asks "Configure LLM providers now?"
(default yes) before provider selection; declining returns an empty
selection and continues to GitHub.
- Hidden non-interactive `--skip-llm` flag, mutually exclusive with
`--llm-provider` / `--llm-api-key-stdin` / `--llm-api-key-env` via clap
`conflicts_with_all`. Missing LLM flags are still validation errors
unless `--skip-llm` is present. Non-interactive usage text updated with
a skip example.

## Code review

Ran a 12-reviewer `ce:review` pass (correctness, testing,
maintainability, project-standards, agent-native, learnings, security,
api-contract, reliability, adversarial, cli-readiness,
kieran-typescript). No P0/P1 findings; agent-native parity PASS. Applied
fixes in `40a29c591`:
- Re-entrancy guard on `runStepSubmit` so a fast double-click on "Skip
LLM setup" can't fire two requests.
- `validate()` only suggests `--skip-llm` in the missing-provider error
when no credential flag is set (it conflicts with those flags).
- Added tests: all three `--skip-llm` conflict arms, the review screen's
"Not configured" branch, and the skip-button failure path.

One advisory finding left as report-only: an empty `PUT /install/llm`
overwrites previously-saved credentials if a user navigates Back and
clicks Skip — judged acceptable since the button is explicitly labeled
and clicking it is deliberate.

## Testing

- `cargo nextest run -p fabro-server -p fabro-cli -p fabro-install` —
1521 passed
- `cargo build -p fabro-api`, `cargo fmt --check`, `cargo clippy`
(changed crates) — clean
- `bun test` (install-app) — 14 passed; `bun run typecheck` — clean
- New coverage: server accepts empty providers + session shows `llm`
complete with `providers:[]`; finish with skipped LLM persists no LLM
vault credentials but keeps GitHub secrets; web skip button PUTs
`providers:[]` and navigates to GitHub; review renders Skipped / Not
configured; CLI `--skip-llm` requires `--non-interactive`, conflicts
with all credential flags, `validate()` succeeds with `--skip-llm`,
usage text documents `--skip-llm`.

Not added (out of plan scope): an automated test for the interactive
`InstallInputSource` skip branch — `InteractiveInstallInputSource` is
TTY-coupled and has no existing tests; the non-interactive `--skip-llm`
path is fully covered.

## Post-Deploy Monitoring & Validation

This change is install-time only; there is no continuous runtime impact.
Validate during the next install/release smoke:

- **Web installer:** run a fresh browser install, click "Skip LLM setup"
on the LLM step, confirm it advances to GitHub and the review screen
reads `LLM providers: Skipped`. Finish the install and confirm the
server restarts into normal mode with no LLM credentials in the vault
(`secrets.json` has no credential entries) and
GitHub/server/object-store/sandbox settings written normally.
- **CLI:** run `fabro install --non-interactive --skip-llm
--github-strategy token --github-username <user>` and confirm it
completes; run interactive `fabro install` and confirm declining
"Configure LLM providers now?" continues to GitHub.
- **Healthy signals:** install completes (web `/install/finish` → 202;
CLI exits 0), server boots in normal mode, `fabro doctor` runs and
reports no LLM providers configured (expected, unchanged behavior).
- **Failure signals / rollback trigger:** install fails to finish,
server fails to boot after a skipped install, or `/install/finish`
rejects a completed-but-empty LLM step. Rollback = revert this PR;
install behavior returns to requiring at least one LLM provider.
- **Validation window/owner:** next install smoke / release
verification, owned by whoever runs the release.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

---------

Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
2026-05-14 22:34:39 -04:00
Bryan Helmkamp
e05a04cb55
test(cli): refresh help-text snapshots for backtick-wrapped placeholders
Commit 258e46e95 wrapped angle-bracket placeholders in backticks
without updating the inline snapshots, breaking CI on main.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
2026-05-06 08:29:53 -04:00
Bryan Helmkamp
c28b040c6f
fix(install): reject wildcard public URLs
Normalize bind-address wildcards before presenting install URLs, reject wildcard public origins at CLI and server install boundaries, and surface recovery guidance in the installer and doctor output.
2026-04-25 18:58:00 -04:00
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
7a58ab4e2b
refactor(server): unify daemon runtime metadata 2026-04-22 16:07:52 -04:00
Bryan Helmkamp
5b1c40764d
refactor(cli): enforce CLI/server settings boundary
Move server-only settings reads out of user-facing CLI commands into a
dedicated local_server module, the install/uninstall exceptions, and the
worker subcommand. Adds bin/dev/check-boundary.sh to prevent regressions.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
2026-04-21 23:37:15 -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
a79f9ae43f
fix(install): use consistent web URL for GitHub App callback and generated settings
--web-url defaulted to http://localhost:3000 but merge_server_settings
hardcoded http://127.0.0.1:32276, causing GitHub OAuth redirect_uri
mismatch. Now both derive from the same --web-url flag (default:
http://127.0.0.1:32276 via DEFAULT_TCP_PORT).

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
2026-04-16 07:29:48 -04:00
Bryan Helmkamp
4217d67659
feat(install): add github-only reconfiguration command
Add `fabro install github` for reconfiguring GitHub auth on an
existing install without rerunning full setup.

Ensure app/token switches replace stale settings and secrets, and
cover the new flow with CLI and integration tests.
2026-04-14 23:23:06 -04:00
Bryan Helmkamp
f04e59f83f feat(install): support non-interactive github app setup 2026-04-13 19:42:03 -04:00
Bryan Helmkamp
f3aa30d782 Complete provider credential auth and scripted install 2026-04-13 09:15:45 -04:00
Bryan Helmkamp
330b7ef5e8 test(nextest): share one session server by default
Eagerly start one shared test server per nextest session and point default
TestContext commands at that session socket instead of leaking per-test
daemons keyed by FABRO_STORAGE_DIR. Add isolated_server() for tests that
need an explicit separate daemon, and tighten the ps filtering test so it
still proves the contract without timing out under full-suite load.
2026-04-07 09:44:34 -04:00
Bryan Helmkamp
23231eb1ae refactor(cli): separate local socket and storage defaults
Keep local server targeting based on explicit server targets instead of
implicitly deriving a socket from storage_dir. This makes ~/.fabro/fabro.sock
the default local socket again, keeps storage under ~/.fabro/storage, threads
FABRO_CONFIG through server autostart paths, and updates the CLI test harness
for the new split.
2026-04-06 11:57:14 -04:00
Bryan Helmkamp
3233ef529b refactor(server): make secrets and operational checks server-canonical
Move secret storage, diagnostics, and repo/provider validation behind the
server API so credentials live under the server storage dir and take effect
immediately without process env mutation.

This also removes the old .env runtime path, rewires doctor/install/secret/
provider login/repo init around the server contract, and regenerates the
TypeScript client for the new endpoints.
2026-04-05 17:34:01 -04:00
Bryan Helmkamp
819facbb1e refactor(cli): deglobalize server and storage target flags
Move --storage-dir and --server-url off GlobalArgs and onto the
leaf commands that actually honor them.

This aligns help, parser behavior, and env-var wiring with the
current command architecture while preserving the intended model
and exec targeting semantics.
2026-04-05 16:06:42 -04:00
Bryan Helmkamp
cdddcdeadf refactor(cli): remove global execution mode 2026-04-05 15:20:00 -04:00
Bryan Helmkamp
563a9eeb49 Remove server feature flag from fabro-cli, always compile server in
The server subcommand and related code were gated behind
cfg(feature = "server"). This removes the feature flag entirely,
making fabro-server a required dependency so the server command
is always available.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
2026-04-03 10:09:56 -07:00
Bryan Helmkamp
b56b82d34b Cut over Fabro web app to a server-backed SPA
Replace the old React Router SSR setup with a static SPA build served by
fabro-server, move setup and GitHub auth handling into Rust, and update the
default local web URL and stale Arc-era references to match the Fabro name.
2026-04-01 21:36:01 -07:00
Bryan Helmkamp
e99ebfa3f2 Add global JSON output mode 2026-03-31 07:51:19 -04:00
Bryan Helmkamp
c3634c0740 Replace trycmd tests with fabro-test crate and insta snapshots
Adopt uv's testing pattern: a shared `fabro-test` crate with TestContext
and fabro_snapshot! macro, plus one test file per subcommand under
tests/it/cmd/. This replaces the trycmd-based tests which were hard to
read and didn't compose well with programmatic assertions.

- Create lib/crates/fabro-test with TestContext, run_and_format,
  apply_filters, INSTA_FILTERS, and test_context!/fabro_snapshot! macros
- Add 42 snapshot tests across 16 subcommand files
- Delete trycmd.rs and all tests/cmd/ trycmd files
- Remove trycmd dependency, add fabro-test dev-dependency

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
2026-03-30 11:28:41 -04:00