Commit graph

364 commits

Author SHA1 Message Date
Bryan Helmkamp
e34affcf94
Merge remote-tracking branch 'origin/main'
# Conflicts:
#	lib/crates/fabro-types/src/lib.rs
#	lib/crates/fabro-types/src/status.rs
2026-04-22 17:59:59 -04:00
Bryan Helmkamp
b0b37c5f5e
fix(status): align archive outcomes and clean lint drift 2026-04-22 17:55:07 -04:00
Bryan Helmkamp
18cfd1b92d
refactor: adopt strum for enum string conversions
Replace hand-written Display/FromStr/as_str boilerplate with strum
derives on Provider, RunStatus, StatusReason, Speed, ReasoningEffort,
SandboxProvider, Fidelity, ModelTestMode, ModelTestStatus. Update a few
downstream callers whose FromStr::Err = String assumption no longer
holds. Net -172 lines, zero wire-format change.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
2026-04-22 17:16:25 -04:00
Bryan Helmkamp
6c3211e3ce
refactor(run-status): unify tagged lifecycle states 2026-04-22 17:15:13 -04:00
Bryan Helmkamp
911f957200
refactor(config): move Bind and ServerDaemon into fabro-config
`Bind` and `ServerDaemon` are serde-serialized descriptions of on-disk
server state (the `server.json` record). They belong with
`RuntimeDirectory` in fabro-config rather than in fabro-server's web
layer.

The practical payoff: fabro-test was hand-parsing `server.json` via
`serde_json::Value["pid"]` because fabro-server already depends on
fabro-test (cycle blocked the reverse edge). Moving these types into
fabro-config lets fabro-test call `ServerDaemon::{load_running, read,
remove}` directly, dropping ~20 lines of duplicated record parsing.

fabro-config gains `fabro-proc` and `tempfile` as deps to cover
`ServerDaemon::{is_running, write}`. All 16 `fabro_server::{bind,
daemon}` import sites in fabro-server and fabro-cli are rewritten to
`fabro_config::{bind, daemon}`.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
2026-04-22 16:44:59 -04:00
Bryan Helmkamp
acc6dc49bf
Merge remote-tracking branch 'origin/main' 2026-04-22 16:25:53 -04:00
Bryan Helmkamp
9063a6a129
refactor(server): extract Bind::to_target and harden record removal
Deduplicates the `match bind { Unix(p) => p.to_string_lossy(), Tcp(a) => format!("http://{a}") }`
formatting shared between `worker_command` and the `server_target` test helper
by moving it onto `Bind` itself. Also surfaces unexpected errors from
`ServerDaemon::remove` via `tracing::warn!` instead of silently discarding
them, while still short-circuiting the common `NotFound` path.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
2026-04-22 16:25:46 -04:00
Bryan Helmkamp
7a58ab4e2b
refactor(server): unify daemon runtime metadata 2026-04-22 16:07:52 -04:00
Bryan Helmkamp
27cc1bb6c2
test(server): make global attach filter deterministic 2026-04-22 16:04:33 -04:00
Bryan Helmkamp
0d5e6cb0dd
fix(server): inline test-auth helper so release build compiles
The local ensure_test_auth_methods helper in server.rs is reachable from
the pub fn create_app_state_with_store chain, which is compiled in
release builds even though only integration tests call it. After
2cb623561, the helper called SettingsLayer::ensure_test_auth_methods()
— which is gated behind #[cfg(any(test, feature = "test-support"))] —
so cargo build --release broke with E0599.

Inline the auth-methods setup locally. This one helper only needs the
ServerAuthMethod::DevToken default; it doesn't share the "new required
SettingsLayer field" concern that motivated the centralization.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
2026-04-22 10:56:41 -04:00
Bryan Helmkamp
2cb6235617
refactor(types): centralize test SettingsLayer fixture builder
Add SettingsLayer::test_default() and SettingsLayer::ensure_test_auth_methods()
to fabro-types behind a "test-support" feature, then collapse the five
near-identical ensure_fixture_auth_methods/default_settings/test_default_settings
helpers that the dev-token gating cleanup spread across fabro-config,
fabro-server, and fabro-workflow.

Why: the next required SettingsLayer field would otherwise need updating in
five places. With the canonical helper in fabro-types, adding a required field
becomes a one-line change.

