[dhruv8sh]

Summary

• CI scripts that drive EdgeZero have had to scrape key=value log lines that were never a stable contract. Nine commands now accept --format json: active-version, auth status, build, config gc, config validate, deploy, healthcheck, provision and rollback. Each one writes a single versioned envelope, { schema_version, command, ok, result, error }, to stdout, on success and on failure.
• stdout stays clean because, under JSON, logs and every child process's stdout (cargo, fastly, wrangler, spin, manifest commands) go to stderr. A new clippy disallowed-methods lint means future code can't bypass this. --format text is the default, and its output is byte-identical to main.
• Follows the spec that Add --json output across the CLI #383 requires, included in this PR as docs/superpowers/specs/2026-09-25-cli-format-json-design.md. config diff --format json is unchanged.

Changes

Crate / File	Change
edgezero-adapter (registry.rs)	Adapter::execute returns ActionOutcome, provision returns ProvisionReport, gc_config_entries returns GcReport, instead of () or prose lines. A negative result that was still measured (unhealthy probe, unauthenticated session, partly failed gc) is an Ok outcome carrying a failure message.
edgezero-adapter (process.rs, new)	Process-wide child-stdout policy plus process::status, the one sanctioned inheriting spawn.
edgezero-adapter (cli_support.rs)	native_auth_status. run_native_cli goes through process::status.
edgezero-adapter-{fastly,cloudflare,spin,axum}	Return typed outcomes and reports. Fastly no longer prints the version= / healthy= / status-code= / rolled-back-to= data lines; the CLI prints the same bytes. All inheriting spawns go through process::status.
edgezero-cli (output.rs, new)	OutputScope (routes stdout while alive), Failure / Outcome, the envelope, and the serde wire schema, kept separate from the adapter types so internal refactors can't change the JSON.
edgezero-cli (args.rs)	New OutputFormat { Text, Json } and a --format flag on the nine commands. DiffFormat is unchanged.
edgezero-cli (lib.rs, auth.rs, provision.rs, config.rs, adapter.rs)	Each run_* keeps its public signature and emits the envelope itself, so CLIs already generated from the template get JSON without regenerating main.rs. The logger's info output moves to stderr under JSON.
clippy.toml	disallowed-methods for Command::status / Command::spawn. Piped spawns carry a documented #[expect].
edgezero-cli/tests/format_json.rs (new)	End-to-end tests against the real binary.
docs/guide/cli-reference.md	--format on each command, plus a new "Machine-readable output" section covering the envelope, streams, exit codes, compatibility policy, per-command schemas and changelog.
docs/superpowers/specs/…-cli-format-json-design.md	The spec, revision 2 (§13 notes what changed during implementation).

Closes

Closes #383

Test plan

• cargo test --workspace --all-targets
• cargo clippy --workspace --all-targets --all-features -- -D warnings
• cargo fmt --all -- --check
• cargo check --workspace --all-targets --features "fastly cloudflare spin"
• WASM builds: wasm32-wasip1 (Fastly) / wasm32-wasip2 (Spin) / wasm32-unknown-unknown (Cloudflare), via the full format.yml wasm clippy matrix and cargo check -p edgezero-adapter-spin --target wasm32-wasip2 --features spin
• examples/app-demo workspace: cd examples/app-demo && cargo test --workspace --all-targets (--locked), plus its fmt and clippy
• Docs build: cd docs && npm run lint && npm run format && npm run build
• Manual testing via edgezero serve --adapter axum (not applicable: serve is out of scope)
• Other:
  • Text mode compared byte-for-byte against main: both binaries ran 21 hermetic scenarios (success and failure paths for every in-scope command, a usage error, the bundled stub), with stdout, stderr and exit codes identical in all of them.
  • cargo test -p edgezero-cli --test generated_project_builds -- --ignored, cargo test -p edgezero-adapter-fastly --features cli, and the check_no_nested_app_config steps.
  • All checks were run on the pinned toolchain 1.95.0.

Checklist

• Changes follow CLAUDE.md conventions
• No Tokio deps added to core or adapter crates
• Route params use {id} syntax (not :id)
• Types imported from edgezero_core (not http crate)
• Store wiring goes through KvRegistry / ConfigRegistry / SecretRegistry (not the legacy single-handle setters) — see spec §6.6
• New code has tests
• No secrets or credentials committed

[aram356]

PR Review

Summary

Adds --format json to nine commands: one versioned envelope on stdout, with logs and child stdout moved to stderr, and typed adapter outcomes instead of printed lines. The design holds up, and the text path is unchanged (see CI Status). Three JSON results are wrong and should be fixed before schema v1 ships: version_verified on an unhealthy probe, build silently dropping a trailing --format json, and config gc --yes reporting deleted: null when there was nothing to reclaim.

Findings

Blocking

• 🔧 version_verified: true on an unhealthy probe: the after-probe check only runs when the probe is healthy (crates/edgezero-adapter-fastly/src/cli.rs:5641)
• 🔧 build swallows a trailing --format json: exits 0 with text on stdout (crates/edgezero-cli/src/args.rs:251)
• 🔧 config gc --yes with nothing to reclaim reports deleted: null: the docs say null means a dry run (crates/edgezero-adapter-fastly/src/cli.rs:2497)
• ❓ Spec approval: the spec header says "Status: Approved", and #383 asks for the spec to be approved before implementation starts. I couldn't find the approval on #383 or on this PR (the spec and the implementation arrived in the same push). Was it approved somewhere else? If so, a link in the spec header would help.

Non-blocking

