Commit graph

2419 commits

Author SHA1 Message Date
Bryan Helmkamp
83a21e5d37
test(server): Run Files HTTP integration suite (P2-9)
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>
2026-04-19 17:40:14 -04:00
Bryan Helmkamp
fd9596883a
test(server): tracing allowlist + coalescer cancellation (P2-11, P2-13)
P2-11: Assert RunFilesMetrics::emit writes ONLY the allowlisted field
set (run_id, file_count, bytes_total, duration_ms, truncated,
binary_count, sensitive_count, symlink_count, submodule_count, message).
Uses a tracing-subscriber Layer with a Visit impl that captures every
field name emitted under the run_files target; fails the test if any
non-allowlisted field appears. Catches future refactors that might add
paths/contents to the log line.

P2-13: Assert that when the first coalesce caller is cancelled mid-
materialization, the spawned task continues to completion and a
subsequent caller still receives the shared result. Proves the
tokio::spawn-based design survives request dropout without
re-materializing the diff.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
2026-04-19 17:37:44 -04:00
Bryan Helmkamp
1ef72fb6bc
refactor(server): extract run_files_security with globset denylist (P3-2)
Moves the sensitive-path denylist, sandbox-git env helper, and metrics
emitter into a dedicated run_files_security module so the Run Files
Changed endpoint has a single, testable surface for security controls.

Denylist upgrades to globset::GlobSet with two explicit lists:
- Basename globs: .env, .env.*, *.pem, id_rsa, id_rsa.*, id_ed25519*,
  *.p12, *.keystore, *.key