The cfg(any(test, feature = "test-support")) gate keeps the helpers out of
production builds. Consumer crates enable the feature via dev-dependencies.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
2026-04-22 09:56:42 -04:00
Bryan Helmkamp
041aca3ea3
chore: fix pre-existing clippy lints and missing-auth-methods test failures
After removing the implicit [server.auth] dev-token default, several unit
tests still passed empty SettingsLayer values into paths that resolve
server settings, so they panicked with "server.auth.methods: field is
required". Restore them by injecting dev-token methods in test fixtures
(consistent with the existing fabro-config resolve_server test pattern),
and rescue create_test_app_state_with_session_key, which bypassed the
existing ensure_test_auth_methods helper.

Clippy clean-ups unblock `cargo clippy --workspace -- -D warnings`:
- fabro-config: bring SettingsLayer into scope, flatten single-arm match
- fabro-cli: gate storage_dir unit tests with allow(deprecated), drop
  unnecessary borrow, scope effective_settings imports, drop needless
  raw-string hashes
- fabro-server: replace Option<Option<String>> test helper with an
  EnvOverride enum, widen test unwrap → expect

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
2026-04-22 09:17:56 -04:00
Bryan Helmkamp
cdbbe87e38
style: rustfmt --all
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
2026-04-22 08:36:42 -04:00
Bryan Helmkamp
db5c372275
Merge remote-tracking branch 'origin/main'
# Conflicts:
#	lib/crates/fabro-cli/src/commands/install.rs
#	lib/crates/fabro-cli/src/commands/server/foreground.rs
#	lib/crates/fabro-cli/src/commands/server/start.rs
#	lib/crates/fabro-cli/src/server_client.rs
#	lib/crates/fabro-cli/src/user_config.rs
2026-04-22 08:05:49 -04:00
Bryan Helmkamp
77fc778725
Gate dev-token handling on explicit auth methods 2026-04-22 07:47:54 -04:00
Bryan Helmkamp
702c18cbb4
Merge remote-tracking branch 'origin/main' 2026-04-22 00:46:59 -04:00
Bryan Helmkamp
0a587d11cb
refactor(auth): simplify canonical-origin validation and OAuth handlers
Inline the validate_canonical_origin wrapper, move reload-failure logging
to the single caller with accurate wording, drop a hand-rolled tracing
capture layer from tests, and migrate web_auth OAuth handlers to
state.canonical_origin() so the is_empty/resolve guards fall out.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
2026-04-22 00:15:48 -04:00
Bryan Helmkamp
537a5125cb
feat(auth): tighten server auth surface with single origin
Implements plan: single origin, drop CLI preflight, gate demo toggle.
Removes loopback client target and CLI auth config preflight endpoint;
adds canonical_origin module on the server; regenerates SPA and TS API
client.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
2026-04-21 23:38:31 -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
710f9869f0
fix(auth): stop demo mode from overriding authentication
The demo router hardcoded AuthMode::Disabled, which caused /auth/config
and /auth/me to lie and let demo endpoints be reached without a session
whenever the fabro-demo=1 cookie was set. With the cookie set on a
GitHub-configured server, /login rendered "Paste your dev token" with
no input and no GitHub button because /auth/config returned empty
methods.

Have the demo router inherit the real AuthMode so demo mode is purely a
data-source toggle: authentication is identical regardless of the
cookie. Update the translate test that locked in the old bypass, add a
companion test for the authed happy path, and add a regression test
that /auth/config returns real methods under the demo cookie.

