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.
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>
- Drop dead http_client() accessor and its allow(dead_code).
- Scope map_api_error to module-private; all call sites are in-file.
- Rename test helper test_api_client to test_client to match what it returns.
- Run system df's two independent server GETs concurrently with try_join!.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Wrap the remaining CLI server API calls in server_client::Client,
remove the api/connect_api_client escape hatches, and migrate
model/install/tests to the new facade.
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.
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.
The CLI façade is the primary type callers reach for, so it deserves
the bare `Client` name (per `reqwest::Client`, `hyper::Client`
convention). The raw generated HTTP binding is secondary and is more
accurately named `ApiClient`. "Store" in `ServerStoreClient` was
leftover from the SlateDB-ownership refactor and no longer describes
the type.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Retry CLI API calls once after a 401 access_token_expired response by
refreshing the stored OAuth session and rebuilding the generated API
client. Clear local auth when the refresh chain is expired or revoked,
and add end-to-end CLI scenarios covering login, authenticated use,
refresh, and logout.
Add the server-side CLI OAuth endpoints and token persistence needed to
mint JWT access tokens and rotating refresh tokens from the existing
GitHub web auth flow.
Add CLI auth storage plus `fabro auth login`, `logout`, and `status`, and
prefer stored OAuth access tokens when building target clients.
Enable clippy::allow_attributes_without_reason at the workspace level.
Add concise, callsite-specific reasons to existing allow attributes, including generated code paths.
Resolve rm/archive/unarchive selectors through the server-owned
runs/resolve endpoint instead of CLI-side summary matching, and move
active-run delete force semantics into DELETE /runs/{id}.
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>
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.
Integrates 39 commits from origin/main (archive/unarchive feature, UI
unification, theme/light-mode polish, Settings nav promotion, server
and CLI hardening).
Conflict resolutions:
- apps/fabro-web/app/routes/run-detail.tsx: origin removed the
`broken` field from the tab config; local added the Files Changed
tab. Kept the Files Changed tab, dropped the broken field per
origin's shape.
- lib/crates/fabro-store/src/run_state.rs: both sides added tests
in the same region. Kept local's two final_patch tests and all
four of origin's archive/unarchive tests.
- lib/crates/fabro-spa/assets/: embedded SPA bundle rebuilt from
the merged web source.
- lib/crates/fabro-workflow/src/operations/archive.rs: origin's new
archive tests construct Event::WorkflowRunFailed{..}; added the
final_patch: None field that local's lifecycle change introduced.
Workspace verification after merge: 4247 Rust tests + 95 web tests
all pass; clippy clean; fmt clean.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
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>
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.
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.
- 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.
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>
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.
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>
Rewrites apps/fabro-web/app/routes/run-files.tsx to consume the real
PaginatedRunFileList response and removes the fallbackFiles fixture
and the Steer subsystem. The new component:
- Loads via apiJsonOrNull, so a 404/501 (dev without the route)
renders the empty state instead of the root error boundary
- Branches on meta.degraded + meta.patch to render PatchDiff with a
DegradedBanner whose copy reflects degraded_reason
- Renders per-entry placeholders for sensitive, binary, symlink/
submodule, and truncated files with the priority order
sensitive > binary > symlink/submodule > truncated -- security
flags never get hidden behind a lesser placeholder
- Renders one MultiFileDiff per regular entry
- Uses role="region" + aria-label on each file row
Also unhides the Files Changed tab in run-detail.tsx by flipping
broken: true -> false. Adds missing final_patch: None to the runner
RunFailed test fixtures to match the lifecycle change from Unit 2.
Refs plan docs/plans/2026-04-19-002-feat-run-files-changed-tab-plan.md
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
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>
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.
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.
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.
- 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>
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.
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>
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.
## 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>
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>
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>
Tighten the browser-based install flow after correctness and adversarial
review, without changing the external wizard shape.
- Persist the actual bind in server.listen, not the canonical URL
- Reject concurrent /install/finish and rapid GitHub App retries
- Keep the prior GitHub Token strategy until App callback succeeds
- Recover from poisoned install locks instead of propagating panics
- Rollback both settings and vault on failed persistence
- Redirect GitHub callback errors back into the wizard UI
- Validate LLM keys via /models probe instead of a billed generate()
- Reject canonical URLs with trailing slash, path, query, or fragment
- Accept any valid install-token source, not just the first present one
- Redact the install token in structured logs
- Assert install-mode SPA marker injection at startup
- Warn on suspected concurrent operators via UA + X-Forwarded-For
- Add component-level test for the GitHub callback error banner
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Implement the web-first install experience across the server, CLI, API spec,
web app, and packaged SPA assets.
This also removes test-side process env mutation by pushing env-dependent
decision points behind explicit helpers and test wiring.
Unify /api/v1/runs and /api/v1/boards/runs around a shared
paginated summary contract with additive convenience fields.
Update the server, demo data, generated clients, CLI pagination,
and web consumers so board views become a thin projection over the
canonical run summary surface.
Non-release builds now append the profile to `fabro --version`
(`x.y (sha date debug)`), `fabro version`, and `fabro system info`,
so users can tell a local build apart from a shipped release. The
API's `SystemInfoResponse` gains a `profile` field so the client
can render the server's build profile too.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
