diff --git a/AGENTS.md b/AGENTS.md index adc9b5d79..a02fc8e78 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -25,7 +25,7 @@ macOS note: if `cargo nextest run` fails with `Too many open files (os error 24) - `scripts/refresh-fabro-spa.sh` — **run this before committing any TypeScript change in `apps/fabro-web/` or `lib/packages/fabro-api-client/`**. It runs the production build and then copies `dist/` into `lib/crates/fabro-spa/assets/` (which is tracked in git). CI's TypeScript `Build` job reruns this script and then `git diff --exit-code -- lib/crates/fabro-spa/assets` — if the committed bundle drifts from source (e.g. content-hashed filenames like `entry-.js` change), the check fails. `bun run build` on its own is not enough. ### Docker image -- `bin/dev/docker-build.sh` — builds the local Docker image from the current tree using the release pipeline's cargo-zigbuild approach. Honors `--arch amd64|arm64`, `--tag ` (default `fabro`), and `--compile-only` (stages `docker-context//fabro` without `docker build`). Prefer this over writing a throwaway Dockerfile; the release pipeline, `Dockerfile`, and this script share the same binary layout. +- `cargo dev docker-build` — builds the local Docker image from the current tree using the release pipeline's cargo-zigbuild approach. Honors `--arch amd64|arm64`, `--tag ` (default `fabro`), `--compile-only` (stages `docker-context//fabro` without `docker build`), and `--dry-run` (prints the Docker commands without running them). Prefer this over writing a throwaway Dockerfile; the release pipeline, `Dockerfile`, and this command share the same binary layout. - Refresh the embedded SPA before rebuilding the image after any `apps/fabro-web` change: `scripts/refresh-fabro-spa.sh` runs the bun build and copies `dist/` into `lib/crates/fabro-spa/assets/`. Skipping this step produces a Docker image whose Rust binary embeds a stale SPA bundle. ### Marketing site (apps/marketing) diff --git a/bin/dev/docker-build.sh b/bin/dev/docker-build.sh deleted file mode 100755 index fee091486..000000000 --- a/bin/dev/docker-build.sh +++ /dev/null @@ -1,100 +0,0 @@ -#!/usr/bin/env bash -# Build a local fabro Docker image from the current working tree. -# -# The Docker image uses musl binaries for a small Alpine-based runtime. We -# compile fabro-cli for the target musl triple inside rust:1-bookworm using -# cargo-zigbuild (zig as the C compiler + linker). Cargo registry and target -# dir are cached in named volumes so subsequent runs are incremental. -# -# Usage: -# bin/dev/docker-build.sh # host arch, build image at end -# bin/dev/docker-build.sh --arch amd64 # force amd64 (uses QEMU on non-amd64 hosts — slow) -# bin/dev/docker-build.sh --tag fabro:smoke # tag image as fabro:smoke (default: fabro) -# bin/dev/docker-build.sh --arch arm64 --compile-only -# # stage binary only; skip `docker build` -# # (useful for multi-arch buildx verification) - -set -euo pipefail - -cd "$(dirname "$0")/../.." - -arch="" -compile_only=0 -tag="fabro" -while [ $# -gt 0 ]; do - case "$1" in - --arch) - arch="$2" - shift 2 - ;; - --tag) - tag="$2" - shift 2 - ;; - --compile-only) - compile_only=1 - shift - ;; - *) - echo "unknown argument: $1" >&2 - exit 1 - ;; - esac -done - -if [ -z "$arch" ]; then - case "$(uname -m)" in - x86_64|amd64) arch=amd64 ;; - aarch64|arm64) arch=arm64 ;; - *) echo "unsupported host arch: $(uname -m)" >&2; exit 1 ;; - esac -fi - -case "$arch" in - amd64) target=x86_64-unknown-linux-musl zig_arch=x86_64 ;; - arm64) target=aarch64-unknown-linux-musl zig_arch=aarch64 ;; - *) echo "unsupported arch: $arch (expected amd64 or arm64)" >&2; exit 1 ;; -esac - -ZIG_VERSION=0.13.0 - -echo "Building fabro-cli for $target inside rust:1-bookworm via cargo-zigbuild..." -docker run --rm --platform "linux/$arch" \ - -v "$PWD:/src" \ - -v fabro-docker-cargo-registry:/usr/local/cargo/registry \ - -v "fabro-docker-cargo-target-$arch:/target" \ - -v "fabro-docker-zig-$arch:/opt/zig" \ - -v "fabro-docker-cargo-tools-$arch:/opt/cargo-tools" \ - -w /src \ - -e CARGO_TARGET_DIR=/target \ - -e LIBZ_SYS_STATIC=1 \ - rust:1-bookworm \ - bash -c " - set -e - apt-get update -qq && apt-get install -y -qq pkg-config perl make cmake xz-utils curl >/dev/null - if [ ! -x /opt/zig/zig-linux-$zig_arch-$ZIG_VERSION/zig ]; then - curl -fsSL https://ziglang.org/download/$ZIG_VERSION/zig-linux-$zig_arch-$ZIG_VERSION.tar.xz | tar -xJ -C /opt/zig - fi - export PATH=/opt/cargo-tools/bin:/opt/zig/zig-linux-$zig_arch-$ZIG_VERSION:\$PATH - if ! command -v cargo-zigbuild >/dev/null; then - cargo install --locked --root /opt/cargo-tools cargo-zigbuild - fi - rustup target add $target - cargo zigbuild --release -p fabro-cli --target $target - " - -echo "Extracting binary from builder cache..." -mkdir -p "docker-context/$arch" -docker run --rm --platform "linux/$arch" \ - -v "fabro-docker-cargo-target-$arch:/target" \ - -v "$PWD/docker-context/$arch:/out" \ - rust:1-bookworm \ - cp "/target/$target/release/fabro" /out/fabro - -if [ "$compile_only" -eq 1 ]; then - echo "Staged docker-context/$arch/fabro (skipping docker build per --compile-only)." - exit 0 -fi - -echo "Building Docker image as $tag..." -docker build --platform "linux/$arch" -t "$tag" . diff --git a/docs-internal/updating-web-screenshots.md b/docs-internal/updating-web-screenshots.md index 8510ee66c..efdfb30bc 100644 --- a/docs-internal/updating-web-screenshots.md +++ b/docs-internal/updating-web-screenshots.md @@ -11,7 +11,7 @@ Screenshots of the Fabro web UI are embedded in the public docs. This guide cove - Docker installed - Chrome running with DevTools MCP or similar screenshot tool - Header-injection tool (browser extension or proxy) to send `X-Fabro-Demo: 1` -- The `fabro` Docker image built locally: `bin/dev/docker-build.sh` +- The `fabro` Docker image built locally: `cargo dev docker-build` ## Boot the demo environment @@ -28,7 +28,7 @@ curl -s -H "X-Fabro-Demo: 1" -o /dev/null -w "%{http_code}" http://localhost/run ## Applying temporary UI changes for screenshots -With the SPA baked into the binary, there's no in-container edit path. Commit the nav change on a local branch, rerun `bin/dev/docker-build.sh`, then `docker compose up -d --force-recreate`. +With the SPA baked into the binary, there's no in-container edit path. Commit the nav change on a local branch, rerun `cargo dev docker-build`, then `docker compose up -d --force-recreate`. ## Updating logos diff --git a/docs/plans/2026-04-23-003-refactor-pr-commands-server-side-plan.md b/docs/plans/2026-04-23-003-refactor-pr-commands-server-side-plan.md index bb2511200..0c4fc9fc4 100644 --- a/docs/plans/2026-04-23-003-refactor-pr-commands-server-side-plan.md +++ b/docs/plans/2026-04-23-003-refactor-pr-commands-server-side-plan.md @@ -486,7 +486,7 @@ Each `pr/*.rs` file collapses to: parse args → call `client.()` → pr - **Interaction graph:** Server handlers plug into the existing `AppState::github_credentials` + run-store event pipeline. `pr create` emits `PullRequestCreated` via `fabro_workflow::event::append_event` (stage-less envelope — pre-flight consumer audit in Unit 5). `pr merge` and `pr close` call GitHub and return — they do not write to run state. - **Error propagation:** Errors from `fabro_github::*` now surface as HTTP status + JSON body instead of anyhow errors in the CLI. Status codes are structured: 400 for input errors (missing git metadata, empty diff, unsupported host), 404 for no stored record, 502 for GitHub-said-not-found, 503 for missing server creds, 409 for already-exists, 500 only for actual code bugs. CLI prints the server's error message. - **State lifecycle risks:** Creating a PR now writes back to the authoritative server run store (fixes the existing bug where client-side create never persisted the record). **State beyond creation is not tracked.** After merge or close, `state.pull_request` is unchanged — Fabro does not mirror GitHub's merge/close lifecycle. `pr view` and `pr list` always re-read from GitHub for current status. -- **API surface parity:** Four new operations added to OpenAPI (view, create, merge, close); `pr list` has no new endpoint. **No changes to `RunSpec` schema.** TypeScript client regenerates automatically; run `bin/dev/docker-build.sh` + `scripts/refresh-fabro-spa.sh` once after OpenAPI changes land. +- **API surface parity:** Four new operations added to OpenAPI (view, create, merge, close); `pr list` has no new endpoint. **No changes to `RunSpec` schema.** TypeScript client regenerates automatically; run `cargo dev docker-build` + `scripts/refresh-fabro-spa.sh` once after OpenAPI changes land. - **Integration coverage:** Cross-boundary test — after `fabro pr create`, a `fabro pr view` against the same run returns the populated record. Unit 5 test scenarios cover this. - **Unchanged invariants:** `PullRequestRecord` field shape; `fabro_github::*` surface; in-run pipeline PR stage (still creates + emits `PullRequestCreated` with its stage scope when the pipeline is configured for PR creation); `RunSpec.repo_origin_url` + `RunSpec.base_branch` stay `Option`; runs without git metadata remain valid. diff --git a/docs/plans/2026-04-24-001-refactor-adopt-uv-patterns-plan.md b/docs/plans/2026-04-24-001-refactor-adopt-uv-patterns-plan.md index 425a34228..519313fc0 100644 --- a/docs/plans/2026-04-24-001-refactor-adopt-uv-patterns-plan.md +++ b/docs/plans/2026-04-24-001-refactor-adopt-uv-patterns-plan.md @@ -632,7 +632,7 @@ Solid arrows are real dependencies. Dashed lines show phases that are independen --- -- [ ] **Unit 5.3: Port `docker-build.sh` to `fabro-dev docker-build`** +- [x] **Unit 5.3: Port `docker-build.sh` to `fabro-dev docker-build`** **Goal:** Replace `bin/dev/docker-build.sh` with a subcommand. diff --git a/lib/crates/fabro-dev/src/commands/docker_build.rs b/lib/crates/fabro-dev/src/commands/docker_build.rs new file mode 100644 index 000000000..86d3614af --- /dev/null +++ b/lib/crates/fabro-dev/src/commands/docker_build.rs @@ -0,0 +1,291 @@ +use std::fmt; +use std::path::{Path, PathBuf}; +use std::process::Command; + +use anyhow::{Context, Result, bail}; +use clap::{Args, ValueEnum}; + +const ZIG_VERSION: &str = "0.13.0"; + +#[derive(Debug, Args)] +pub(crate) struct DockerBuildArgs { + /// Target Docker architecture. + #[arg(long, value_enum)] + arch: Option, + /// Docker image tag to build. + #[arg(long, default_value = "fabro")] + tag: String, + /// Stage the compiled binary without running docker build. + #[arg(long)] + compile_only: bool, + /// Print the Docker commands instead of running them. + #[arg(long)] + dry_run: bool, +} + +#[derive(Clone, Copy, Debug, Eq, PartialEq, ValueEnum)] +enum DockerArch { + Amd64, + Arm64, +} + +impl DockerArch { + fn detect() -> Result { + match std::env::consts::ARCH { + "x86_64" | "amd64" => Ok(Self::Amd64), + "aarch64" | "arm64" => Ok(Self::Arm64), + arch => bail!("unsupported host arch: {arch}"), + } + } + + fn target(self) -> &'static str { + match self { + Self::Amd64 => "x86_64-unknown-linux-musl", + Self::Arm64 => "aarch64-unknown-linux-musl", + } + } + + fn zig_arch(self) -> &'static str { + match self { + Self::Amd64 => "x86_64", + Self::Arm64 => "aarch64", + } + } +} + +impl fmt::Display for DockerArch { + fn fmt(&self, formatter: &mut fmt::Formatter<'_>) -> fmt::Result { + match self { + Self::Amd64 => formatter.write_str("amd64"), + Self::Arm64 => formatter.write_str("arm64"), + } + } +} + +struct DockerBuildPlan { + arch: DockerArch, + compile_only: bool, + tag: String, + workspace_root: PathBuf, +} + +#[expect( + clippy::print_stdout, + reason = "dev docker-build command reports progress and dry-run commands directly" +)] +pub(crate) fn docker_build(args: DockerBuildArgs) -> Result<()> { + let plan = DockerBuildPlan { + arch: args.arch.map_or_else(DockerArch::detect, Ok)?, + compile_only: args.compile_only, + tag: args.tag, + workspace_root: workspace_root(), + }; + + if args.dry_run { + for line in plan.dry_run_lines() { + println!("{line}"); + } + return Ok(()); + } + + plan.run() +} + +impl DockerBuildPlan { + #[expect( + clippy::print_stdout, + reason = "dev docker-build command reports progress directly" + )] + fn run(&self) -> Result<()> { + println!( + "Building fabro-cli for {} inside rust:1-bookworm via cargo-zigbuild...", + self.arch.target() + ); + let build_command = self.build_command(); + self.run_command(&build_command)?; + + println!("Extracting binary from builder cache..."); + std::fs::create_dir_all(self.context_dir()).with_context(|| { + format!( + "creating Docker context directory {}", + self.context_dir().display() + ) + })?; + let extract_command = self.extract_command(); + self.run_command(&extract_command)?; + + if self.compile_only { + println!( + "Staged docker-context/{}/fabro (skipping docker build per --compile-only).", + self.arch + ); + return Ok(()); + } + + println!("Building Docker image as {}...", self.tag); + let image_build_command = self.image_build_command(); + self.run_command(&image_build_command) + } + + fn dry_run_lines(&self) -> Vec { + let mut lines = vec![ + self.build_command().to_shell_line(), + format!("mkdir -p {}", shell_arg(self.relative_context_dir())), + self.extract_command().to_shell_line(), + ]; + if self.compile_only { + lines.push(format!("staged docker-context/{}/fabro", self.arch)); + } else { + lines.push(self.image_build_command().to_shell_line()); + } + lines + } + + fn build_command(&self) -> PlannedCommand { + let arch = self.arch.to_string(); + let target = self.arch.target(); + let zig_arch = self.arch.zig_arch(); + PlannedCommand::new("docker") + .arg("run") + .arg("--rm") + .arg("--platform") + .arg(format!("linux/{arch}")) + .arg("-v") + .arg(format!("{}:/src", self.workspace_root.display())) + .arg("-v") + .arg("fabro-docker-cargo-registry:/usr/local/cargo/registry") + .arg("-v") + .arg(format!("fabro-docker-cargo-target-{arch}:/target")) + .arg("-v") + .arg(format!("fabro-docker-zig-{arch}:/opt/zig")) + .arg("-v") + .arg(format!("fabro-docker-cargo-tools-{arch}:/opt/cargo-tools")) + .arg("-w") + .arg("/src") + .arg("-e") + .arg("CARGO_TARGET_DIR=/target") + .arg("-e") + .arg("LIBZ_SYS_STATIC=1") + .arg("rust:1-bookworm") + .arg("bash") + .arg("-c") + .arg(build_script(target, zig_arch)) + } + + fn extract_command(&self) -> PlannedCommand { + let arch = self.arch.to_string(); + PlannedCommand::new("docker") + .arg("run") + .arg("--rm") + .arg("--platform") + .arg(format!("linux/{arch}")) + .arg("-v") + .arg(format!("fabro-docker-cargo-target-{arch}:/target")) + .arg("-v") + .arg(format!("{}:/out", self.context_dir().display())) + .arg("rust:1-bookworm") + .arg("cp") + .arg(format!("/target/{}/release/fabro", self.arch.target())) + .arg("/out/fabro") + } + + fn image_build_command(&self) -> PlannedCommand { + PlannedCommand::new("docker") + .arg("build") + .arg("--platform") + .arg(format!("linux/{}", self.arch)) + .arg("-t") + .arg(&self.tag) + .arg(".") + } + + #[expect( + clippy::disallowed_methods, + reason = "dev docker-build intentionally runs synchronous Docker subprocesses" + )] + fn run_command(&self, planned: &PlannedCommand) -> Result<()> { + let status = Command::new(&planned.program) + .args(&planned.args) + .current_dir(&self.workspace_root) + .status() + .with_context(|| format!("running {}", planned.to_shell_line()))?; + + if !status.success() { + bail!("command failed with {status}: {}", planned.to_shell_line()); + } + + Ok(()) + } + + fn context_dir(&self) -> PathBuf { + self.workspace_root + .join("docker-context") + .join(self.arch.to_string()) + } + + fn relative_context_dir(&self) -> String { + format!("docker-context/{}", self.arch) + } +} + +struct PlannedCommand { + program: String, + args: Vec, +} + +impl PlannedCommand { + fn new(program: impl Into) -> Self { + Self { + program: program.into(), + args: Vec::new(), + } + } + + fn arg(mut self, arg: impl Into) -> Self { + self.args.push(arg.into()); + self + } + + fn to_shell_line(&self) -> String { + std::iter::once(shell_arg(&self.program)) + .chain(self.args.iter().map(shell_arg)) + .collect::>() + .join(" ") + } +} + +fn build_script(target: &str, zig_arch: &str) -> String { + format!( + "set -e; \ + apt-get update -qq && apt-get install -y -qq pkg-config perl make cmake xz-utils curl >/dev/null; \ + if [ ! -x /opt/zig/zig-linux-{zig_arch}-{ZIG_VERSION}/zig ]; then \ + curl -fsSL https://ziglang.org/download/{ZIG_VERSION}/zig-linux-{zig_arch}-{ZIG_VERSION}.tar.xz | tar -xJ -C /opt/zig; \ + fi; \ + export PATH=/opt/cargo-tools/bin:/opt/zig/zig-linux-{zig_arch}-{ZIG_VERSION}:$PATH; \ + if ! command -v cargo-zigbuild >/dev/null; then \ + cargo install --locked --root /opt/cargo-tools cargo-zigbuild; \ + fi; \ + rustup target add {target}; \ + cargo zigbuild --release -p fabro-cli --target {target}" + ) +} + +fn shell_arg(arg: impl AsRef) -> String { + let arg = arg.as_ref(); + if arg + .chars() + .all(|ch| ch.is_ascii_alphanumeric() || "_-./:=@".contains(ch)) + { + return arg.to_string(); + } + + format!("'{}'", arg.replace('\'', "'\\''")) +} + +fn workspace_root() -> PathBuf { + let mut root = Path::new(env!("CARGO_MANIFEST_DIR")).to_path_buf(); + root.pop(); + root.pop(); + root.pop(); + root +} diff --git a/lib/crates/fabro-dev/src/commands/mod.rs b/lib/crates/fabro-dev/src/commands/mod.rs index db5953e98..58d44b6e9 100644 --- a/lib/crates/fabro-dev/src/commands/mod.rs +++ b/lib/crates/fabro-dev/src/commands/mod.rs @@ -2,11 +2,10 @@ use anyhow::{Result, bail}; use clap::Args; mod check_boundary; +mod docker_build; pub(crate) use check_boundary::{CheckBoundaryArgs, check_boundary}; - -#[derive(Debug, Args)] -pub(crate) struct DockerBuildArgs; +pub(crate) use docker_build::{DockerBuildArgs, docker_build}; #[derive(Debug, Args)] pub(crate) struct ReleaseArgs; @@ -17,10 +16,6 @@ pub(crate) struct RefreshSpaArgs; #[derive(Debug, Args)] pub(crate) struct CheckSpaBudgetsArgs; -pub(crate) fn docker_build(_args: DockerBuildArgs) -> Result<()> { - not_yet_implemented("docker-build") -} - pub(crate) fn release(_args: ReleaseArgs) -> Result<()> { not_yet_implemented("release") } diff --git a/lib/crates/fabro-dev/tests/it/docker_build.rs b/lib/crates/fabro-dev/tests/it/docker_build.rs new file mode 100644 index 000000000..fcf16901c --- /dev/null +++ b/lib/crates/fabro-dev/tests/it/docker_build.rs @@ -0,0 +1,99 @@ +fn fabro_dev() -> assert_cmd::Command { + assert_cmd::cargo::cargo_bin_cmd!("fabro-dev") +} + +fn output_text(bytes: &[u8]) -> String { + String::from_utf8(bytes.to_vec()).expect("command output should be valid utf-8") +} + +#[test] +fn help_lists_docker_build_flags() { + let output = fabro_dev() + .args(["docker-build", "--help"]) + .assert() + .success() + .get_output() + .clone(); + let stdout = output_text(&output.stdout); + + for flag in ["--arch", "--tag", "--compile-only", "--dry-run"] { + assert!( + stdout.contains(flag), + "docker-build help should list {flag}:\n{stdout}" + ); + } +} + +#[test] +fn invalid_arch_fails_with_clap_error() { + let output = fabro_dev() + .args(["docker-build", "--arch", "invalid"]) + .assert() + .failure() + .code(2) + .get_output() + .clone(); + let stderr = output_text(&output.stderr); + + assert!( + stderr.contains("invalid value 'invalid'"), + "invalid arch should be rejected by clap:\n{stderr}" + ); +} + +#[test] +fn dry_run_prints_equivalent_build_commands() { + let output = fabro_dev() + .args([ + "docker-build", + "--arch", + "amd64", + "--tag", + "fabro:smoke", + "--dry-run", + ]) + .assert() + .success() + .get_output() + .clone(); + let stdout = output_text(&output.stdout); + + assert!( + stdout.contains("docker run --rm --platform linux/amd64"), + "dry-run should print builder docker run:\n{stdout}" + ); + assert!( + stdout.contains("cargo zigbuild --release -p fabro-cli --target x86_64-unknown-linux-musl"), + "dry-run should print cargo-zigbuild target:\n{stdout}" + ); + assert!( + stdout.contains("docker build --platform linux/amd64 -t fabro:smoke ."), + "dry-run should print image build command:\n{stdout}" + ); +} + +#[test] +fn dry_run_compile_only_skips_image_build() { + let output = fabro_dev() + .args([ + "docker-build", + "--arch", + "arm64", + "--compile-only", + "--dry-run", + ]) + .assert() + .success() + .get_output() + .clone(); + let stdout = output_text(&output.stdout); + + assert!( + stdout.contains("docker-context/arm64/fabro"), + "dry-run compile-only should print staged binary path:\n{stdout}" + ); + assert!( + !stdout.contains("docker build --platform"), + "dry-run compile-only should not print image build:\n{stdout}" + ); +} diff --git a/lib/crates/fabro-dev/tests/it/main.rs b/lib/crates/fabro-dev/tests/it/main.rs index 017e0d254..b80d4d1f1 100644 --- a/lib/crates/fabro-dev/tests/it/main.rs +++ b/lib/crates/fabro-dev/tests/it/main.rs @@ -2,6 +2,7 @@ use std::path::PathBuf; use std::process::{Command, Output}; mod check_boundary; +mod docker_build; fn fabro_dev() -> assert_cmd::Command { assert_cmd::cargo::cargo_bin_cmd!("fabro-dev")