StubEnv was a thin newtype only used by tests but compiled into every
build. Implementing EnvSource directly on HashMap<String, String> lets
test sites pass a HashMap and removes the type entirely.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
`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>
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>
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>
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>
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.
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>
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>
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.
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.
- 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>
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.
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.
Move GitHub webhook intake onto the main API router, add explicit
server_url and tailscale_funnel strategies, and validate strategy
requirements at config resolution. This also updates the API contract,
generated client, and operator docs to match the new webhook model.
Enable clippy::allow_attributes_without_reason at the workspace level.
Add concise, callsite-specific reasons to existing allow attributes, including generated code paths.
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>
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.
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.
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).
Adds lib/crates/fabro-server/tests/it/api/run_files.rs covering the
HTTP-level plumbing branches of GET /api/v1/runs/{id}/files:
- Invalid run_id path returns 400
- Unknown run returns 404 (IDOR-safe; same status as missing-run case)
- Malformed from_sha / to_sha query params return 400 before any work
- Non-default from_sha value returns 400 even when hex-well-formed
(v1 reserves the parameter for a future version)
- Submitted run with no sandbox record returns empty envelope
- Demo mode (X-Fabro-Demo: 1) returns the 3-entry fixture without
touching the run store, with at least one populated-content entry
- Response envelope shape matches PaginatedRunFileList contract:
data: FileDiff[], meta: { truncated, total_changed, ... } with
correct field types
Sandbox-path happy case (live diff) and degraded-fallback scenarios
stay covered by unit tests on stitch_file_diff, build_fallback_response,
and the sandbox_git helpers, since integration-level scheduler setup
for terminal-run tests is flaky without broader harness scaffolding.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
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>
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>
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>
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>
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>
Harden the remaining install flow regressions and add the missing
coverage for startup dispatch, finish-time shutdown behavior, and
partial-state persistence after vault failures.
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>
Restrict the browser install flow to Anthropic, OpenAI, and Gemini,
remove the unused install-time base URL surface, and reject
openai_compatible with a stable 422 response.
Also fix the finishing health poller so it only redirects after the
server comes back healthy outside install mode instead of jumping early
on transient restart failures.
