Commit graph

1150 commits

Author SHA1 Message Date
Bryan Helmkamp
64cf35cc7d
fix(lint): restore clippy cleanliness after async ripple from std::fs work
Two follow-ups the workspace lint now catches:

- fabro-server tests/it/api/install.rs: a newer install-router integration
  test was missing the `.await` after `build_install_router(...)` -- the
  fn became async when the devcontainer/install-mode resolver was
  converted to tokio::fs in commit 19939c5f0.
- fabro-cli main.rs: add #[expect(clippy::disallowed_methods)] to the
  #[cfg(test)] module whose write_test_settings helper uses sync
  std::fs::write to stage CLI settings fixtures.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
2026-04-19 19:41:47 -04:00
Bryan Helmkamp
a2b77b1a30
Merge remote-tracking branch 'origin/main'
# Conflicts:
#	lib/crates/fabro-cli/src/commands/run/rewind.rs
#	lib/crates/fabro-cli/src/logging.rs
2026-04-19 19:33:40 -04:00
Bryan Helmkamp
26db764e6c
feat(server): resolve run selectors on the server
Add a server-native run selector endpoint and migrate CLI single-run flows to
use it instead of local workflow-store heuristics. This also moves store dump
export assembly into the CLI, removes the production CLI dependency on
fabro_workflow run lookup and dump helpers, and records the remaining
cli-to-workflow coupling in an audit document.
2026-04-19 19:31:13 -04:00
Bryan Helmkamp
32b88d7833
fix(async-fs): move remaining filesystem work off Tokio paths
Replace the remaining blocking filesystem touches in shared async code with
Tokio-native I/O or explicit blocking boundaries. This keeps provider file
loading, workflow metadata rebuilds, and related export paths compatible with
the stricter clippy async-fs rules without changing their external behavior.
2026-04-19 19:31:03 -04:00
Bryan Helmkamp
4959778ea6
Merge origin/main into archived-run-status feature branch
Origin brought 21 commits of UI/install/test-helpers work that lived in
parallel with the archive feature. Only the SPA build outputs conflicted
(old bundle hashes on both sides). Resolution: accept origin's
resolution on the deleted files, then re-run scripts/refresh-fabro-spa.sh
from the merged source so the embedded bundle reflects both sides —
origin's Settings-nav/theme/stage-sidebar work plus this branch's
archived-status TypeScript changes in apps/fabro-web/app/data/runs.ts.

Verification:
- cargo build --workspace: clean
- cargo nextest run --workspace: 4198 passed, 182 skipped
- cargo +nightly-2026-04-14 clippy --workspace --all-targets -- -D warnings: clean
- cargo +nightly-2026-04-14 fmt --check --all: clean
- apps/fabro-web bun run typecheck: clean
- apps/fabro-web bun test app/data/runs.test.ts: 8 pass

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
2026-04-19 18:57:07 -04:00
Bryan Helmkamp
a21ab24521
fix(store): harden archive/unarchive projection apply against corrupt logs
Adversarial review surfaced that both RunArchived and RunUnarchived apply
arms were naive — any out-of-spec event in the log (concurrent double
archive, tampered import, replayed retry) would permanently corrupt the
projection:

- RunArchived unconditionally captured current status into prior_status.
  A second RunArchived would set prior_status=Some(Archived). Unarchive
  would then emit restored_status=Archived, the apply arm would set
  status=Archived and clear prior_status, and the run would be unrecoverable.
- RunUnarchived trusted restored_status unconditionally. An imported
  event with restored_status=Running produced a projection reporting
  status=Running with no RunRunning event in the log — breaking
  is_active/is_terminal invariants.

Both arms now require a sensible pre-state before mutating:
- RunArchived only transitions from Succeeded|Failed|Dead.
- RunUnarchived only runs from Archived with a terminal restored_status.

Adds three regression tests:
- double_archive_preserves_prior_status
- run_unarchived_with_non_terminal_restored_status_is_ignored
- run_archived_on_non_terminal_projection_is_ignored

The operations layer (archive/unarchive in fabro-workflow) still validates
at emit time; the projection guards are a defensive second line for
replay, imports, and any future code path that double-writes.
2026-04-19 18:40:21 -04:00
Bryan Helmkamp
48eb5efed6
refactor(archive): consolidate helpers after review
Three cleanups from `/simplify` review:

- Promote `archived_rejection_message` and `ensure_not_archived` to `pub`
  via operations/mod.rs and reuse them from `resume`, the CLI rewind
  caller, and the server's `reject_if_archived` guard so the canonical
  error string lives in exactly one place.
- Tighten `RewindInput.current_status` from `Option<RunStatus>` to
  `RunStatus`. The runtime check for None was enforcing a compile-time
  invariant. CLI callers already load the projection and now surface a
  clean error up-front if it's missing. Drop the None-branch test that
  existed only to cover the removed runtime check.
- Collapse `archive_run` / `unarchive_run` HTTP handlers into a shared
  `run_archive_action` body with an `ArchiveAction` enum, mirroring the
  CLI pattern. Removes ~20 lines of copy-paste and unifies error-mapping.

Also drop narrative comments that referenced plan unit numbers in the
scenario tests, and clean up the convoluted `ps_runs` helper pattern
that built an empty-slot arg vec before filling it in.

No behavior change. Full workspace: 4185 tests pass, clippy clean.
2026-04-19 18:14:16 -04:00
Bryan Helmkamp
828a09636e
test(server): cover submit_answer, artifact, sandbox, blob guards
Extends `archived_runs_reject_mutations_with_actionable_body` to assert
the archive guard fires on the four write surfaces the Unit 4 audit
guarded but the scenario skipped: POST /questions/{qid}/answer, POST
/stages/{stage_id}/artifacts, PUT /sandbox/file, POST /blobs. Synthetic
stage/question/filename values are fine — `reject_if_archived` runs
before each endpoint's state-specific lookups.
2026-04-19 17:59:32 -04:00
Bryan Helmkamp
7fdcf3156b
fix(cli): correct archive/unarchive bulk failure grammar
Bulk error messages now read 'could not be archived' / 'could not be
unarchived' instead of the broken 'could not be archive'. Caught by
manual smoke: the previous `verb_ing()` helper returned the base verb
for both forms. Dropped `verb_ing()` and reused the already-correct
`past()` helper.
2026-04-19 17:53:10 -04:00
Bryan Helmkamp
ac10cb8016
polish: clarify -a hint, test archived status in web UI, tighten rewind input
- Empty-list hint in `fabro ps` now mentions archived explicitly so users
  discover the new surface (plan Unit 7 follow-up).
- apps/fabro-web runs.test.ts gains an `isRunStatus('archived')` +
  `runStatusDisplay` assertion so the web UI type stays in lockstep with
  the Rust enum.
- `operations::rewind` now requires callers to pass `current_status`
  rather than silently skipping the archived guard when absent, closing a
  silent-bypass hole.
2026-04-19 17:43:00 -04:00
Bryan Helmkamp
3ae46b37c1
test(server): verify R14 contract and tighten rewind guard
Scenario coverage for the plan's R14 read-only-on-archived contract over
HTTP:

- archived_runs_reject_mutations_with_actionable_body drives a run to
  succeeded, archives it, then asserts 409 on /cancel, /pause, /unpause,
  /start, and /events with the actionable 'fabro unarchive' body.
- appending_run_archived_event_directly_is_rejected covers the widened
  denylist on append_run_event.
- archive_returns_404_for_unknown_run proves the RunNotFound mapping.
- list_runs_respects_include_archived_flag exercises Unit 5's listing
  filter.

Also adds inline server.rs tests that pin the spec/router behavior at
the unit layer and documents that rewind.rs now requires callers to
pass current_status (already threaded through from the CLI and scenario
tests).
2026-04-19 17:42:50 -04:00
Bryan Helmkamp
05743050ec
test(cli): add end-to-end archive lifecycle scenario
Single #[test] that exercises the full CLI archive flow: run a dry-run to
succeeded, verify ps -a shows it, archive, verify default ps hides it and
ps -a shows archived, unarchive, verify the prior terminal status is
restored, then re-archive and rm to confirm archived runs remain
delete-able (plan Scope Boundaries).
2026-04-19 17:42:43 -04:00
Bryan Helmkamp
13a501ec1d
test(cli): add archive and unarchive command integration tests
Adds the CLI-layer integration coverage the archived-run plan called for
in its Unit 6 test scenarios but never landed: help snapshots, required-arg
handling, happy paths (including ps/ps -a visibility switching), precondition
errors (archive on active runs, unarchive on not-archived runs), unknown-id
errors, idempotent no-ops, JSON output shape, and mixed-batch per-id error
aggregation. 15 new tests across archive.rs and unarchive.rs mirror rm.rs's
fabro_snapshot style.
2026-04-19 17:42:37 -04:00
Bryan Helmkamp
5c248fc885
fix(clippy): clean up nightly-clippy findings on merged CLI work
Nothing behavioral — each change is what clippy asked for:

- fabro-test: wrap the three polling-helper thread::sleep calls in a
  single poll_sleep() with an #[expect(clippy::disallowed_methods,
  reason = …)] since the helpers are deliberately blocking
- fabro-test: server_log_files now uses Path::extension() with
  eq_ignore_ascii_case("log") instead of a case-sensitive ends_with
- fabro-workflow: import default_storage_dir rather than calling it
  through its full module path
- fabro-cli/server/record: same absolute_paths fix
- fabro-cli/main tests: use a `use tokio::runtime::Runtime` to stop
  referencing `tokio::runtime::Runtime` by full path
- fabro-cli/tests: replace three `as u32` casts on as_u64() results
  with u32::try_from(...).expect(…)
- fabro-cli/tests: six `format!("...", var)` assertions switched to
  the inline `{var}` form clippy prefers

Full verification passes: fmt, clippy, cargo nextest (4141 tests),
bun typecheck, bun test (40 tests), bun build, SPA embed diff clean.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
2026-04-19 17:39:06 -04:00
Bryan Helmkamp
948d59062a
Merge origin/main into UI cleanup branch
Brings in the CLI storage/logging refactor (858e8e127), bootstrap
helper collapse (a256c14a7), and shared server-lifecycle test helpers
(f8c560a9a). No overlap with the web UI work on this branch.
2026-04-19 17:30:22 -04:00
Bryan Helmkamp
9b5bc8ede4
style(run-stages): copy affordance, collapsed long output, sticky header
- extract a shared CopyButton into components/ui.tsx and drop the
  install wizard's local duplicate
- sticky stage header at the top of the turn stream so users always
  know which stage they're reading as they scroll
- copy-to-clipboard button on System, Assistant, and Command blocks;
  revealed on hover/focus
- stdout/stderr longer than 20 lines collapse to the last 20 with a
  "Show N earlier lines" expander
- bump the [10px] labels in tool-use.tsx to [11px] for readability

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
2026-04-19 17:27:25 -04:00
Bryan Helmkamp
54867d29ed
feat(web): unify empty/error/loading states in one primitive
Adds apps/fabro-web/app/components/state.tsx exposing EmptyState,
ErrorState, and LoadingState on a shared StatePanel chrome so every
"the content isn't ready" surface looks like the same app.

Swaps in place of bare <p> tags and ad-hoc bordered divs:
- run-detail: "Run not found" is now an ErrorState
- run-stages: "No stages yet" is an EmptyState
- run-overview: empty-graph panel is an EmptyState
- run-billing: empty-billing panel is an EmptyState
- runs: filtered-empty ("no matching runs") now renders an EmptyState
  (the branded landing empty is preserved as RunsLandingEmpty)
- install-app: session-loading StatusPanel replaced by LoadingState

No change to the root ErrorBoundary — full-page crashes stay there.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
2026-04-19 17:22:57 -04:00
Bryan Helmkamp
b87265867a
fix(archive): close mutation gaps and correct error mapping
Four fixes from post-merge review:

- Freeze archived runs on the remaining write surfaces the Unit 4 audit
  missed: `put_stage_artifact`, `put_sandbox_file`, and `write_run_blob`
  now all call `reject_if_archived` so a client cannot mutate artifacts,
  sandbox files, or blobs on an archived run.

- Widen the `append_run_event` lifecycle denylist to cover every event
  with a dedicated operation endpoint: archive, unarchive, and the three
  control-request events (cancel/pause/unpause). Worker-emitted lifecycle
  transitions and rewind's `RunRewound` / `RunSubmitted` replay still flow
  through the endpoint as before.

- Map `fabro_store::Error::RunNotFound` to a distinct `Error::RunNotFound`
  at the operations layer so the archive and unarchive HTTP handlers return
  a 404 on unknown run ids instead of collapsing into a generic 500.