- Path-suffix globs: .aws/credentials, .git/config, .ssh/**

Matching semantics explicitly pinned:
- Case-insensitive via lowercased normalization
- Path traversal (`../`, `./`, leading `/`) stripped before match
- Basename globs match the final segment only — prevents
  `log/.env_audit/data.txt` from matching `.env.*`
- Empty/pathological paths fail closed (sensitive=true safe default)

Also ships:
- sandbox_git_env() returning the env-hardening map
- RunFilesMetrics struct + emit() so tracing never leaks paths/contents

Handler migrates to consume the new module; inline denylist and inline
info!() call removed.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
2026-04-19 17:35:26 -04:00
Bryan Helmkamp
f0ee17b5b8
fix(server): capture to_sha_committed_at (P2-1)
Sandbox path: issue a best-effort `git show -s --format=%cI <to_sha>`
against the reconnected sandbox to resolve the commit time of HEAD,
parsed into chrono::DateTime<Utc>. Failures (command error, non-zero
exit, unparseable output) return None so the handler still succeeds;
the client simply won't show a "Checkpoint Xm ago" label.

Degraded path: populate meta.to_sha_committed_at from
projection.conclusion.timestamp (the run-end time). The patch was
captured then, so it's a reasonable proxy for "captured X ago" in the
UI.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
2026-04-19 17:31:04 -04:00
Bryan Helmkamp
611772535f
fix(server): P1 correctness fixes for Run Files handler
Three P1 bugs from code review:

P1-1: Modified and renamed files now return real before/after contents.
Previously the handler fetched only each entry's new_blob and duplicated
that single blob onto both sides, so every modified file rendered as a
no-op diff in MultiFileDiff. The fetch path now collects both old_blob
and new_blob, deduplicated, into a single batched `cat-file --batch`
call and stitches contents back via a SHA->contents table. Added
regression tests for modify and rename.

P1-2: Degraded-patch denylist now matches both `a/<old>` and `b/<new>`
sides of each `diff --git` header. A sensitive file renamed to a benign
path was leaking its patch body through the fallback branch. Added
regression test with `.env.production -> docs/NOTES.md`.

P1-3: Sensitive classification now runs BEFORE the 200-file cap, per
the plan's R31-before-R27 ordering. Sensitive entries no longer evict
real changes when the cap is hit. Replaced the (fetch, prebuilt) Vec
pair with a single ClassifiedEntry-ordered list so response ordering
matches git diff --raw output.

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>
2026-04-19 17:29:02 -04:00
Bryan Helmkamp
46813e3b05
docs(plan): mark Run Files tab plan completed
All 13 units shipped. Follow-ups noted inline: globset-based denylist
extraction and Virtualizer wrapping for very large runs.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
2026-04-19 17:13:40 -04:00
Bryan Helmkamp
6050b5fa4b
feat(web): Run Files tab empty/error/loading states, refresh, a11y
Completes Units 11 and 12 of the Run Files Changed plan:

- Empty-state taxonomy: distinct copy for total_changed==0 vs no
  recoverable diff
- LoadingSkeleton on initial loader navigation (shimmer respects
  prefers-reduced-motion via motion-safe:animate-pulse)
- ErrorBoundary export handling 401/403/503/429 and generic 5xx
- Refresh button + Toolbar with freshness indicator; relative
  timestamps tick every 10s
- SSE subscription to /runs/{id}/attach with a 500ms debounce that
  revalidates on checkpoint.completed, run.completed, run.failed
- After a revalidation completes, focus returns to the Refresh button
- j/k keyboard navigation over file rows, ignoring key presses while
  a text field is focused
- md (768 px) breakpoint collapses split to unified without writing
  any persisted preference
- #file=<encoded-path> deep link scrolls + focuses the matching row
  on mount; absent file surfaces a 5s toast; patch-only mode shows
  a toast explaining the limitation
- Touch targets on the Refresh button meet WCAG 2.5.5 AAA (44x44)

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>
2026-04-19 17:13:17 -04:00
Bryan Helmkamp
b589b581ff
docs(plan): mark Run Files tab plan partially completed
Captures what landed in this session (Units 1-10, 13) vs what's
deferred (Units 11-12) so follow-up work can pick up from a clean
baseline.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
2026-04-19 17:06:44 -04:00
Bryan Helmkamp
731a8cc6e0
feat(web): rewrite Run Files tab, surface it in navigation
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>
2026-04-19 17:06:06 -04:00
Bryan Helmkamp
420e375134
build(web): upgrade @pierre/diffs to 1.1.15
Bumps @pierre/diffs from 1.0.11 to 1.1.15 to pick up the Virtualizer
component and renderHeaderPrefix/renderCustomHeader hooks the Run
Files tab relies on for large-diff performance. 1.0 -> 1.1 merged
MouseEventManager/LineSelectionManager into InteractionManager but
the public React components (MultiFileDiff, PatchDiff, FileDiff,
File) keep their existing shape, so no consumer changes are needed
yet -- Unit 10 exercises the new features.

Pins an exact version (1.1.15) rather than a caret range so bun
doesn't resolve up to 1.1.16, which was published today and would
trip the "no packages younger than 24 h" rule in the user-global
policy.

The redundant apps/fabro-web/bun.lock is removed; bun workspaces
resolve against the root bun.lock and the per-app lockfile was
drifting from it. Embedded SPA bundle (lib/crates/fabro-spa/assets/)
is refreshed to match the new build output.

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>
2026-04-19 17:00:50 -04:00
Bryan Helmkamp
86f0140ee0
docs(logging): add prohibited-fields section
Extends the Fabro logging strategy with an explicit prohibited-fields
table covering the Run Files Changed endpoint's sensitive surface:
diff_contents, per-changed-file file_path values, raw git_stderr,
and credential-ish strings. Each entry pairs the prohibition with a
concrete cardinality-bounded alternative, so future handlers have a
precedent to follow rather than rediscovering the rule.

The Run Files handler (Unit 5) already emits exactly the allowlisted
field set (run_id, file_count, bytes_total, duration_ms, truncated,
binary_count, sensitive_count, symlink_count, submodule_count); this
change makes the policy enforceable for other endpoints.

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>
2026-04-19 16:58:20 -04:00
Bryan Helmkamp
c78c1fd17b
feat(server): demo-mode stub for /runs/{id}/files
Replaces the not_implemented placeholder in the demo router with a
demo::list_run_files_stub that returns a small illustrative
three-file diff (modified, added, renamed) matching the real handler's
PaginatedRunFileList wire shape. The stub ignores run_id and state so
demo mode and real mode cannot cross-contaminate (R34).

Unit 10 (frontend rendering paths) will remove the now-obsolete
client-side fallbackFiles fixture when it rewrites run-files.tsx.

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>
2026-04-19 16:57:25 -04:00
Bryan Helmkamp
04c14f06d0
feat(server): degraded final_patch fallback for Run Files
Extends the Run Files handler with the patch-only fallback branch.
When the sandbox is unreachable (reconnect failed, provider not
compiled in, or the base revision has been garbage-collected), the
response now:

- Reads RunProjection.final_patch (captured at run end by Unit 2
  for both Success/PartialSuccess and now Failed runs)
- Caps the patch at 5 MiB on a UTF-8 char boundary
- Filters denylisted file sections out via a regex-level `diff --git`
  header scan (no full patch parser; the placeholder line kept so
  clients still render the surrounding context)
- Picks the right degraded_reason: provider_unsupported for Docker-
  provider runs this build can't reconnect to, sandbox_gone for
  terminal runs, sandbox_unreachable for still-running ones
- Populates meta.to_sha from conclusion.final_git_commit_sha and
  meta.total_changed from a `diff --git` header count

When final_patch is absent (old Failed runs, projection write
failures), returns the empty envelope that the UI maps to R4(c).

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>
2026-04-19 16:55:58 -04:00
Bryan Helmkamp
62126f07a9
feat(server): real GET /runs/{id}/files handler (sandbox path)
Implements the sandbox branch of the Run Files Changed endpoint. When
a run has a reachable sandbox, the handler:

- Parses the run_id and authenticates via AuthenticatedService
- Rejects any non-default from_sha/to_sha (v1 reserves them)
- Validates SHA format with a 7-40 hex regex before use
- Returns 404 for both missing-run and unauthorized access so
  run-ID enumeration is not possible (IDOR-safe)
- Reconnects to the sandbox via a new try_reconnect_run_sandbox that
  returns Ok(None) for the reconnect-failed case (Unit 6 will insert
  the final_patch fallback there instead of today's empty envelope)
- Enumerates changes via list_changed_files_raw + list_binary_paths,
  batched blob fetching via stream_blob_metadata / stream_blobs
- Applies an inline sensitive-path denylist first (Unit 8 extracts),
  then a 200-file count cap, per-file 256 KiB cap, and 5 MiB
  aggregate cap - truncated entries carry an explicit
  truncation_reason
- Builds a single tracing::info! span at response end with only the
  allowlisted fields (run_id, file_count, bytes_total, duration_ms,
  truncated, binary_count, sensitive_count, symlink_count,
  submodule_count) -- no paths, contents, or git stderr

All calls go through the Unit 4 coalescing primitive, so concurrent
viewers of the same run share one materialization.

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>
2026-04-19 16:52:33 -04:00
Bryan Helmkamp
e5370c1d18
feat(server): per-run request coalescing primitive
Adds the concurrency primitive the upcoming GET /runs/{id}/files
handler needs so concurrent viewers of the same run share one
sandbox-git materialization (different runs still materialize in
parallel).

Design notes:
- Materialization runs on a detached tokio::spawn so an abandoned
  caller cannot leave orphan git subprocesses in the sandbox
- tokio::sync::watch is used (not broadcast) so late subscribers that
  arrive after the value is sent still see it via the cached `borrow`
- AssertUnwindSafe().catch_unwind() turns materializer panics into
  500 ApiErrors for every concurrent caller; a subsequent request on
  the same run_id then triggers a fresh materialization (no poisoning)
- ApiError::Clone is derived so the shared Arc<Result<T, ApiError>>
  can fan out cheap copies

The FilesInFlight registry is now a field on AppState; Unit 5 will
consume it from the real handler.

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>
2026-04-19 16:40:39 -04:00
Bryan Helmkamp
8f7afd4bfc
feat(workflow): machine-readable sandbox git helpers for Run Files
Adds sandbox-side helpers the upcoming GET /runs/{id}/files handler
needs to produce structured diff entries without a full unified patch:

- list_changed_files_raw: git diff --raw -z --find-renames=50%,
  returns RawDiffEntry variants (Added/Modified/Deleted/Renamed/
  Symlink/Submodule) with SHA-addressed blob references; paths are
  metadata only and never re-interpolated into shell
- list_binary_paths: git diff --numstat text/binary classifier so
  binary blobs are never piped through cat-file
- stream_blob_metadata / stream_blobs: batched git cat-file
  --batch-check / --batch driven by printf into stdin, avoiding
  per-file RPC storms for 200-file runs
- DiffError discriminates Transient (timeout, process kill) from
  Permanent (bad/invalid revision, unknown object) so the server can
  surface 503 vs fall through to the patch-only fallback

All new invocations use a hardened git prefix (core.hooksPath=/dev/null,
protocol.file.allow=never, core.fsmonitor=false) plus a small env
hardening map (GIT_TERMINAL_PROMPT=0, GIT_EXTERNAL_DIFF cleared) and a
10 s timeout per R32.

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>
2026-04-19 16:35:20 -04:00
Bryan Helmkamp
296eca568b
feat(workflow): capture final_patch on RunFailed
Previously only Success/PartialSuccess outcomes captured the final
unified-patch string into the run projection. Failed runs left
RunProjection.final_patch empty, which meant the upcoming Files
Changed tab could not degrade to a patch-only view once the sandbox
was gone.

Extend on_run_end to run git diff on Failed too, with a tighter 10 s
timeout (vs 30 s on success) so a pathological workspace doesn't
stall downstream terminal notifications (Slack, SSE, CI). Plumb the
optional field through Event::WorkflowRunFailed, RunFailedProps, and
the projection.

Back-compat: final_patch is serde default-None, so pre-change events
in SlateDB replay cleanly as None. No backfill required; old Failed
runs show R4(c) empty state on the Files tab.

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>
2026-04-19 15:39:17 -04:00
Bryan Helmkamp
f65c7c3fd8
feat(api): add GET /runs/{id}/files spec + RunFilesMeta schema
Reintroduces the endpoint deleted in the April 5 server-only cleanup,
this time targeted at the web UI (not the CLI). Route registered with
not_implemented; real handler lands in Unit 5.

- FileDiff gains optional change_kind, truncated, truncation_reason,
  binary, sensitive fields (all additive, back-compat)
- New RunFilesMeta replaces PaginationMeta on PaginatedRunFileList
  (truncated, total_changed, to_sha, to_sha_committed_at, degraded,
  degraded_reason, patch, files_omitted_by_budget)
- from_sha / to_sha query params reserved for future use (non-default
  values 400 in v1)

Generated TS client picks up the new model; typecheck + openapi
conformance tests pass. No existing consumers of
PaginatedRunFileList['meta'] found in the monorepo.

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>
2026-04-19 15:33:24 -04:00
Bryan Helmkamp
b849738a5b
plans 2026-04-19 15:18:25 -04:00
Bryan Helmkamp
5958470a11
plan 2026-04-19 15:18:22 -04:00
Bryan Helmkamp
a1c816724b
plans 2026-04-19 15:18:21 -04:00
Bryan Helmkamp
5543e276a0
Merge pull request #166 from fabro-sh/feat/web-install-wizard
feat(install): browser-based install wizard
2026-04-19 15:18:08 -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
908f078cec
chore(docker): add --tag to docker-build.sh, document it in AGENTS.md
Lets a smoke-test harness pick its own image tag without racing the default
fabro:latest, and points future agent sessions at bin/dev/docker-build.sh
so they don't hand-roll a throwaway Dockerfile.

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
20e4ef32ae
refactor(install): collapse overlapping React state into discriminated unions
Two wins here. First, the `session`/`loadingSession`/`sessionError`
triple is replaced with a single `SessionState` discriminated union, so
the component can switch on `.status` instead of juggling three
correlated flags. Second, the seven flat `useState` calls for the
GitHub step are grouped into `githubStrategy` + `tokenForm` + `appForm`,
with `appForm.owner` typed as the generated `InstallGithubAppOwner`
tagged object. Invalid states like "token flow but org slug set" simply
stop existing.

\`buildInstallGithubAppOwner\` is deleted (unused) — form handlers build
the tagged object in place, which is small enough to stay readable.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
2026-04-19 13:59:09 -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
Bryan Helmkamp
985373cad4
chore(lockfile): sync merged workspace versions 2026-04-19 13:37:10 -04:00
Bryan Helmkamp
dd4e467bfc
Merge remote-tracking branch 'origin/main'
# Conflicts:
#	lib/crates/fabro-server/tests/it/api/mod.rs
2026-04-19 13:36:54 -04:00
Bryan Helmkamp
3f21644d80
fix(install): cover follow-up edge cases
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.
2026-04-19 13:32:46 -04:00
Bryan Helmkamp
e8d0f75be9
Merge remote-tracking branch 'origin/main' 2026-04-19 12:46:19 -04:00
Bryan Helmkamp
75f8ed845b
fix(install): harden web wizard against review findings
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>
2026-04-19 12:43:00 -04:00
Bryan Helmkamp
1e6543528f
fix(server): satisfy workspace clippy 2026-04-19 12:02:19 -04:00
Bryan Helmkamp
e9daf2db3a
Merge remote-tracking branch 'origin/main' 2026-04-19 11:48:15 -04:00
Bryan Helmkamp
a086a694f1
Merge remote-tracking branch 'origin/main' 2026-04-19 11:48:02 -04:00
Bryan Helmkamp
9dd792c8b8
fix(install): exclude openai-compatible from v1 setup
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.
2026-04-19 11:43:38 -04:00
Bryan Helmkamp
ba3e760313
fix(runs): use generated demo status reason parser 2026-04-19 11:38:52 -04:00
Bryan Helmkamp
b5bb134890
fix(runs): bound board enrichment and demo normalization
Paginate board-eligible summaries before enriching them from run state,
add safety caps to paginated web fetches, and make demo run summaries
follow the production title and status-reason normalization rules.
2026-04-19 11:35:30 -04:00
Bryan Helmkamp
ecdfdd82d8
feat(install): add browser-based setup flow
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.
2026-04-19 11:20:58 -04:00
Bryan Helmkamp
ec239aaf9c
fix(cli): update install test for listener tls removal 2026-04-19 11:18:34 -04:00
Bryan Helmkamp
6226858648
fix(runs): finish canonical run summary rollout
Complete the /runs and /boards/runs canonicalization work by fixing the
run-detail response shape, preserving lifecycle status separately from board
columns, loading all board pages in the web client, and aligning the shared
status_reason typing.
2026-04-19 11:12:58 -04:00