As defense in depth, the login page now renders an explicit "no
authentication method is configured" state when methods is empty
instead of the misleading dev-token prompt.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
2026-04-21 16:02:49 -04:00
Bryan Helmkamp
cdba970891
refactor(oauth): brand CLI callback pages to match the Fabro auth theme
The ephemeral loopback server that finishes `fabro auth login` returned
four raw HTML fragments (one literally `<p>Logged in. You can close
this tab.</p>`, and two that weren't even wrapped in a document).
Replace them with a self-contained dark-theme shell that mirrors the
redesigned /auth/cli/resume page: inline Fabro logo SVG, dark panel
over the atmosphere gradient, mint status dot for success, coral for
failure, consistent typography. Shell is fully offline — this process
doesn't have /logo.svg or the SPA CSS available, so everything is
inlined. Also HTML-escape the oauth error_description before
interpolation, and add `white-space: nowrap` to inline <code> in the
resume shell so `fabro auth login` never wraps mid-command.

Copy alignment: success eyebrow "Signed in" + headline "You're signed
in to Fabro"; error eyebrow "Sign-in failed" + headline "CLI sign-in
could not continue", with remediation pointing at the exact command.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
2026-04-21 16:02:49 -04:00
Bryan Helmkamp
964d531a6e
refactor(auth): restyle /auth/cli/resume to match Fabro SPA theme
The CLI login confirmation and error pages rendered as a light-theme
white panel with a navy pill button, jarring against the dark SPA the
user arrives from. Rebuild the inline shell against the app's semantic
tokens — navy page with the same two-radial atmosphere gradient as
app.css, translucent panel, mint status-dot eyebrow, teal-500 primary
button on navy-950 text, Fabro logo at the top — and tighten the
identity card to use a real metadata line instead of a nested
paragraph. Error variant reuses the same shell with a coral eyebrow
and names `fabro auth login` explicitly in the remediation copy. Button
now reads `Continue as @login`, matching the identity row and making
it read as a GitHub handle rather than a bare string.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
2026-04-21 16:02:49 -04:00
Bryan Helmkamp
3f6cc1f1ba
Merge remote-tracking branch 'origin/main' 2026-04-21 14:47:23 -04:00
Bryan Helmkamp
328bddea33
fix(install): pass GitHub App manifest state as form field, not URL query
GitHub's App Manifest endpoint rejects `redirect_url` values that carry a
query string with "invalid redirect_uri", leaving the web wizard stuck:
the 10-minute pending-setup guard then blocked every retry for ten
minutes. Move the CSRF state out of `redirect_url` and into a hidden
`state` form field on the auto-submit — GitHub preserves it on the
callback, matching the CLI's working Manifest flow. Drop the retry
conflict so a fresh POST to /install/github/app/manifest always replaces
the pending entry and mints a new state token; stale callbacks are
already rejected by the existing state-match check on the redirect
handler.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
2026-04-21 14:21:33 -04:00
Bryan Helmkamp
3d831ef273
fix(auth): restrict CLI start session reuse to GitHub auth
Dev-token sessions now carry a non-empty IdpIdentity, so filtering by
identity presence alone let the CLI start flow auto-resume under a
dev-token session. Tighten eligibility to GitHub-authenticated sessions
and update the auth_harness test helper to pass auth_mode by reference
to match the current build_router_with_options signature.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
2026-04-21 12:13:07 -04:00
Bryan Helmkamp
17020ee445
Merge remote-tracking branch 'origin/main' 2026-04-21 10:58:30 -04:00
Bryan Helmkamp
6eee5a5a72
Merge remote-tracking branch 'origin/main' 2026-04-21 09:44:10 -04:00
Bryan Helmkamp
a3e4e2bd3d
Merge remote-tracking branch 'origin/main' 2026-04-21 09:40:09 -04:00
Bryan Helmkamp
7b28b12f49
refactor(auth): simplify translation middleware and jwt_auth
- drop per-request AuthMode clone in auth_translation_middleware
- remove dead CredentialSource enum and VerifiedAuth field
- collapse cookie_key_error + jwt_key_error into session_secret_key_error
- use header::AUTHORIZATION constant in bearer_token
- remove narrating doc comments on extractor structs

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
2026-04-21 09:39:54 -04:00
Bryan Helmkamp
ab6c7f3e8c
test(auth): cover refresh-token pass-through and session+demo mint
Two middleware gaps from the translation-refactor plan: a
fabro_refresh_* bearer should flow through unchanged, and a session
cookie should still mint a JWT when x-fabro-demo is also set.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
2026-04-21 09:13:39 -04:00
Bryan Helmkamp
e92a2ba9db
refactor(server): drop duplicate run status adapters
Use the unified status family directly in server.rs and remove the manual
conversion helpers that only existed to bridge duplicate API/domain types.
2026-04-21 09:10:57 -04:00
Bryan Helmkamp
7a70af1e2b
refactor(auth): translate non-jwt auth into bearer tokens
Move session cookie and dev credential handling into middleware so the
real router only sees Bearer JWTs. This also carries profile claims
through /auth/me and requires session signing material whenever auth is
enabled.
2026-04-21 09:05:20 -04:00
Bryan Helmkamp
0d2be41e57
Merge remote-tracking branch 'origin/main' 2026-04-21 08:38:37 -04:00
Bryan Helmkamp
bf1aa6eed5
Merge remote-tracking branch 'origin/main'
# Conflicts:
#	lib/crates/fabro-store/src/lib.rs
#	lib/crates/fabro-store/src/slate/mod.rs
#	lib/crates/fabro-store/src/slate/run_store.rs
2026-04-21 08:35:07 -04:00
Bryan Helmkamp
f75e5c2ef6
refactor(store): extract Record/Repository abstractions
Replaces hand-written K/V stores in fabro-store with a shared Record trait
plus Repository<R> typed K/V layer. Adds KeyedMutex for per-key serialization
and transaction() for all-or-nothing WriteBatch commits. Renames
SlateAuthCodeStore/SlateAuthTokenStore to AuthCodeStore/RefreshTokenStore and
adds BlobStore and RunCatalogIndex wrappers on top of Repository. Deletes
catalog.rs in favor of RunCatalogIndex. Database gains blobs() and
catalog_index() accessors; auth_tokens() is renamed refresh_tokens().

Plan: docs/plans/2026-04-20-003-refactor-fabro-store-record-abstractions-plan.md

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
2026-04-21 08:29:34 -04:00
Bryan Helmkamp
705dccdea7
Merge remote-tracking branch 'origin/main'
# Conflicts:
#	lib/crates/fabro-store/src/lib.rs
#	lib/crates/fabro-store/src/run_state.rs
2026-04-21 07:53:10 -04:00
Bryan Helmkamp
7dd058cc40
refactor: unify run vocabulary and metadata snapshot layout
Implements the plan at
docs/plans/2026-04-20-003-refactor-unify-run-vocabulary-metadata-plan.md.

- Rename RunRecord to RunSpec and RunProjection.run to .spec everywhere
  in Rust source, tests, helpers, test names, and error messages.
- Introduce SerializableProjection wrapper that trims bulky node text
  fields (prompt, response, diff, stdout, stderr) for run.json snapshots.
- Collapse metadata-branch and CLI export to one RunDump::from_projection
  builder emitting run.json + graph.fabro + stages/{stage_id}/... and
  drop legacy top-level start/status/checkpoint/sandbox/retro/conclusion
  split files.
- Replace MetadataStore::write_checkpoint with write_snapshot returning
  the commit SHA; add read_run_projection/read_run_spec; demote
  read_checkpoint/read_start_record to projection-field extractors.
- Switch fork, rewind, rebuild_meta, CLI rewind recovery, and retro
  upload to read the unified projection layout.
- Add additive query methods on RunSpec and RunProjection.

Serde-level `alias = "spec"` shim dropped; `rename = "run"` retained to
keep the server API wire format stable per the plan's scope boundary.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
2026-04-20 22:17:23 -04:00
Bryan Helmkamp
f0f04abf44
refactor(client): extract fabro-client crate
Lift shared client DTOs into fabro-types, move auth/target/error/session
logic into fabro-client, and reduce fabro-cli to orchestration around the
builder-based client path.

This also lands the remaining plan cleanup for ApiError, ServerTarget
canonicalization, and the RunEventStream rename at the CLI boundary.
2026-04-20 20:42:20 -04:00
Bryan Helmkamp
c6cafceae0
test: de-flake pr_list and full_http_lifecycle_cancel
Two CLI/server tests racing against peer state on the shared fabro server
session, surfaced by running the default nextest profile 20 times.

pr_list_missing_github_credentials_errors depended on an empty shared
store; if pr_view_reads_pull_request_from_store_without_pull_request_json
ran first it left a PR record behind and this test hit the
credentials-required branch instead of "No pull requests found." The
snapshot captured the empty path, but the test name promises the error
path. Seed a PullRequestCreated event against the test's own run so the
store is guaranteed non-empty and the credentials-required error fires
deterministically.

full_http_lifecycle_cancel asserted that the cancel response body's
pending_control == "cancel", but that field is re-read from the store
projection after the worker has been signaled. The worker is sitting at
a human gate; on hot CI it can emit a clearing event before the handler
re-reads the projection, yielding a legitimate null. Relax the
assertion to accept "cancel" or null; durable convergence to
failed/cancelled is still asserted below.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
2026-04-20 15:04:08 -04:00
Bryan Helmkamp
ed6fecfc5a
fix(auth): harden loopback checks and align CSP tests
Tighten CLI loopback target classification to use literal host checks,
update explicit local TCP auth coverage to match the remote-target
contract, and align server CSP assertions with the current external-script
SPA bundle. Also enable reqwest cookies in fabro-http so package-scoped
server tests compile without relying on workspace feature unification.
2026-04-20 13:30:45 -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
4c35c4b69f
Merge remote-tracking branch 'origin/main'
# Conflicts:
#	lib/crates/fabro-server/src/jwt_auth.rs
#	lib/crates/fabro-server/src/serve.rs
#	lib/crates/fabro-server/src/server.rs
#	lib/crates/fabro-server/src/web_auth.rs
2026-04-20 09:47:47 -04:00
Bryan Helmkamp
153fcf20b5
fix(auth): require browser confirmation for CLI login
Harden the CLI browser auth flow by moving auth-code issuance behind
an explicit same-origin confirmation step, and update the real-browser
test harness to submit the confirmation page.
2026-04-20 09:40:05 -04:00
Bryan Helmkamp
1ed8e6cbd5
fix(server): treat zombie processes as stopped
Split raw PID existence from actual process liveness in fabro-proc and
switch the server shutdown paths to the running-process predicate. This
avoids waiting out stop timeouts for unreaped zombie children while
keeping process-group behavior covered by measured regression tests.
2026-04-20 09:05:29 -04:00
Bryan Helmkamp
6ac8bf6d81
fix(auth): harden CLI auth review gaps
Add the missing refresh transport guard, actionable auth-store lock errors
for unsupported filesystems, explicit OAuth state expiry, and the remaining
CLI auth regression coverage around replay revocation, HTML headers, and
secret-safe logging.
2026-04-20 09:03:22 -04:00
Bryan Helmkamp
277fe8ca5f
refactor(server): relocate github helpers and use macros for test context
- Move the GitHub App webhook config update to fabro-github as
  update_app_webhook_config, matching the crate's existing HttpClient +
  Result<_, String> conventions. Server-side callers go through the new
  symbol.
- Add Bind::tcp_port() on the enum itself and drop the free function.
- Collapse the six near-identical "webhook strategy configured but ...;
  skipping webhook startup" warn branches into resolve_webhook_preconditions
  returning a Ready/Skip enum, with one warn! at the call site.
- Replace the per-file test-helper wrappers (assert_status, checked_response,
  response_json, response_bytes) with local macro_rules! macros so
  file!()/line!() expand at the caller. Panic context now identifies the
  failing assertion's source line instead of the wrapper's definition.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
2026-04-20 08:39:12 -04:00
Bryan Helmkamp
86bb88cdca
refactor(server): tidy webhook wiring and test-helper context
- Distinguish the two GitHub webhook auth-failure warn messages (missing
  signature header vs. HMAC mismatch) so logs can tell them apart.
- Route update_github_app_webhook through fabro_github::github_api_base_url()
  so GITHUB_BASE_URL overrides the webhook config endpoint too.
- Drop a narrative shutdown comment that restated the next two lines.
- Replace concat!(file!(), ":", line!()) inside local test-helper wrappers;
  those macros expand at the wrapper definition site, so every panic
  reported the same phantom location. Pass the wrapper name instead.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
2026-04-20 08:27:50 -04:00
Bryan Helmkamp
9c3c66c59a
test(http): improve HTTP test failure diagnostics
Add shared axum/reqwest response assertion helpers in fabro-test,
migrate the Rust HTTP test surface to use them, and document the
new rule in the testing strategy.
2026-04-20 08:06:14 -04:00
Bryan Helmkamp
1ef10b46ce
refactor(server): simplify github webhook wiring
- Capture webhook secret at route mount time via Arc<[u8]> router
  state so the handler drops its per-request server_secret lookup
  and the dead NOT_FOUND fallback.
- Extract WEBHOOK_ROUTE and WEBHOOK_SECRET_ENV constants; apply
  across serve.rs, server.rs, and TailscaleFunnelManager so the
  mounted route and the URLs pushed to GitHub cannot drift.
- Flatten the seven-level nested webhook startup match in serve.rs
  into a single start_webhook_strategy helper with early returns,
  short-circuiting when the secret is absent and replacing the
  server.api.url .expect with a propagated error.
- Share compute_signature and a new read_repo_file helper across
  tests; delete the duplicated webhook_signature, TestHmacSha256,
  and read_doc/repo_root copies.
- Replace the nested for-loops in the new webhook auth tests with
  five flat #[tokio::test] cases per CLAUDE.md.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
2026-04-20 07:32:38 -04:00
Bryan Helmkamp
0686b82bce
fix(api): correct GitHub webhook OpenAPI contract
Model the GitHub webhook request body as JSON so the generated TypeScript
client exposes a coherent request shape, and add explicit conformance
coverage for the secret-gated webhook route.
2026-04-20 07:13:49 -04:00