- Centralize the archived-run guard in `operations::rewind` by threading
  `current_status` through `RewindInput` and calling the new
  `ensure_not_archived` helper alongside a shared canonical error message.
  The CLI caller drops its ad-hoc string comparison in favor of the typed
  status it already loads from the server.
2026-04-19 17:22:56 -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
b448a4e246
fix(theme): contrast on primary buttons and muted text in light mode
- introduce --color-on-primary (navy-950 in dark, white in light)
  so text on bg-teal-500 reads clearly regardless of mode; swap
  hardcoded text-navy-950 occurrences on teal fills for text-on-primary
- darken --color-fg-muted in light mode from slate-400 (#94a3b8) to
  slate-500 (#64748b); slate-400 failed AA on the tinted page
- deepen page tint to #eef2f7 and strengthen line/line-strong so
  white cards have real edges, not invisible hairlines

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
2026-04-19 17:12:01 -04:00
Bryan Helmkamp
d0e23848d8
style(theme): soften light mode so cards read as the lighter surface
Light mode had the page at pure #ffffff with panels at #f8fafc — so
panels read darker than the page, the opposite of dark mode's
hierarchy and a big source of "blinding white" fatigue.

- page tinted to #f3f6fa (cool off-white, matching the brand's navy
  palette) so it no longer glows
- panel set to #ffffff so cards, the nav, and auth panels pop
- panel-alt (#e9eef5) sits between them for recessed wells
- overlay / line colors shifted from pure black rgba to the navy tint
  so the whole system reads coherent

Dark-mode tokens unchanged.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
2026-04-19 17:08:31 -04:00
Bryan Helmkamp
74cd7fe434
fix(settings): drop inner max-w wrapper so content aligns with shell header
The page wrapped its body in mx-auto max-w-4xl, which centered the
description and JSON inside the shell's max-w-5xl column. The
shell's "Settings" header used the outer 5xl bounds, so everything
below it shifted right. Let the page inherit the shell's width.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
2026-04-19 17:04:59 -04:00
Bryan Helmkamp
4e4937c714
docs(settings): tighten copy — point at settings.toml instead of the CLI
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
2026-04-19 17:04:04 -04:00
Bryan Helmkamp
81903b9ba4
chore(spa): refresh embedded bundle for Settings nav item
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
2026-04-19 17:03:28 -04:00
Bryan Helmkamp
9d1c0d98c7
fix: migrate blocking fs calls off Tokio paths
Phase 1 of the std::fs lint initiative. Refactors blocking std::fs entry
points that ran inside async contexts. Caller chains either converted to
async (using tokio::fs) or wrapped in tokio::task::spawn_blocking where
sync callers were already natural (Command builders, flock semantics).

HIGH (per-request async hot paths):
  - fabro-sandbox local.rs: wrap recursive std::fs::read_dir traversal in
    spawn_blocking. Fixes /api/runs/{id}/files stalling workers under
    concurrent or deep listings.
  - fabro-server static_files.rs: convert serve/serve_install/serve_with_mode
    and the static-asset load chain to async; use tokio::fs::read for the
    debug-only disk fallback. Cascades through install.rs build_install_router
    (now async) and ~17 test call sites.

LOW (async but not per-request):
  - fabro-workflow artifact.rs: sync_artifacts_to_env, offload_large_values
    → tokio::fs::read_to_string.
  - fabro-workflow artifact_snapshot.rs: compute_artifact_info → async +
    tokio::fs::read.
  - fabro-server ip_allowlist.rs: load_cache and store_cache → async +
    tokio::fs::{read,write,create_dir_all}.
  - fabro-server server.rs: wrap worker_command invocation in spawn_blocking
    at the async boundary in execute_run_subprocess; keep the sync
    worker_command + current_server_target signatures intact.
  - fabro-cli server/start.rs: wrap the OpenOptions::open call in
    acquire_lock in spawn_blocking; file-lock semantics require a real
    std::fs::File, and the flock polling loop stays async with time::sleep.

Deferred:
  - fabro-llm load_file_as_base64 (file:// attachment loader): 7 call sites
    across 4 providers, each inside sync translators. Left for Phase 3
    annotation with a FOLLOW-UP marker; file:// URLs are rare in practice.

Verified: workspace builds, 4131 tests pass, 182 skipped. The lint that
enforces this discipline lands in the next commit.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
2026-04-19 17:00:56 -04:00
Bryan Helmkamp
55c396d2ff
feat(cli+web): wire archived into listing visibility
The CLI's store-run lookup now passes `include_archived=true` so resolve
and bulk operations (archive, unarchive, rm, inspect, rewind) can still find
archived runs. The web UI's hand-maintained `RunStatus` union and display
map learn `archived` with a gray style so archived runs render correctly.
Default `fabro ps` continues to hide archived via `is_active()`; `-a`
shows everything including archived.
2026-04-19 16:58:55 -04:00
Bryan Helmkamp
8bf243fe47
feat(cli): add fabro archive and fabro unarchive commands
Two new top-level commands mirror `fabro rm`'s bulk-by-ID shape: positional
run identifiers, per-ID success/error aggregation, and a final non-zero exit
if any item failed. Calls the new server endpoints from Unit 5. Shared bulk
loop covers both directions and emits structured JSON with an `archived` or
`unarchived` list alongside `errors`. Top-level help snapshot updated.
2026-04-19 16:56:53 -04:00
Bryan Helmkamp
03fc375cdd
feat(server): add archive/unarchive endpoints and include_archived listing
Extends the OpenAPI spec with POST /api/v1/runs/{id}/archive and /unarchive
operations, adds `archived` to the RunStatus enum, and adds an
`include_archived` query param to listRuns. Regenerates the progenitor-built
Rust types and the typescript-axios client. Implements `archive_run` and
`unarchive_run` handlers via `operations::archive/unarchive`, and extends
`list_runs` to filter archived runs unless opted in. Archived runs continue
to map through `api_status_from_workflow` and bypass the board column.
2026-04-19 16:53:33 -04:00
Bryan Helmkamp
cd42d9bb8e
chore(spa): refresh embedded bundle for the real-mode UI cleanup
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
2026-04-19 16:52:06 -04:00
Bryan Helmkamp
ce85151b37
feat(server): reject mutations on archived runs with actionable error
Adds an `archived → unarchive first` guard to every mutation entry point
that could otherwise hit an opaque 409 or confusing 404 on an archived run:
start, cancel, pause, unpause, submit_answer, and append_run_event server
handlers; the resume operation; and the rewind CLI command. append_run_event
also rejects client-injected `run.archived` and `run.unarchived` bodies so
lifecycle transitions cannot bypass the operations layer. Worker-emitted
run.completed / run.failed events still flow through as before. Fork reads
from the source's metadata branch only — no source mutation — so no guard
is needed there.
2026-04-19 16:44:23 -04:00
Bryan Helmkamp
f8c560a9a6
refactor(test): promote shared server-lifecycle test helpers into fabro-test
Move wait_for_path, wait_for_log_line, stop_pid, server_log_files, and
isolated_storage_dir out of the three integration test files that duplicated
them and into fabro-test's public surface next to apply_test_isolation.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
2026-04-19 16:43:26 -04:00
Bryan Helmkamp
a256c14a77
refactor(cli): collapse duplicated bootstrap and settings helpers
- Merge prepare_foreground_server_bootstrap and prepare_server_sink_bootstrap
  into one prepare_server_bootstrap(config, storage, foreground).
- Drop three one-line settings_layer_* passthroughs from user_config; callers
  now use load_settings_with_{storage_dir,config_and_storage_dir} directly.
- Swap underscore-prefixed lock field for #[expect(dead_code, reason=…)] to
  document RAII intent explicitly.
- Remove two narrate-what-it-does comments.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
2026-04-19 16:40:33 -04:00
Bryan Helmkamp
34e1d806a6
feat(workflow): add archive/unarchive operations
Centralizes the terminal-only precondition, idempotent behavior, and
event emission for archiving and unarchiving runs. Both operations
return typed outcomes distinguishing a real transition from an idempotent
no-op. `unarchive` reads prior_status from the projection (populated by
the RunArchived apply arm) rather than scanning the event log, so replay
stays pure append-and-apply.
2026-04-19 16:37:19 -04:00
Bryan Helmkamp
858e8e1270
fix(cli): unify server state and logging under storage
Route server-owned logs to <storage>/logs/server.log from the start of
tracing, remove legacy home/config ownership paths, and fail fast when
a running legacy daemon is detected instead of silently proceeding.

This also adds the missing sink-resolution, truncate/append,
concurrency, legacy-config, and uninstall regression coverage for the
home/storage cleanup plan.
2026-04-19 16:34:27 -04:00
Bryan Helmkamp
54ddaa2cee
feat(install): redesign web wizard and scope dev token to PAT installs
Redesign the install wizard for clarity:
- swap the sidebar layout for a centered column and a horizontal stepper
- make completed/current stepper entries clickable links
- reorder steps so Server URL precedes LLMs
- use env-var placeholders (ANTHROPIC_API_KEY, etc.) with
  per-provider "Where do I get this?" disclosures
- replace the readonly "Validated username" input with a success pill
- drop the GitHub App name field (GitHub confirms the name anyway)
- re-label the GitHub App option and split review rows by strategy
- add a copy action to the Server URL on the review screen

Scope the dev token to PAT installs:
- only generate the dev token, write its files, and set FABRO_DEV_TOKEN
  inside the GithubInstallState::Token arm
- mark dev_token optional on InstallFinishResponse in the OpenAPI spec
- hide the Development token card on /install/finishing when absent
- add app_install_finish_omits_dev_token_and_does_not_write_it test

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
2026-04-19 16:11:02 -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
beda6b00d8
feat(events): add run.archived and run.unarchived event variants
Adds `RunArchived` and `RunUnarchived` events end-to-end through the engine.
Internal `Event` carries `actor` (and `restored_status` on unarchive); wire
`EventBody` serializes as `run.archived`/`run.unarchived` with typed props.
Projection gains `prior_status: Option<RunStatus>` — `RunArchived` captures
the current status before switching to Archived; `RunUnarchived` applies the
event's `restored_status` payload (authoritative) and clears `prior_status`.
2026-04-19 15:42:39 -04:00
Bryan Helmkamp
ed5e3f1792
feat(types): add archived run status with split terminal/immutable guards
Adds `RunStatus::Archived` variant and splits the overloaded `is_terminal()`
into `is_terminal()` (reached terminal outcome) and `is_immutable()` (cannot
transition outbound). `can_transition_to()` now allows Succeeded|Failed|Dead to
and from Archived, preserving the `* -> Dead` escape hatch. Downstream
exhaustive matches in the CLI and server are updated with conservative Archived
arms; the server's public-enum mapping and board-column placement carry TODOs
for the OpenAPI update in a later unit.
2026-04-19 15:34:39 -04:00
Bryan Helmkamp
9ddf6c06be
security(server): clamp pagination offset before iterator traversal
CodeQL's rust/uncontrolled-allocation-size alert flagged `paginate_items`
and the models list handler because `PaginationParams.offset: u32` was
cast to `usize` without an upper bound and handed to `Iterator::skip`.
In practice the underlying stores are bounded and `skip` on a Vec
iterator is O(1), so the existing callers couldn't be coerced into
allocating arbitrary memory, but an unbounded `offset` still takes an
unbounded time to walk past and CodeQL had no way to see that.

Clamp `offset` to `MAX_PAGE_OFFSET = 1_000_000` (beyond our largest
expected run count by several orders of magnitude) in both the shared
`paginate_items` helper and the models list handler that rolls its own
pagination. `limit` was already clamped to 100.

Closes code-scanning alert #27.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
2026-04-19 15:17:40 -04:00
Bryan Helmkamp
ec33c6a0ea
security(install): sanitize upstream URLs before issuing HTTP requests
CodeQL's Rust SSRF detector flagged the GitHub-and-provider HTTP calls in
install mode because the `base_url` values flow through `pub` test-only
setters (`with_github_api_base_url`, `with_provider_base_url`) that the
analyzer treats as external entry points. In production these values are
always the hardcoded `DEFAULT_*` constants, so the flagged paths are
unreachable, but the fix also hardens the real request sites.

Route every upstream URL through `parse_install_upstream_url`, which
- parses the URL,
- requires the scheme to be `http` or `https`, and
- requires a host.

Build request endpoints via `install_upstream_endpoint(base, &[segments])`
so each segment is percent-encoded by `url`; a caller cannot inject
extra path components, host overrides, or scheme changes via a path
segment. GitHub's manifest `code` (from the browser callback) is also
checked against the short base64url character set it uses.

Closes code-scanning alerts #28 and #29.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
2026-04-19 15:14:29 -04:00
Bryan Helmkamp
2505cb6d46
Merge remote-tracking branch 'origin/main' into feat/web-install-wizard
# Conflicts:
#	lib/crates/fabro-spa/assets/assets/entry-ez8gc920.js
#	lib/crates/fabro-spa/assets/index.html
#	lib/packages/fabro-api-client/src/.openapi-generator/FILES
#	lib/packages/fabro-api-client/src/models/index.ts
2026-04-19 15:06:12 -04:00
Bryan Helmkamp
d463bb276b
chore(spa): refresh embedded install-wizard bundle
Rebuild the bundled SPA via scripts/refresh-fabro-spa.sh so the Rust server
embeds the current install-wizard sources (OpenAI-compatible removed,
GitHub error banner consolidated into a single effect).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
2026-04-19 15:03:08 -04:00
Bryan Helmkamp
b8af65a9c6
refactor(runs): blocked status canonicalization cleanup (#165)
## Summary

Stacked cleanup of the `canonicalize blocked run status` work (local
commit `d13cdf374`) plus reconciliation with origin's `canonicalize
paginated run list responses` (origin commit `8ab689da7`). Both efforts
ran in parallel and diverged on the column name (`blocked` vs `waiting`)
and on how the board response is shaped — this PR converges them,
keeping `blocked` as the canonical column id while adopting origin's
`column` field on `RunListItem` and `StoreRunSummary` shape.

Also fixes a production-worker regression introduced by the
canonicalization: the worker's start-precondition only accepted
`Submitted | Starting`, so once runs started transitioning through
`Queued` on the way to `Starting`, every subprocess-worker run failed
with `Precondition failed: cannot start run: status is Queued`. That
cascaded into ~90 failing CLI/server integration tests locally.

## Commits

1. `f65843168` refactor(runs): simplify blocked status follow-ups
2. `1492d956c` chore: resolve clippy warnings
3. `676fd9f44` first merge of origin/main
4. `23fc92a2f` **fix(runs): allow Queued status in start precondition**
← the cascade-fix
5. `36b507a83` refactor: simplify pause/unpause + dedupe web status
tables
6. `8d8d27748` refactor(workflow): encapsulate BlockedStateTracker
inside HumanHandler
7. `1c17fda35` second merge of origin/main — resolves waiting vs blocked
8. `4cd3ef7b1` refactor(workflow): Mutex<usize> → AtomicUsize
9. `2e5a58e8a` fix(demo): align run-4 lifecycle status with Blocked
board column

## Test plan

- [x] fmt, clippy, build, doctests all clean
- [x] `cargo nextest run --workspace` — **4092/4092 pass**
- [x] `bun test` — **26/26 pass**, typecheck + production build clean
- [x] Manual CLI repro of the Queued-precondition fix
- [x] Browser smoke test: all 5 columns render with correct
labels/colors, demo run-4 appears in Blocked lane with question text
intact

## Known follow-up (not blocking)

A "paused-while-blocked" run (status `Paused` + `blocked_reason: Some`)
lands in the `running` column because the visible status chooses
`Paused` over `Blocked`. The pending question is not prominent on the
board. Addressing it would require `board_column()` to branch on
`(status, blocked_reason)` rather than just `status` — worth a separate
ticket.

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

---------

Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
2026-04-19 14:53:46 -04:00
Bryan Helmkamp
87dc7de140
fix(install): clear carried clippy warnings across install code paths
The web-install feature was carrying nine pedantic-tier clippy errors
from its initial commit. Fix them in place:

- \`install.rs\` \`InstallAppState\` switches \`install_token\`,
  \`storage_dir\`, and \`config_path\` from \`Arc<String>/Arc<PathBuf>\` to
  \`Arc<str>/Arc<Path>\` so we stop heap-duplicating buffers.
- Bring \`Infallible\`, \`axum::middleware\`, \`axum::extract::Request\`,
  and \`fabro_types::settings::SettingsLayer\` into scope instead of
  using absolute paths inline.
- Replace \`Duration::from_secs(10 * 60)\` with \`Duration::from_mins(10)\`.
- \`generate_ephemeral_secret\` never returns \`Err\`; drop the \`Result\`.
- \`server/start.rs ensure_storage_server_autostart_allowed\` takes
  \`Option<&OsStr>\` instead of consuming an \`OsString\` it only reads.
- \`server/mod.rs\` storage_dir fallback uses \`map_or_else\` to satisfy
  \`map_unwrap_or\`.

CI now passes \`cargo +nightly-2026-04-14 clippy --workspace
--all-targets -- -D warnings\` cleanly and the 892-test suite still
passes.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
2026-04-19 14:13:10 -04:00
Bryan Helmkamp
0e1d66b137
fix(install): hoist install-shell OnceLock to silence clippy
\`items_after_statements\` flagged the static declaration. Move it to
the top of \`cached_install_mode_shell\` — same behavior, same caching
semantics, one less lint to carry forward.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
2026-04-19 14:02:27 -04:00
Bryan Helmkamp
9bbd7099fe
chore(fmt): apply nightly rustfmt to server.rs
CI runs nightly rustfmt and flags this untouched for-loop header.
Pre-existing on the branch; clearing it here so the install-wizard
cleanup commits pass fmt --check cleanly.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
2026-04-19 13:59:30 -04:00
Bryan Helmkamp
2e7a6c95ea
refactor(install): use generated @qltysh/fabro-api-client types
Run \`bun run generate\` inside lib/packages/fabro-api-client to pick up
the new install schemas. Swap install-api.ts from hand-written
interfaces to re-exports from @qltysh/fabro-api-client and drop the
last duplicated type surface for the install wizard.

Keeps the \`installFetch\` wrapper and \`readInstallError\` helper so the
session-storage token handling and our custom error parser stay local
to the wizard. The generated Axios client is available as a future
migration if we decide to drop the wrapper.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
2026-04-19 13:55:58 -04:00
Bryan Helmkamp
c6ff36d9fb
refactor(install): consolidate shared primitives in fabro-install crate
The `fabro-install` crate was introduced for the web wizard but the CLI
kept its own copies of the same JWT keypair generation, TOML merging,
and GitHub auth settings helpers. Delete the duplicates and route the
CLI through `fabro_install::*`. The CLI keeps a thin
`merge_server_settings` wrapper because it only ever binds TCP and
derives the authority from `--web-url`.

Also tighten `persist_install_outputs_direct` to take its
`PendingSettingsWrite` argument by reference (satisfies
`needless_pass_by_value`) and pull the remaining absolute paths in the
crate's test module into `use` statements, clearing the nightly clippy
warnings that this branch was carrying.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
2026-04-19 13:54:47 -04:00
Bryan Helmkamp
acb6b3f9d6
refactor(install): tag GithubAppOwner with discriminated object shape
The install GitHub App manifest shape encoded owner as `"personal"` or
`"org:<slug>"` - a magic string parsed in install-app.tsx, built by
install-api.ts, and reparsed server-side. Replace with a tagged object
`{ kind: "personal" } | { kind: "org", slug }` in the OpenAPI spec, the
progenitor-generated Rust types, and the frontend.

Server-side, the internal `GitHubAppOwner` enum keeps its semantic
shape but gains a `TryFrom<GithubAppOwnerInput>` conversion and emits
the tagged JSON via `as_session_value`.

Frontend drops `buildGithubOwnerValue` in favor of
`buildInstallGithubAppOwner`, and the ready-screen renders the owner
through a small helper instead of string concatenation.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
2026-04-19 13:48:31 -04:00
Bryan Helmkamp
ad7fdc8d13
refactor(install): return spec-conformant ApiError shape
Install handlers returned `{"error": "..."}` while the OpenAPI paths
referenced the repo-wide `ErrorResponse` schema
(`{"errors":[{status,title,detail}]}`). Funnel the install helper through
`ApiError::into_response`, switch the invalid-token 401 and the
persistence-failure INTERNAL_SERVER_ERROR to the same shape, and update
the TS `readInstallError` helper + test fixtures to read
`body.errors[0].detail`.

The install-finish failure path still carries `leftover_env_keys`
alongside the error envelope so the rollback integration tests retain
their diagnostic field.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
2026-04-19 13:43:57 -04:00