• 🤔 Raw config validate reports app_config: null although it reads and requires that file (crates/edgezero-cli/src/config.rs:235)
• 🤔 Provision dry-run actions don't pair with real-run actions (crates/edgezero-adapter-spin/src/cli.rs:249, crates/edgezero-adapter-fastly/src/cli.rs:522)
• 🤔 A deploy that went live but couldn't resolve its version reports result: null (crates/edgezero-cli/src/lib.rs:382)
• 🤔 deploy.service_id reflects only the flag (crates/edgezero-cli/src/lib.rs:216)
• ♻️ Outcome types can hold contradictory values (crates/edgezero-adapter/src/registry.rs:116)
• ⛏ Small items: the AuthSub::Status variant (args.rs:237), OutputFormat compared with == (output.rs:75), a leftover bare block (edgezero-adapter-fastly/src/cli.rs:3958), and an unreachable --require-active promise in the spec (spec L152)
• 🤔 Docs: nullability and edge cases
  • The compatibility policy says making a non-null key nullable is breaking, but the Results section lists key names only, so readers can't tell which keys may be null. Spec §10 called for one schema table per command. Nullable today: active-version.version, build.artifact, deploy.service_id, deploy.version, healthcheck.staging_ip, healthcheck.status_code, rollback.rolled_back_to, provision.entries[].store, provision.entries[].store.logical, config gc older_than_secs / deleted / store.id, and config validate.app_config.
  • Exit code 2 means three things in a generated CLI: a command failure, a clap usage error, and an unsupported config diff. The "empty stdout means no envelope" rule in Streams is what tells them apart; worth repeating next to Exit codes.
  • --format json --help (and --version) print clap's text to stdout and exit 0, which contradicts "stdout holds exactly one JSON document".
  • JSON routing depends on init_cli_logger() being the installed logger. A downstream CLI that installs its own logger (e.g. simple_logger, which writes every level to stdout) would put log lines on stdout. One sentence in the docs would cover it.
• 🤔 Test gaps
  • No test produces version_verified: true (the existing healthcheck test removes the token).
  • The config gc mapping from failure_diagnostic to a partial result (config.rs:467-469), and how failed / stranded / uncertain / deleted are filled, have no JSON-level test.
  • deploy, rollback and active-version have no tests of their result mapping, or of the version= / rolled-back-to= / "no active version yet" lines that moved from the adapter into the CLI.
  • config validate has no failure (null result) or typed-mode test.
  • run_shell_tee (the deploy capture path) isn't exercised under JSON.
  • The end-to-end tests cover 5 of the 9 commands, and the 21-scenario byte comparison from the PR description isn't committed. Even a reduced version committed as a test would keep the text contract from drifting.
• 🌱 Lint gaps: disallowed-methods fires on every status / spawn form I tried (checked in a scratch crate using this clippy.toml). It does not catch .stdout(Stdio::inherit()).output() (which really does leak at runtime), CommandExt::exec, or writeln!(io::stdout(), …) (print_stdout only covers the macros). None of these is used today. Adding std::io::stdout to the list, with #[expect] at the three existing call sites (output.rs:537, adapter.rs:431, config.rs:1370), would close the last gap. The generated project's templates/root/clippy.toml.hbs doesn't carry the lint, so a downstream CLI's own subcommands are unguarded.
• 🌱 A closed stderr loses the envelope: CliLogger writes routed info with eprintln!, which panics on a broken pipe. Under JSON all info goes to stderr, so if the stderr reader exits first, the command panics at its first log line (exit 101) and writes no envelope. Text mode has the same failure on stdout, so this isn't new, but JSON makes stderr the busy stream. let _ = writeln!(io::stderr(), "{}", record.args()); avoids it.

📌 Out of Scope

