Persist dev-token credentials in auth.json alongside OAuth entries so CLI targets resolve credentials consistently across TCP and Unix socket flows.
Move install-time token minting to runtime storage, add auth login --dev-token, and refresh the embedded SPA after updating the stale dev-token hint.
Move secret redaction and DisplaySafeUrl into fabro-redact so credential handling has a narrow ownership boundary. Update direct consumers and docs to depend on fabro_redact instead of fabro_util::redact.
Add DisplaySafeUrl under fabro-util::redact so URL Display and Debug output redact credentials by default. Migrate token-bearing GitHub, OAuth, server, LLM, sandbox, and workflow paths to use the wrapper at logging/error boundaries while keeping raw URLs explicit for wire and shell transit.
Color the install URL and add a separate block that prints the install
token on its own line so users can copy it without parsing the query
string. The token block shows in both the URL and reverse-proxy branches.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Add fabro-static::EnvVars as the shared registry for fixed environment variable names and migrate env reads, clap env bindings, and subprocess/test allowlists to use it.
Add clippy bans for raw std::env lookup APIs so future dynamic env facades must be documented explicitly.
The AWS access key ID is not a secret — swap its PasswordInput for a
regular text input so operators can read and edit it directly.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Add a Supported platforms table to the quick-start so Intel Mac and
Windows users learn they're unsupported before running the installer.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
New deep plan covering DisplaySafeUrl (fabro-redacted), EnvVars
registry (fabro-static), expanded snapshot helpers, miette CLI
diagnostics, fabro-dev unified CLI, and OptionsMetadata-driven docs
generation. Six phases, thirteen implementation units, ships as
independent PRs.
Includes deepening-pass revisions and reviewer feedback:
- Clippy enforcement reframed as workspace-wide bans with crate-level
#![allow] opt-outs (clippy.toml has no per-crate scoping).
- Phase 4 (miette) narrowed: fancy rendering + help: footer only; no
source-highlighting promise since no fabro error type carries spans.
main() retains its telemetry/shutdown lifecycle.
- Phase 6 split into 6.2 (cli.mdx) and 6.3 (user-configuration.mdx)
with fenced-region commitment upfront; resolves the previously
orphaned user-configuration drift problem.
- Phase 2 gains an explicit allow/deny list for DisplaySafeUrl use
and neutralizes the .to_string() trap via explicit .redacted_string()
/ .raw_string() methods plus a clippy deny on the implicit path.
install.rs:1227,1546 and fabro-cli/commands/server/mod.rs:266-270
are explicitly kept as raw String (Location headers, user-facing
install URLs).
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Rewind now creates a resumable replacement run from the selected checkpoint, archives the source run, and records run.superseded_by for auditability. Fork, rewind, and timeline listing now share server-backed git-store plumbing, with generated API clients and docs updated for the new contract.
Reuse the existing merge strategy type across CLI/API/GitHub paths, consolidate repeated PR command setup, and serialize server-side PR creation per run to avoid duplicate external work.
Two code-reuse findings from the simplify review:
1. PullRequestGithubContext carried owner/repo String fields obtained by
re-parsing record.html_url, even though PullRequestRecord already
carries typed non-optional owner/repo fields. Dropped the redundant
fields; the 3 PR handlers read via &ctx.record.owner /
&ctx.record.repo instead. The incidental non-github.com URL
rejection is preserved as an explicit one-line host-validation
call (documented by the rejects_non_github_record_url tests).
2. RunPrInputs held run_spec: &RunSpec purely to read goal()
downstream. Narrowed to goal: &str stored directly; the server
handler passes inputs.goal to OpenPullRequestRequest::from_run_state,
which no longer needs the full RunSpec. Fewer fields, clearer
dependency at the call site.
Also tightened the from_run_state doc comment (was narrating peer
callers' behavior rather than the method's contract).
Verified: workspace fmt clean, clippy --all-targets -D warnings clean,
cargo nextest run --workspace 4581 passed, 182 skipped.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Apply the five findings from the external review: reject archived sources
with 409 (was contradictory); emit RunSupersededBy only on archive success
(was self-contradicting with the ordering rationale); look up working_directory
from the run's RunSpec instead of hand-waving AppState.repo_path; plumb
superseded_by through RunSummary + OpenAPI to honor the 'helps fabro ps' claim;
reconcile test scenarios to the archive-first ordering.
Also add GET /runs/{id}/timeline to Unit 2 so --list display moves server-side
alongside the mutating rewind call (web-UI parity). Normalize all status
codes from 412 to 409 to match fabro-server's CONFLICT convention.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Record the five decisions from the targeted Unit 2 review: 207 Multi-Status
for archive-failure partial success, graceful-degradation mapping for TOCTOU
precondition races, archive-first event ordering, accept-orphan retry posture,
and a new superseded_by projection field. Also add spawn_blocking and
operations-layer composite guidance from the review's autofixes.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
MergeMethod derives serde(rename_all = "snake_case"), so json!({
"merge_method": method }) emits the same `"squash"` / `"merge"` /
`"rebase"` strings as the as_str() round-trip. Inlining the typed value
removes the only remaining manual string conversion in the merge path.
Verified: workspace fmt clean, clippy --all-targets -D warnings clean,
cargo nextest run --workspace 4581 passed (fabro-github merge_pr unit
tests still pass — they assert against status codes not payload bytes,
but the twin-mode integration test create_merge_and_verify_state
exercises the on-the-wire JSON shape).
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The 4 callers of create_completed_run_ready_for_pull_request all paired
it with pr_test_app(...) and used identical defaults for base_branch
("main"), run_branch ("fabro/run/42"), and diff. Only repo_origin_url
varied per test. Bundle into pr_test_app_with_completed_run(token,
github_base_url, repo_origin_url) -> (state, app, run_id); each call
site shrinks from 12 lines to 1 helper invocation.
Verified: workspace fmt clean, clippy --all-targets -D warnings clean,
cargo nextest run -p fabro-server 439 passed.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Add an async sibling helper that bundles state + app + a fresh
create_run(&app, MINIMAL_DOT) into one (state, app, run_id) tuple.
Updated the 2 PR tests that had built this triple manually
(merge/close not_found_when_record_missing). The third holdout at
line 10148 keeps its own setup — it has an intervening
assert_eq!(state.github_api_base_url, github.base_url()) that
documents a load-bearing invariant about app state construction.
Verified: workspace fmt clean, clippy --all-targets -D warnings clean,
cargo nextest run -p fabro-server 439 passed.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Public functions now take ctx: &GitHubContext<'_> instead of by-value
GitHubContext<'_>. Matches the surrounding &str / &GitHubCredentials
convention. The type stays Copy so internal call sites that pass `ctx`
through still work without explicit reborrows.
Touched: 8 fabro-github functions + matching _with_client variants,
plus call sites in fabro-server, fabro-workflow, fabro-sandbox, and
fabro-github's integration + unit tests. Pure mechanical change.
Verified: workspace fmt clean, clippy --all-targets -D warnings clean,
cargo nextest run --workspace 4581 passed, 182 skipped.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Session keeps llm_client: Client as its internal model — a session is
bounded (≤ 1 hour) and its cached client stays fresh within that
window. Session::new(client, ...) remains the primitive (used by the
server-mediated agent adapter path in fabro-cli/exec.rs, which builds
a Client with a custom ProviderAdapter, no source involved).
Add Session::from_source(source, ...) for callers that hold a source
directly — resolves a Client via Client::from_source and delegates to
new. Lets workflow-level callers that store Arc<dyn CredentialSource>
build a Session without hand-resolving first.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The "Anthropic uses x-api-key header, everyone else uses Bearer" logic
was written three times: env_source (env-based construction), resolve
(vault-based construction), and provider_auth (CLI key validation).
Any future header rename would need three edits.
Add ApiCredential::from_api_key(provider, key) as a canonical
constructor. Each callsite now builds via the helper and overrides only
the fields specific to its path (env base URLs, vault-sourced org/project
IDs, codex mode, etc.).
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
build_pr_body and maybe_open_pull_request now take the two things they
actually need — run_store: &RunStoreHandle and llm_source: &dyn
CredentialSource — instead of services: &RunServices. The workflow
PULL_REQUEST phase decomposes services at the callsite; the standalone
fabro pr create command passes its own directly.
This removes RunServices::for_cli, a stub constructor that fabricated
an emitter, sandbox, and provider just to satisfy the RunServices type
for two fields it cared about. The "leaky fake" is gone.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Add a pr_test_app(token, github_base_url) -> (state, app, run_id)
helper that bundles the create_github_token_app_state +
build_router(...) + fixtures::RUN_1 triple every PR-endpoint test
shared. Updated 15 call sites; the 3 tests that derive run_id from
create_run(&app, MINIMAL_DOT).await keep their own setup since they
need the app before the run_id exists.
Verified: workspace fmt clean, clippy --all-targets -D warnings clean,
cargo nextest run --workspace 4581 passed, 182 skipped.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
