Commit graph

327 commits

Author SHA1 Message Date
Bryan Helmkamp
4e9dd6fb52
Merge origin/main (preserving RunServices architecture)
Reconciles 61 origin commits (settings/config architectural reshape:
sparse layers → dense snapshots via builders, WorkflowSettings rename,
RunLayer/CliLayer moves, workflow builders, drop of public load wrappers)
with our LLM credential + RunServices refactor.

Our architecture preserved where it conflicted with origin's:

- RunServices / EngineServices stay (services.rs does not exist on
  origin, which inlined the fields onto Initialized). Origin's new
  Initialized fields (inputs, run_store, emitter, sandbox, registry,
  env, dry_run, llm_client, provider) are absorbed through RunServices
  and EngineServices instead of being inlined.
- llm_source: Arc<dyn CredentialSource> stays on AppState and
  RunServices. Origin had a parallel ProviderCredentials struct in
  fabro-server; our CredentialSource trait is more general and
  complies with docs-internal/llm-client-resolution.md. Point-of-use
  Client::from_source(...) rebuild preserves OAuth refresh.
- CommandContext.llm_source() uses self.storage_dir (origin's direct
  field) instead of self.machine_settings (our side's field, removed
  by origin).
- standalone_llm_source in fabro-agent drops the dead Result wrap and
  uses fabro_config::user::default_storage_dir (origin's entrypoint)
  instead of the removed load_settings_user/resolve_storage_root.

Absorbed from origin wholesale:

- SettingsLayer → WorkflowSettings rename everywhere
- Dense run settings: RunOptions.settings is WorkflowSettings, inputs
  read via settings.run.inputs directly (not Option<RunLayer>)
- AppState.manifest_run_defaults / manifest_run_settings
- fabro_config re-exports of CliLayer/RunLayer/CliOutputLayer/etc.
- Lifecycle terminal-event changes, finalize dedup, list_events
  consolidation — already brought in on the previous merge, kept

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
2026-04-24 00:10:01 -04:00
Bryan Helmkamp
b9911fc6e7
Merge origin/main
Reconciles origin's "emit terminal event from FINALIZE" refactor
(41c47dbe1, e8a89ac39, 904c8842f) with the local RunServices refactor.

finalize() now performs origin's single list_events walk for stage
durations + artifact count, origin's compute_final_patch, deduped
stages/billing via billing_from_checkpoint, and origin's terminal event
emission — but reads run_store/sandbox/emitter from the shared
RunServices instead of individual Retroed fields. services.emitter.notice
replaces origin's local emit_run_notice helper.

test_support's execute_and_emit_terminal (added by origin) now accesses
run_store/emitter via executed.engine.run.* since Executed bundles
EngineServices. execute/tests.rs drops the terminal-event status
assertion origin deleted — status is no longer set at EXECUTE end.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
2026-04-23 21:46:24 -04:00
Bryan Helmkamp
099dd881a8
merge origin/main into local main
Integrates origin's worker-JWT-auth work (commits 8a6f83bb0..c847a828d)
with the config-boundary refactor that landed locally. Conflicts
resolved:

- commands/dump.rs: take origin's removal of the 500-line in-process
  test block (replaced by real-server integration coverage).
- commands/run/runner.rs: keep local's dense WorkflowSettings import,
  drop dead SettingsLayer import, pull in origin's ActorRef.
- manifest_builder.rs: adopt origin's lifted working_directory
  resolution (fixes #159 - manifest git detection in nested repos),
  but via local's resolve_working_directory_from_run API that takes
  the dense RunNamespace. Update the regression test's
  ManifestBuildInput literal to local's run_overrides/cli_overrides
  field shape.
- server.rs: keep origin's jwt_auth_mode/jwt_auth_state/
  test_user_subject/issue_test_user_jwt/issue_test_worker_token/
  create_run_with_bearer/bearer_request test helpers, adapt
  jwt_auth_state to local's create_test_app_state_with_session_key
  signature (ServerSettings + RunLayer), keep local's dense
  canonical_origin_settings that returns ServerSettings via
  server_settings_from_toml. Rewrite
  build_app_state_requires_session_secret_for_worker_tokens against
  the dense AppStateConfig (resolved_settings +
  resolved_runtime_settings_for_tests).

Post-merge verification: workspace builds clean, cargo +nightly
fmt --check all clean, cargo +nightly clippy --workspace
--all-targets -- -D warnings clean, cargo nextest run --workspace
4560 tests passed.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
2026-04-23 21:13:28 -04:00
Bryan Helmkamp
ba2eea3148
fix(llm): close remaining source resolution gaps 2026-04-23 19:40:57 -04:00
Bryan Helmkamp
6fc7251471
close config boundary audit and settings snapshot naming 2026-04-23 19:30:30 -04:00
Bryan Helmkamp
904c8842f0
refactor(workflow): consolidate list_events walk and dedupe test helpers
FINALIZE loaded the run event log twice: once via build_conclusion_from_store
for stage durations, then again to count ArtifactCaptured events. Merged into
a single walk feeding both the conclusion and the artifact count.

Collapsed six near-identical pipeline::execute + emit_terminal + flush blocks
in test_support into one execute_and_emit_terminal helper. Also trimmed
narrative comments that described caller ordering, control flow, or the fix
commit rather than non-obvious invariants.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
2026-04-23 19:01:58 -04:00
Bryan Helmkamp
db132d11a7
migrate cli install tests off sparse settings layers 2026-04-23 18:37:37 -04:00
Bryan Helmkamp
41c47dbe12
fix(workflow): emit terminal run event from FINALIZE, not on_run_end
The `WorkflowRunCompleted` / `WorkflowRunFailed` event was emitted from
`EventLifecycle::on_run_end`, a callback the executor fires at the end of
the EXECUTE phase. But the run isn't done at that point — RETRO and
FINALIZE still need to run, and FINALIZE writes the meta branch's finalize
commit. Observers that treat the event as "done" (CLI attach, daemon SSE
consumers) could observe terminal state and act on it before the worker
flushed its remaining writes.

The recovery scenario test exposed this: it deletes the meta branch
right after `fabro run` returns, then asserts the branch is empty. On
loaded CI runners the worker's finalize commit landed after the delete,
recreating the branch and failing the assertion.

Move the terminal event emission to `pipeline::finalize::finalize`, after
`write_finalize_commit`. The lifecycle's `on_run_end` overrides for event
and git become empty (deleted — the trait already provides a no-op
default). Three pieces of cross-cutting state (`final_patch`,
`captured_artifact_count`, the dead `EventLifecycle` reads of
`last_git_sha`) only existed to ferry data from EXECUTE to the terminal
event; deleted those too. The aggregator collapses to a one-line
delegate to `hook.on_run_end`.

`write_finalize_commit` now takes the conclusion as a parameter and
injects it into the projection copy, since the terminal event hasn't run
through the run store yet when the meta branch is written.

`build_terminal_event` is `pub(crate)` so `test_support` helpers (which
stop at EXECUTE) can mirror the production payload.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
2026-04-23 17:48:56 -04:00
Bryan Helmkamp
077469d0c6
refactor(auth): scrub FABRO_WORKER_TOKEN from worker env at startup
The worker subprocess is spawned with env_clear+allowlist by the server, so
the only sensitive value in its env is FABRO_WORKER_TOKEN itself. Read the
token and remove_var it from the process env in main() before Tokio starts
worker threads, then thread it explicitly through runner::execute(&str).

Every descendant (hooks, local sandbox, devcontainer initializeCommand,
MCP stdio, etc.) now inherits a worker env with no bearer in it, so an
unscrubbed spawn site cannot leak the token. This makes the prior denylist
scrub in fabro-hooks and fabro-sandbox redundant — delete it and the shared
WORKER_SECRET_ENV_DENYLIST constant. The sandbox keeps its _api_key/_secret/
_token/_password/_credential suffix heuristic for user-supplied env_vars
hygiene.

Extend the server-dispatched-worker env-leak integration test to also
assert a Bash stage running in the worker does not observe FABRO_WORKER_TOKEN.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
2026-04-23 15:10:58 -04:00
Bryan Helmkamp
fbb924b9c4
chore: fix nightly clippy lints on worker jwt branch
- fabro-client: collapse identical match arms for DevToken/Worker bearer
- fabro-hooks: rewrite filter_map(bool::then) as filter().map() chain
- fabro-sandbox: import WORKER_SECRET_ENV_DENYLIST rather than absolute path
- fabro-server: box large execute_run_in_process future; take path: &str in
  test-only bearer_request; use let-else in session-secret test; replace unit
  pattern _ with () in worker_token request_parts helper; import StatusCode
- fabro-cli run/mod.rs: box large runner::execute future
- fabro-cli worker_auth.rs: drop unused async on shutdown, allow
  clippy::unwrap_used at file level for subprocess test harness setup

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
2026-04-23 12:08:55 -04:00
Bryan Helmkamp
9220af6e80
migrate remaining run settings consumers to dense snapshots 2026-04-23 11:55:12 -04:00
Bryan Helmkamp
0b36ed985b
test(auth): share cli test jwt helpers 2026-04-23 11:42:50 -04:00
Bryan Helmkamp
b2bcf0d5a8
refactor settings builders and dense run snapshots 2026-04-23 11:35:21 -04:00
Bryan Helmkamp
891b7f90ae
test(auth): backfill worker jwt regression coverage 2026-04-23 11:35:08 -04:00
Bryan Helmkamp
519f46038d
refactor(auth): use per-run worker JWTs for subprocesses 2026-04-23 11:04:45 -04:00
Bryan Helmkamp
4ad4d8fd36
Merge remote-tracking branch 'origin/main'
# Conflicts:
#	lib/crates/fabro-cli/src/commands/dump.rs
#	lib/crates/fabro-cli/src/commands/store/mod.rs
#	lib/crates/fabro-cli/src/main.rs
2026-04-23 08:42:16 -04:00
Bryan Helmkamp
37cd8ff45f
Merge remote-tracking branch 'origin/main'
# Conflicts:
#	lib/crates/fabro-server/src/lib.rs
#	lib/crates/fabro-server/src/serve.rs
#	lib/crates/fabro-server/src/server.rs
2026-04-23 08:34:44 -04:00
Bryan Helmkamp
a8623fb996
simplify: flatten commands/store/ after dump rename
The `fabro store dump` -> `fabro dump` rename left `commands/store/` as a
vestigial directory with a stale one-line `StoreRunExport` alias. Move
`dump.rs` and `rebuild.rs` up to `commands/`, import `RunDump` directly,
rename `dump::dump_command` -> `dump::run`, and clean up stale docs and
a noise test that only asserted clap's default error output.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
2026-04-23 07:58:26 -04:00
Bryan Helmkamp
56667c17a8
fix(server): cover secret boundary enforcement 2026-04-23 07:51:28 -04:00
Bryan Helmkamp
cb9b762119
feat(cli): rename store dump to dump 2026-04-23 07:48:03 -04:00
Bryan Helmkamp
b794ec5343
fix(cli): suppress preview open for JSON output 2026-04-23 07:29:37 -04:00
Bryan Helmkamp
e6e091fe8e
refactor(server): lock down server secrets 2026-04-23 07:15:27 -04:00
Bryan Helmkamp
2b933597b2
refactor(cli): finish command context cleanup 2026-04-23 07:14:32 -04:00
Bryan Helmkamp
fa62da5d9d
Merge remote-tracking branch 'origin/main'
# Conflicts:
#	lib/crates/fabro-api/build.rs
#	lib/crates/fabro-server/src/install.rs
#	lib/crates/fabro-server/src/serve.rs
#	lib/packages/fabro-api-client/src/.openapi-generator/FILES
#	lib/packages/fabro-api-client/src/models/index.ts
2026-04-22 21:18:50 -04:00
Bryan Helmkamp
ebb8bf7add
refactor settings API entrypoints 2026-04-22 18:58:47 -04:00
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
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
6ba533a3a4
Adjust store dump export paths 2026-04-22 16:13:27 -04:00
Bryan Helmkamp
7a58ab4e2b
refactor(server): unify daemon runtime metadata 2026-04-22 16:07:52 -04:00
Bryan Helmkamp
d0b3410b28
test: provision auth methods + dev-token in remaining integration tests
The dev-token gating commit added ensure_home_server_auth_methods only to
run_cmd/create_cmd helpers, but many integration tests use context.command()
directly to invoke run/start/attach/etc. Patch the offenders rather than
hoisting auth-injection into command() itself, since command() is also used
by tests (e.g. uninstall) that explicitly want a stable settings file.

- attach, start, scenario lifecycle/recovery, json_global graph: call
  context.ensure_home_server_auth_methods() up front
- validate(): hoist into the helper itself, since every validate test
  needs it
- server_status, uninstall legacy-record tests: bake methods=["dev-token"]
  into their hand-written settings.toml fixtures and pass FABRO_DEV_TOKEN
  via env so the spawned server actually boots
- install: write_artifact_store_metadata_creates_marker test fixture also
  needs explicit methods after the resolver became strict

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
2026-04-22 09:36:55 -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
ff6538b77c
refactor(settings): make fabro settings --local truly client-side
`--local` is supposed to render the settings that apply to the local CLI
/ client side, so it has no business resolving server settings. Drop the
server section and the warning path, return only project/workflow/run/
cli/features. Removes the boundary violation that was about to break the
CI boundary check, and restores the legacy_*_silently_ignored tests to
their original silent assertion.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
2026-04-22 08:36:25 -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
112f9c176a
Merge remote-tracking branch 'origin/main' 2026-04-22 00:17:08 -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
105559bc8a
feat(cli): exit with code 4 on authentication-required failures
Returns exit code 4 whenever the CLI fails because the user needs to run
fabro auth login, so scripts and the install wizard can distinguish
re-auth from generic failures.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
2026-04-21 23:34:21 -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
41135dd9d1
fix(cli): remove explicit remote same-host auth fallbacks 2026-04-20 14:17:01 -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
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
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