• wrangler whoami exit code when logged out (unverified; wrangler wasn't available to test): some wrangler versions print "You are not authenticated" and still exit 0. If current ones do, auth status --adapter cloudflare --format json reports state: authenticated. Text mode already exited 0 in that case, so this predates the PR, but the JSON value now states it outright. Worth checking, and if so parsing the whoami output rather than trusting the exit code.

CI Status

• fmt: PASS
• clippy: PASS
• tests: PASS (cargo test --workspace --all-targets: 1459 passed, 0 failed)
• cargo check --workspace --all-targets --features "fastly cloudflare spin": PASS
• cargo check -p edgezero-adapter-spin --target wasm32-wasip2 --features spin: PASS
• cargo test -p edgezero-adapter-fastly --features cli: PASS (288 passed)
• Text mode vs main: 130 paired hermetic runs across the 9 commands (fake curl / fastly / wrangler / spin / cargo) gave byte-identical stdout, stderr and exit codes. The only difference is the usage-error text for --format yaml.

[prk-Jr]

PR Review (re-review at cc6f1ac)

Summary

The fix commit addresses aram356's blocking items, and the new tests fail against the old code: version_verified only when both checks ran, deleted: 0 on an empty real gc run, and the late---format guard. Almost every non-blocking item is closed too.

I also built this branch and main and ran them with fake fastly / curl / wrangler / spin / cargo:

• Under --format json, stdout held exactly one valid envelope in every scenario tried. That covered all 9 commands on success and failure paths, more than 1 MB of output from a child process, a killed child, RUST_LOG=trace, a closed stderr, and Unicode/escaping in paths and messages.
• In text mode, 49 of 54 scenarios were byte-identical to main. All 5 differences come from the new flag, and one is the build -- --format case below.

What remains is the unanswered spec-approval question, two doc errors in the output contract, and some test gaps.

😃 Praise

• One gate for child stdout: every child process that inherits our stdout goes through edgezero_adapter::process::status, the disallowed-methods lint enforces it, and the exceptions carry #[expect] with reasons. "stdout stays clean" can't quietly regress.
• Routing can't get stuck: OutputScope saves and restores the routing flags in Drop, so a ?, an early return or a panic always restores routing.
• Each fact stored once: AuthState::Unauthenticated { reason }, HealthcheckOutcome::healthy() and GcFailure rule out the contradictory states.
• Tests that check behaviour: the fake-curl API call log proves both version checks ran, rather than just trusting the flag. sole_json_document and assert_envelope_invariants make the one-envelope rule hard to break.
• Docs match the code: the per-command result tables match the serde wire structs field by field, including nullability and enum spellings.

Findings

Blocking

• ❓ Spec approval still unanswered (spec:4)
• 🔧 Docs promise a --version flag that doesn't exist (cli-reference.md:662)
• 🔧 Empty-stdout list misses the late---format rejection, which contradicts the exit-1 guarantee (cli-reference.md:657)

Non-blocking

• 🤔 build rejects -- --format json, deploy forwards it. This also changes text mode for build (lib.rs:176).
• 🤔 auth status decides authenticated from the exit code alone. Real wrangler exits 0 when not logged in (cli_support.rs:112).
• 🤔 A broken stdout pipe hides the command's real error (output.rs:545).
• 🤔 The plan doc's status is wrong, and it includes a line specific to one machine (plan:5).
• 🤔 Spec §7.2 types and §9.3 test list are stale (spec:331).
• 🤔 OutputScope doesn't support overlapping runs on separate threads. It needs a doc note (output.rs:68).
• 🤔 Test gaps:
  • config gc is the only one of the 9 commands with no end-to-end test. The CLI-side mapping (dry_run = args.dry_run || !args.yes, older_than_secs, and not printing text_lines on failure) is untested. The fake_fastly_gc helper in the Fastly adapter tests could be reused.
  • auth status has end-to-end tests only for the unauthenticated case. There's no test of the authenticated path or of a missing native CLI (result: null per the docs).
  • Staging deploy / rollback aren't tested. That includes the version= line that moved from the adapter to the CLI, and the resolved service_id (flag, then FASTLY_SERVICE_ID).
  • The generated project's new clippy.toml.hbs lines aren't asserted. The generator test only checks allow-expect-in-tests, so adding assert!(clippy.contains("std::io::stdout")) would cover it.
  • The text-mode checks for build, rollback and deploy use starts_with / ends_with, so lines added in the middle would pass. auth, provision, validate and gc have no text-mode check. The byte comparison against main from spec §9.2 is still not committed.
  • The test harness only clears FASTLY_API_TOKEN / FASTLY_SERVICE_ID. An exported EDGEZERO__* or app-overlay variable on the machine running the tests can change provision and validate results.
• ♻️ Fastly staged deploy works out the service id a second time with .ok(). The production Deploy returns Deploy{None,None} while other adapters return Empty (fastly cli.rs:434).
• ♻️ Result<Result<u64, String>, String> would read better as a small enum. deploy() is about 140 lines (lib.rs:368).
• ⛏ Small items:
  • healthcheck reports status_code: 0 when curl prints 000. The docs say null when no probe got a response.
  • A production Fastly deploy with only FASTLY_SERVICE_ID set reports service_id: null, although fastly used that id. Worth one sentence in the deploy table.
  • The #[expect] reason at config.rs:1374 says "config push has no --format", but config diff calls the same function.
  • GcResult::new clones the whole GcFailure, diagnostic string included, just to read three lists. Destructure it by reference instead.
  • auth status --help describes --format with shorter wording than the other eight commands (args.rs:236).
  • The process tests restore the global setting by hand. If expect("spawn") panics, the setting stays true and POLICY_LOCK is poisoned. Use a drop guard.
  • native_auth_status_maps_exit_status_to_state runs true / false but isn't #[cfg(unix)], unlike the other fake-binary tests.
• 🌱 Lint gaps:
  • disallowed-methods doesn't catch std::os::unix::process::CommandExt::exec or .stdout(Stdio::inherit()).output(). Neither is used today.
  • The public outcome structs (HealthcheckOutcome, GcReport, RollbackOutcome, ProvisionEntry, …) aren't #[non_exhaustive], so adding a field later breaks adapters built outside this repo.

📌 Out of Scope

These all predate this PR. Worth tracking as follow-ups:

• provision failing partway: the result is null and neither stream says what was already created.
• Deploy output parsing: in the tee, a non-UTF-8 line stops the version parsing, so a later SUCCESS … version 12 line is never seen. With more than 64 KB after the bad byte, the child gets SIGPIPE.
• Built-in Fastly production deploy: this path doesn't capture fastly compute deploy's output. Without a token it fails with "no version=<N> line in the deploy output" even when Fastly printed the version.
• Empty token: FASTLY_API_TOKEN= (empty) is treated as set.
• Rollback to the same version: rollback --version 7 --rollback-to 7 is accepted.

CI Status

• fmt: PASS
• clippy (--workspace --all-targets --all-features -D warnings): PASS
• tests (cargo test --workspace --all-targets): PASS, 0 failures
• cargo check --workspace --all-targets --features "fastly cloudflare spin": PASS
• cargo check -p edgezero-adapter-spin --target wasm32-wasip2 --features spin: PASS
• Black-box JSON contract: every scenario tried produced exactly one valid envelope. Text mode vs main: 49/54 byte-identical; the 5 differences are expected, and one is the build -- --format case above.

[ChristianPavilonis]

Review summary

Reviewed all 24 changed files and traced adapter outcomes, CLI consumers, subprocess routing, and Fastly action parsers.

Head: cc6f1ac99aa09de8025bc26847e424dd7b02fba9
Base: 98930917d96cb665c7255f36ca8bbd61fc7539e1

Safety proof

1. JSON remains isolated from logs and child stdout on tested paths. Executed and proven for covered cases: all 18 binary-level format tests passed, including inherited and captured child output, failure envelopes, and token-backed health verification.
2. GC preserves partial-failure information without continuing a damaged generation. Proven through the binary with a fake provider: an injected second-delete failure produced exit 1, deleted: 1, and exact failed/stranded keys. The third delete was not attempted. A rerun retained the incomplete generation; an unknown root blocked deletion.

Findings

No meaningful new findings beyond existing PR feedback. This approval reflects the new-issues-only scope of this review; existing review concerns remain unresolved.

Validation and review context

Passed locally at the locked head:

• cargo test --workspace --all-targets --locked
• cargo fmt --all -- --check
• cargo clippy --workspace --all-targets --all-features --locked -- -D warnings
• cargo check --workspace --all-targets --features "fastly cloudflare spin" --locked
• cargo check -p edgezero-adapter-spin --target wasm32-wasip2 --features spin --locked
• Focused JSON, Fastly CLI, and adapter tests.
• Binary-level GC interruption and rerun checks.

All currently reported GitHub checks passed. Existing reviews, inline threads, and issue comments were inspected to avoid duplicate findings.

No live-provider operations were attempted. The generated-project compilation test remained ignored locally. Head/base were unchanged at completion, and the worktree remained clean. No repository files changed; no delegation.

[dhruv8sh]

Addressed in e6a0b12, beyond the inline threads:

• Test gaps: format_json.rs went from 18 to 28 tests.
  • config gc end to end: a default dry run (dry_run: true, deleted: null, older_than_secs: null) and --yes --older-than 1h with nothing to reclaim (deleted: 0, older_than_secs: 3600).
  • auth status: the authenticated path, and a missing native CLI (result: null).
  • A staged deploy resolving its service from FASTLY_SERVICE_ID, and a staged rollback.
  • Exact text bytes for all nine commands, replacing the starts_with/ends_with checks. This is the committed stand-in for the byte comparison against main.
  • The harness also clears EDGEZERO__* / DEMO_APP__*.
  • The generator test asserts each disallowed-methods path in the generated clippy.toml.
  • Still not covered end to end: the GcFailure to partial-result path. Building a failing chunked generation needs the Fastly crate's private chunker. That mapping is covered by gc_failure_fills_the_partial_result in output.rs and the adapter's gc tests.
• Small items:
  • healthcheck treats curl's 000 as no response rather than status 0: status_code keeps the last real HTTP status, or null if no probe got one.
  • The deploy table notes that a production deploy relying on FASTLY_SERVICE_ID reports service_id: null.
  • The #[expect] reason in config.rs now mentions config diff.
  • GcResult::new destructures GcFailure by reference instead of cloning it.
  • auth status --help uses the same --format wording as the other commands.
  • The process tests use a drop guard and recover a poisoned lock.
  • native_auth_status_maps_exit_status_to_state is #[cfg(unix)].
• Lint gaps:
  • std::os::unix::process::CommandExt::exec is now disallowed (with allow-invalid, for non-unix targets), in the workspace and in the generated project.
  • I didn't ban Stdio::inherit: .stderr(Stdio::inherit()) is legitimate, and the lifecycle fixture harness uses it.
  • I didn't add #[non_exhaustive] to the outcome structs. Adapter crates build them with struct literals, and #[non_exhaustive] forbids that outside edgezero-adapter, so every adapter would need constructors or builders. Today all adapters are in this workspace (publish = false).
• CI: the cargo fmt job was red on the merge head (082168c). The lifecycle fixture from Add opt-in reusable application lifecycles #379 has no clippy.toml of its own, so it picked up the root disallowed-methods guard. It now has its own clippy.toml with just the test exemptions.
• Out of scope: the five pre-existing items (partial provision, the non-UTF-8 tee, the uncaptured built-in production deploy, an empty token, rolling back to the same version) are left for follow-up issues.

[aram356]

PR Review (re-review at 392546a)

Summary

Every finding from the first two review rounds is fixed on the legacy deploy path, and the new tests prove it: 6 of the 28 end-to-end tests fail against ee94a6a because of the bugs they cover. Text output still matches main across 124 of 137 compared scenarios: 11 differences are intended (help text, --format parsing, the build rejection) and 2 are the healthcheck 000 change below. All 158 JSON-mode runs, including main's new managed Fastly deploy, wrote exactly one envelope.

What's left comes from the merge with main (#381). Fastly deploys with --staging, --application-release or declared stores now take an adapter-managed path. The PR's JSON handling and docs weren't brought along, so a managed deploy that fails after creating (or even activating) a version reports result: null, and the deploy docs describe only the legacy path.

Findings

Blocking

• 🔧 A managed deploy that fails after creating a version reports result: null, even when the version went live (crates/edgezero-cli/src/adapter.rs:219)
• 🔧 The deploy docs and spec describe only the legacy path (docs/guide/cli-reference.md:822, spec L757)

Non-blocking

• 🤔 The JSON deploy result has no package_sha256, which the deploy-fastly action reads from text (crates/edgezero-cli/src/output.rs:210)
• 🤔 The healthcheck 000 change alters text output and isn't declared (crates/edgezero-adapter-fastly/src/cli.rs:6833)
• 🤔 The provision schema is stale after the merge: edgezero_runtime_env, a store.logical that is never null, and updated / would_update with no producer (docs/guide/cli-reference.md:873)
• ⛏ Doc comments made stale by the refactor and the merge (crates/edgezero-adapter/src/registry.rs:48)
• ⛏ Loose ends on the late---format rejection (crates/edgezero-cli/src/lib.rs:189)
• 🤔 Test gaps
  • The merge replaced the only successful staged-deploy end-to-end test (staged_deploy_text_and_json_resolve_the_service_from_the_env) with a failure-only one, which reopens the earlier gap for the resolved staged service_id. deploy_managed_with_context and execute_managed_deploy_plan_with_emit have no tests in the repo, so neither a managed success nor a managed failure is tested in either format.
  • Also untested:
    • a DeployFailure::Finalize on the fallback path (built-in fastly compute deploy + finalize_deploy(None));
    • "a finalizer's Deploy outcome takes precedence" (adapter.rs:264), since RecordingDeployAdapter returns Empty everywhere;
    • the GcFailure → partial result mapping in config.rs end to end (gc_failure_fills_the_partial_result re-implements the mapping rather than calling it);
    • the broken-stdout / broken-stderr behaviour from the last round;
    • a mixed 503 / 000 healthcheck.
  • fake_fastly_staging and FASTLY_STAGING_MANIFEST (format_json.rs L55, L271) are now dead; the test that used them fails before any provider call.
• ⛏ Spec §7.2 and the revision notes are stale after the merge
  • L293-294: the Empty comment lists "production deploy", but a managed production deploy returns Deploy.
  • L362: "Data lines move from the Fastly adapter to the CLI" isn't true for deploy. version= and package-sha256= are still logged by the adapter (fastly cli.rs:927, :1048, :1100, :2117).
  • The trait listing doesn't show deploy / finalize_deploy / preflight_deploy, or DeployFailure.
  • L281 counts "two test adapters"; there are four.
  • Rev 3 (L715): nothing passes --require-active any more. Rev 4 (L735): "the Fastly production deploy returns Empty" now holds only for the legacy path.
  • §9.2 (L593) lists the build rejection as the only intended text change, which leaves out the healthcheck 000 change.
• ⛏ Small items
  • A healthy probe whose after-probe version check fails returns result: null. Spec §8 (L504) only covers failures before the probe. Document it, or return the result with version_verified: false.
  • On the legacy path, when a version is parsed from the deploy output but can't be verified (no token), the Finalize partial result drops the parsed version, and the error is just "FASTLY_API_TOKEN must be set in the environment", with no "the deploy went live" context.
  • finish hard-codes the [edgezero] prefix in its write-failure log (output.rs:561), even in generated CLIs that use [<name>]. Rev 4 says "the command's outcome is returned"; for a command that succeeded, the write error replaces it.
• 🌱 Under JSON, a broken stderr pipe kills child processes. process::status (process.rs:42-44) points the child's stdout at our stderr, so if the stderr reader goes away the child dies of SIGPIPE. Reproduced: build = "echo built-ok" succeeds in text mode and fails with signal: 13 under JSON. A managed deploy can fail this way after a provider mutation. run_shell_tee drains the output and ignores write errors, so the two paths behave differently. The envelope itself is still written in every case.

📌 Out of Scope

• The generated project's new disallowed-methods guard isn't exercised in CI. CI only cargo checks a generated project, and the generator test string-matches clippy.toml. Generated projects already fail strict clippy for reasons this PR didn't cause: item ordering in templates/cli/src/main.rs.hbs (Gc after Validate, L80/L86) and lints from the #[fastly::main] expansion on wasm32-wasip1.

CI Status

• fmt: PASS
• clippy (--workspace --all-targets --all-features -D warnings): PASS
• tests (cargo test --workspace --all-targets): PASS (1569 passed, 0 failed; format_json 28/28)
• cargo check --workspace --all-targets --features "fastly cloudflare spin": PASS
• cargo check -p edgezero-adapter-spin --target wasm32-wasip2 --features spin: PASS
• cargo test -p edgezero-adapter-fastly --features cli: PASS (325 passed)
• Text mode vs main (f717674): 137 paired runs against fake fastly / curl / wrangler / spin / cargo, including managed deploys built from real verified releases. 124 byte-identical in stdout, stderr, exit code and call log; 11 intended differences; 2 healthcheck 000 differences (above).
• JSON mode: 158 runs across the 9 commands (including managed staging and production deploys). Each wrote exactly one envelope, and ok matched the exit code.

[prk-Jr]

🔧 Request changes — review of daa10282ff8c97f219e196e9486529eee26cb2bc.

Reviewed all 24 changed files, including CLI orchestration, wire schemas, adapter outcomes, subprocess routing, Fastly deployment lifecycle, authentication, provisioning, GC, generated-project changes, tests, and documentation. Three parallel reviewers covered Fastly, shared/platform adapters, and the contract/tests/docs; findings were cross-checked before inclusion.

🔧 Required changes: restore the pinned-toolchain Clippy gate, correct Cloudflare’s false authenticated result, and preserve known recovery information on managed deployment failures. Details and suggested fixes are in the inline comments.

🤔 Non-blocking: qualify the deploy documentation’s legacy-path claims, which do not describe managed production deployment results.

😃 Praise: The binary-level tests check single-document stdout, failure envelopes, and exact text output across all nine commands. Centralized child stdout routing and the Clippy guard also make future contamination harder to introduce.

📝 Validation on pinned Rust 1.98.1:

• cargo fmt --all -- --check: PASS.
• cargo test --workspace --all-targets: PASS — 1,573 passed, zero failed, one ignored. The initial sandbox run could not bind sockets; the unrestricted rerun passed.
• cargo clippy --workspace --all-targets --all-features -- -D warnings: FAIL on newly added code, as detailed inline. A scoped CLI run exposed the additional inline-bound and Drop failures.
• cargo check --workspace --all-targets --features "fastly cloudflare spin": PASS.
• Adapter WASM checks: PASS — Fastly wasm32-wasip1, Cloudflare wasm32-unknown-unknown, Spin wasm32-wasip2.
• Full docs Prettier check: PASS using Prettier 3.9.6. Docs ESLint/build and the ignored generated-project test were not run.
• Real Wrangler 4.83.0 with an isolated temporary HOME reproduces the false authenticated JSON result without using real credentials.

📝 Scope and cleanup: No source files were changed. The temporary review worktree has been removed; the original checkout is unchanged.

[dhruv8sh]

Round 3/4 follow-ups not covered in the threads:

• Test gaps: added the Finalize failure on the fallback path, finalizer-Deploy precedence, the managed failure mapping in both formats, a mixed 503/000 probe, and a failed envelope write (stdout on /dev/full). Removed the dead fake_fastly_staging / FASTLY_STAGING_MANIFEST. Still open: a managed deploy driven to success or failure through provider fakes needs a verified-release fixture in the Rust suite, and the gc end-to-end failure needs the chunk-envelope fixtures that live in the Fastly crate's private tests. The CLI-side gc glue is a three-line if let over report.failure.
• After-probe version check null: documented in the reference and spec §8.
• Legacy Finalize drops the parsed version: a version reported by the deploy command that can't be verified (no token, or not active) is now kept as version_state: unknown, with the same error text.
• finish's [edgezero] prefix: I left it. Every library log line in edgezero-cli uses [edgezero], including in generated CLIs. Only the generated main's final eprintln! uses [<name>], so changing this one line would make it the odd one out. The write-error-replaces-success behaviour is now stated in revision 6.
• 🌱 SIGPIPE on a broken stderr under JSON: agreed it's a real asymmetry with run_shell_tee. I'd handle it as a follow-up issue rather than in this PR: draining the child through a pipe changes the spawn model for every inheriting call site.
• Out of scope (generated-project clippy): agreed, also a follow-up.

Pushed as be91017.

[prk-Jr]

🔧 Request changes — one blocking finding at 8e0075ad59f0a6b0fce421f986401f5fee601125.

The new Clippy rules also apply to the standalone request-fidelity runner and break its required CI check. The inline finding includes the proposed correction and verification.

😃 Praise: The binary tests verify single-document JSON output, partial failure results, and preserved text output across all nine commands.

📝 Validation on Rust 1.98.1: Workspace tests passed (1,608 passed, zero failed, one ignored) after rerunning with local socket access. Formatting, workspace Clippy, feature compilation, and the Spin WASM check passed. Fixture Clippy fails with the committed configuration and passes when pointed at a temporary copy of the lifecycle fixture's configuration. No live-provider operations were tested.

[prk-Jr]

😃 Approve — reviewed a90da4e0c765c7452366685a3bcb89d4377ca130 against 87456c708ffe9edf86b3a0f8b06dfd15d60b07b5. No new actionable code defects found. The previous Clippy, Cloudflare authentication, and managed-deployment recovery findings are addressed. The binary tests verify a single JSON document and expected text output across all nine commands.

📝 Remaining notes:

• The design spec still says “Proposed, awaiting maintainer approval.” Reconcile that status with the maintainer acceptance required by #383.
• Previously discussed end-to-end coverage gaps remain for managed-deployment provider failures and GC partial failures. These are verification limits, not newly confirmed defects. No live-provider operations were tested.

📝 Validation on Rust 1.98.1: Workspace tests passed (1,608 passed, zero failed, one ignored), including all 29 JSON integration tests. Formatting, workspace Clippy, feature compilation, and the Spin WASM compilation check passed. The request-fidelity runner's formatting, Clippy, and tests passed (20 passed, one ignored). Socket-dependent tests passed after rerunning with loopback socket access. All 29 hosted checks were green at the final revision check.

[aram356]

PR Review (re-review at a90da4e)

Summary

Every finding from the previous rounds is fixed. The managed and legacy deploy paths now keep a recoverable version with version_state and package_sha256, the healthcheck text is back to main's, the provision schema is tightened, and clippy passes on 1.98.1. Against main (87456c7), --format text was byte-identical in 500 of 512 scenarios, and the other 12 are the build rejection and the Cloudflare whoami --json switch. 499 JSON-mode runs, including a failure injected at each of 260 managed-deploy steps, all wrote exactly one valid envelope.

Two things block: the new version_state can say draft for a version that is already live, and the managed progress tracking behind the last round's fix has no test that would catch its removal. The rest is wording and declaration.

Findings

Blocking

• 🔧 draft is reported for a version the code has just seen published (crates/edgezero-adapter-fastly/src/cli.rs:927)
• 🔧 The managed progress tracking has no regression test: a mutation removing it leaves every test green (crates/edgezero-adapter-fastly/src/cli.rs:1043)

Non-blocking

• 🤔 The Cloudflare auth status change is wider than declared (spec L27)
• ⛏ The Cloudflare probe error message is malformed, and some probe edge cases need a decision (crates/edgezero-adapter-cloudflare/src/cli.rs:559)
• 🤔 result: null doesn't mean nothing was created (docs/guide/cli-reference.md:857)
• ⛏ The status_code docs contradict the behaviour (docs/guide/cli-reference.md:870)
• ♻️ version and version_state can disagree (crates/edgezero-adapter/src/registry.rs:121)
• ⛏ The compatibility policy doesn't classify removing an enum value (docs/guide/cli-reference.md:774)
• ⛏ Stale or too-narrow docs and comments
  • unknown is defined as "staging or activation was attempted and failed" (spec L502), but the legacy path also uses it for a reported version it couldn't verify, including when the API shows another version active.
  • draft is described as "created" (cli-reference L829, L849, the Draft doc), but the initial-draft path reports a draft that already existed. "The target draft" would be accurate.
  • The envelope's result row (cli-reference L752, spec L158) and the Failure doc (output.rs:104-106) don't list deploy partial results.
  • cli_support.rs:92 and format_json.rs:67 still cite plain wrangler whoami as the probe.
  • Fastly cli.rs:2136: the comment implies execute(Deploy) is followed by finalize_deploy, which isn't Fastly's CLI path, since it overrides Adapter::deploy.
  • registry.rs:151: status_code "the last HTTP status observed" can be Some(0) for curl's 000.
  • registry.rs:619: the Adapter::deploy # Errors doc doesn't say that a failure after a version exists is Ok(ActionOutcome::Deploy) with failure set.
  • Spec L325-327: the list of deliberately exhaustive enums omits DeployedVersionState. Spec §9.3 (L636) says "30 tests" without noting that json_envelope_write_failure_is_reported is Linux-only.
  • Streams (cli-reference L715-722): with stdout closed (>&-), the CLI exits 0 with no envelope, because Rust's std ignores EBADF on stdout. That contradicts "an empty stdout with a non-zero exit"; worth a sentence.
  • config gc: a key whose delete failed appears in failed and also in stranded or uncertain. The table (L925-927) reads as if the lists are disjoint.
• ⛏ A missing curl gives a partial healthcheck result. It reports ok: false with attempts: 2, healthy: false and status_code: null. That contradicts attempts "the number of probes actually made" (L871) and the convention that a missing CLI gives result: null (as auth status does). Make a curl spawn failure an Err, or document it.
• 🤔 Test gaps
  • No end-to-end test of config gc with a failed delete (the GcFailure → partial result glue in config.rs).
  • No Cloudflare auth status test in JSON mode against a fake wrangler, and no exact-text test for the intended Cloudflare change. Untested probe edge cases: loggedIn missing or non-boolean, a stdout preamble, and loggedIn: true with a non-zero exit.
  • Healthcheck: 503 followed by a transport failure; a missing curl.
  • Deploy: a legacy deploy relying on FASTLY_SERVICE_ID (documented as service_id: null); a failing manifest deploy command; a non-Fastly deploy in JSON mode; version_state: published for --staging.
  • build with --format json first and another --format among the passthrough arguments (documented as writing an ok: false envelope).
  • json_envelope_write_failure_is_reported is #[cfg(target_os = "linux")] (format_json.rs:763), so the write-failure path isn't covered on macOS.
• ⛏ The PR description is stale. It cites spec "revision 2" (now 6) and "the pinned toolchain 1.95.0" (now 1.98.1), says text mode is byte-identical to main (the Cloudflare auth status change is a second exception), and its clippy.toml row lists only Command::status / Command::spawn, not std::io::stdout and CommandExt::exec.
• 🌱 A Finalize failure drops the deploy's own outcome. In lib.rs:318-321, DeployFailure::Finalize renders result(DeployOutcome::default()), discarding whatever the adapter's deploy returned (e.g. a version or package_sha256). Fastly isn't affected, since its fallback deploy returns Empty, but an adapter that reports a version from deploy and fails in finalize_deploy would lose it.

📌 Out of Scope

• Please file the two agreed follow-ups as issues so they don't get lost:
  • Under --format json, a closed stderr reader kills inheriting children with SIGPIPE, because their stdout is routed to our stderr. Text mode is unaffected, and run_shell_tee survives it.
  • Generated projects: the new clippy.toml.hbs guard is never exercised (CI only cargo checks a generated project), and a fresh project already fails strict clippy for reasons main has.

CI Status

Rust 1.98.1 (asdf), confirmed in each log:

• fmt: PASS
• clippy (--workspace --all-targets --all-features -D warnings): PASS
• tests (cargo test --workspace --all-targets): PASS (1608 passed, 0 failed, 1 ignored; format_json 29/29 on macOS)
• cargo check --workspace --all-targets --features "fastly cloudflare spin": PASS
• cargo check -p edgezero-adapter-spin --target wasm32-wasip2 --features spin: PASS
• cargo test -p edgezero-adapter-fastly --features cli: PASS (327 passed)
• Fixture workspaces tests/fixtures/reusable-app and tests/fixtures/request-fidelity-runner: clippy PASS with their own clippy.toml (each fails without it, so both files are load-bearing); both are linted in CI.
• Text mode vs main 87456c7: 512 hermetic scenarios (fake fastly / curl / wrangler / spin / cargo, including managed deploys from verified releases); 500 byte-identical, 12 declared differences.
• JSON mode: 499 runs; every one wrote exactly one envelope, and ok matched the exit code.

[aram356]

🔧 draft is reported for a version the code has just seen published

publish_attempted is the only signal used here, but two checks fail before it is set precisely when the version is no longer a draft:

• the clone check, draft.active || draft.locked || !draft.environments.is_empty() (L1174-1178);
• require_managed_draft_state (L1637-1641), called from revalidate_managed_draft just before publishing.

Both return Err with publish_attempted == false, so this arm labels the version Draft.

Reproduced with the stateful Fastly fakes: a production deploy clones v42, uploads the package, then another actor activates v42 (fastly service version activate, or the UI). Revalidation fails with "Fastly version 42 is no longer an unpublished editable draft", and the envelope is {"ok": false, "result": {…, "version": 42, "version_state": "draft"}} while the provider shows 42 active. --staging behaves the same, and a second reproduction hit it through the active-source revalidation ("active source selection changed").

The CLI reference defines draft as "created, never staged or activated" (L829), and DeployedVersionState::Draft says "Created, but never staged or activated". A recovery script that trusts draft would conclude nothing went live at the one moment EdgeZero has positive evidence that something did. The trigger is a concurrent publish, so it's rare, but this field exists to drive exactly that decision.

Fix: when a not-a-draft check fails, report Unknown (or Published when the readback shows it active or staged). For example, set a flag in ManagedProgress before returning that error and check it here. If you'd rather keep the current mapping, redefine draft everywhere as "EdgeZero never attempted to stage or activate it" and drop "never staged or activated".

[aram356]

🔧 The managed progress tracking that fixes the result: null regression has no test guarding it

In a copy of this head, deleting progress.publish_attempted = true (here) and progress.version = Some(version) (L1148) left all 327 edgezero-adapter-fastly --features cli tests and every edgezero-cli test green, while the real binary went straight back to result: null for an activate 500 and for a failed link create.

The existing tests cover the pure mapping (managed_deploy_outcome_keeps_a_created_version_on_failure) and the CLI rendering through the recording adapter, but nothing drives execute_managed_deploy_plan_with_emit itself, so the wiring between the plan executor and ManagedProgress is unproven. That wiring is the whole fix for the blocking finding from the last round.

Suggestion: add one adapter-level test that runs a managed deploy against a hand-built verified release and fake fastly / curl (the .github/actions/deploy-fastly/tests/make-fake-fastly-env.sh fakes already model the provider state). A release fixture is about 20 lines. Inject a failure after the clone and one at activate, and assert version: 42 with draft and unknown respectively. That would also catch the mislabel in the comment at L927.

[aram356]

🤔 The Cloudflare auth status change is wider than declared, and the reply says it's declared when it isn't

Switching the built-in probe to a captured wrangler whoami --json changes text mode in more ways than "no longer echoes wrangler's output". Against main 87456c7:

• Exit code: logged out with wrangler exiting 0 goes from exit 0 to exit 1. So do plain-text output (a wrangler without --json), an unknown-argument error, and a non-boolean loggedIn.
• New stdout line: [edgezero] wrangler is logged in as <email>.
• stderr: wrangler's stderr is swallowed on success and folded into the error message otherwise.

The exit-code change fixes a real bug (plain whoami exits 0 when logged out), so the behaviour is right. But it contradicts:

• this Goal 2 ("The one exception is build");
• §6.4 L206 ("Exit codes do not change");
• the CLI reference L694 ("leaves each command's output exactly as it was");
• the reference's auth section L680 ("A non-zero exit propagates with its stderr verbatim"), which is no longer true for the Cloudflare built-in;
• §7.2 L333-334 ("a non-zero exit from auth status is Unauthenticated"): with non-JSON output it now gives result: null.

The reply on the Cloudflare thread says this change "is declared in spec §9.2 and revision 6", but both only say the probe is wrangler whoami --json.

Suggestion: list the Cloudflare change as a second exception in Goal 2, §6.4, §9.2 and revision 6, with the exit-code and output changes spelled out, and scope the reference's L680 and L694 statements accordingly. The docs' Adapter built-ins table should also say wrangler whoami --json. Also: no end-to-end test runs Cloudflare auth status in JSON mode against a fake wrangler, or pins the new text bytes.

[aram356]

⛏ This error message is malformed, and a few probe edge cases need a decision

• {} with output.status renders as "exited with status exit status: 0 without reporting loggedIn: ". When stderr is empty it ends with a dangling : , and it never shows stdout, which is where the unparseable output was. Print the exit code (output.status.code()) and a bounded stdout excerpt.
• With real wrangler 4.131.0 and an invalid token, wrangler colours its stderr even when piped, so error.message in the JSON envelope carries raw ANSI escape sequences. Setting NO_COLOR=1 / FORCE_COLOR=0 on the child, or stripping ANSI, would keep the JSON clean.
• {"loggedIn": true} with a non-zero exit is reported as authenticated with exit 0; main exited non-zero there. Real wrangler only prints loggedIn: true on success, so this is an edge case, but requiring output.status.success() for Authenticated costs nothing.
• parse_whoami parses the whole of stdout, so any notice wrangler prints before the JSON makes a logged-in user's probe fail. wrangler 4.131.0 prints pure JSON today. Taking the last {…} block, or the last line that parses, would make this robust to a future banner.

[aram356]

🤔 result: null doesn't mean nothing was created

Two paths give result: null after the provider has changed:

• Legacy: a manifest deploy command or built-in fastly compute deploy that exits non-zero after it may have activated. Reproduced: deploy = "echo version=12; exit 1" prints version=12, and the envelope says result: null.
• Managed: a clone PUT that committed but whose response failed in transit or didn't parse. progress.version isn't set until parse_cloned_version succeeds, so draft 42 exists on the provider and the envelope says result: null.

Neither is a regression (text output matches main), but this sentence, the DeployFailure::Deploy doc ("failed before reporting anything it created", adapter.rs:75) and the comment at fastly cli.rs:913-914 ("nothing was created") overclaim.

Suggestion: "result: null means EdgeZero has no version to report. A deploy command that failed, or a clone request whose response could not be read, may still have changed the service." Spec §8 (L521) needs the same wording.

[aram356]

⛏ The status_code docs contradict the behaviour and each other

Implementation: probe_with_retries keeps the last parsed HTTP code, transport errors don't reset it, and HealthcheckResult::new maps 0 (curl's 000) to null. So:

• 503 then a transport failure (curl exit 7) gives status_code: 503. That contradicts L879 and spec L542 ("null when the last probe got no response").
• 503 then 000 (curl exit 0) gives status_code: null. That contradicts this row ("null when no probe got a response"), which is the old revision-4 wording.

Revision 4 (spec L777-779) also still describes the reverted adapter-side mapping.

Suggested wording: "The last HTTP status any probe received; null if none did, or if the last parsed status was curl's 000." Add an end-to-end test for the 503-then-transport-failure case. probe_with_retries_reports_the_last_code_of_a_mixed_run passes against 392546a too, because the old mapping lived in curl_status, which it bypasses, so it doesn't guard the regression it was added for.

[aram356]

♻️ version and version_state can disagree

The rule "version_state is None when version is" lives only in this comment and in the docs ("null exactly when version is"). DeployOutcome exposes both as independent public Options (and derives Default), and DeployResult::new passes them through unchanged. The PR's own fallback_deploy_prefers_the_finalizer_outcome builds version: Some(1), version_state: None, and any adapter can return the reverse.

Modelling it as one field makes the invalid combinations unrepresentable:

pub struct DeployedVersion {
    pub number: u64,
    pub state: DeployedVersionState,
}

pub struct DeployOutcome {
    pub failure: Option<String>,
    pub package_sha256: Option<String>,
    pub service_id: Option<String>,
    /// The version the deploy created, staged or activated, when known.
    pub version: Option<DeployedVersion>,
}

The wire struct can keep its two flat keys. If you'd rather not change the type, normalising in DeployResult::new (drop the state when there's no version) would at least keep the JSON consistent.

[aram356]

⛏ The compatibility policy doesn't classify removing or narrowing an enum value

This PR removes updated / would_update from provision.entries[].action and narrows store.logical before v1 ships, which is fine. But the policy lists "adding a value to a string enum" as non-breaking and says nothing about removing one, so the next person to drop a value has no rule to follow. Suggest adding "removing a value from a string enum" to the breaking list here and in spec §6.5 (L228-233). It's also worth a one-line note in the schema changelog that v1 was narrowed before its first release.