From 053316bcc723eab06a71ff83b1f4dda3ad1631d0 Mon Sep 17 00:00:00 2001 From: graysurf <10785178+graysurf@users.noreply.github.com> Date: Wed, 20 May 2026 10:37:35 +0800 Subject: [PATCH 1/3] feat(forge-cli): add pr.deliver macro and per-atom compute helpers - Add compute() helpers to auth.status / repo.view / pr.create / pr.wait-checks / pr.ready / pr.merge so the macro captures each step's typed payload without re-spawning forge-cli as a child; pr.wait-checks also exposes WaitOutcome so the caller decides success / failed / timed_out instead of going through the envelope emitters. - Add crates/forge-cli/src/macros/pr_deliver.rs orchestrating the six atoms per spec sequence with short-circuit on failure (later steps omitted from data.steps[], outer exit code matches the failing atom), plus a dry-run path that lists every step in data.plan_steps. --- completions/bash/forge-cli | 38 +- completions/zsh/_forge-cli | 11 + crates/forge-cli/src/cli.rs | 63 +- crates/forge-cli/src/lib.rs | 1 + crates/forge-cli/src/macros/mod.rs | 6 + crates/forge-cli/src/macros/pr_deliver.rs | 687 ++++++++++++++++++ crates/forge-cli/src/ops/auth_status.rs | 25 +- crates/forge-cli/src/ops/pr_create.rs | 69 +- crates/forge-cli/src/ops/pr_merge.rs | 58 +- crates/forge-cli/src/ops/pr_ready.rs | 34 +- crates/forge-cli/src/ops/pr_wait_checks.rs | 93 ++- crates/forge-cli/src/ops/repo_view.rs | 18 +- crates/forge-cli/tests/integration.rs | 1 + .../forge-cli/tests/integration/pr_deliver.rs | 162 +++++ 14 files changed, 1195 insertions(+), 71 deletions(-) create mode 100644 crates/forge-cli/src/macros/mod.rs create mode 100644 crates/forge-cli/src/macros/pr_deliver.rs create mode 100644 crates/forge-cli/tests/integration/pr_deliver.rs diff --git a/completions/bash/forge-cli b/completions/bash/forge-cli index d10b0d83..a73736a0 100644 --- a/completions/bash/forge-cli +++ b/completions/bash/forge-cli @@ -1348,12 +1348,48 @@ _forge-cli() { return 0 ;; forge__cli__pr__deliver) - opts="-h --format --remote --provider --repo --dry-run --help" + opts="-h --kind --title --body --body-file --head --base --method --reviewer --timeout --no-merge --allow-non-default-base --format --remote --provider --repo --dry-run --help" if [[ ${cur} == -* || ${COMP_CWORD} -eq 3 ]] ; then COMPREPLY=( $(compgen -W "${opts}" -- "${cur}") ) return 0 fi case "${prev}" in + --kind) + COMPREPLY=($(compgen -W "feature bug" -- "${cur}")) + return 0 + ;; + --title) + COMPREPLY=($(compgen -f "${cur}")) + return 0 + ;; + --body) + COMPREPLY=($(compgen -f "${cur}")) + return 0 + ;; + --body-file) + COMPREPLY=($(compgen -f "${cur}")) + return 0 + ;; + --head) + COMPREPLY=($(compgen -f "${cur}")) + return 0 + ;; + --base) + COMPREPLY=($(compgen -f "${cur}")) + return 0 + ;; + --method) + COMPREPLY=($(compgen -W "squash merge rebase" -- "${cur}")) + return 0 + ;; + --reviewer) + COMPREPLY=($(compgen -f "${cur}")) + return 0 + ;; + --timeout) + COMPREPLY=($(compgen -f "${cur}")) + return 0 + ;; --format) COMPREPLY=($(compgen -W "text json" -- "${cur}")) return 0 diff --git a/completions/zsh/_forge-cli b/completions/zsh/_forge-cli index e17a8a18..fa2173cc 100644 --- a/completions/zsh/_forge-cli +++ b/completions/zsh/_forge-cli @@ -213,11 +213,22 @@ json\:"Single-record JSON envelope (snake_case)"))' \ ;; (deliver) _arguments "${_arguments_options[@]}" : \ +'--kind=[PR / MR kind (selects branch-prefix rule). Required]:KIND:(feature bug)' \ +'--title=[PR / MR title (required)]:TITLE:_default' \ +'(--body-file)--body=[PR / MR body / description text. Mutually exclusive with \`--body-file\`]:BODY:_default' \ +'--body-file=[Read PR / MR body from a file. Use \`-\` to read from stdin]:PATH:_default' \ +'--head=[Source branch (defaults to the current branch)]:HEAD:_default' \ +'--base=[Target / base branch (defaults to the repo'\''s default branch)]:BASE:_default' \ +'--method=[Merge method (default\: squash)]:METHOD:(squash merge rebase)' \ +'*--reviewer=[Add a reviewer (repeatable)]:USER:_default' \ +'--timeout=[CI-wait budget before declaring \`checks_timeout\` (default \`30m\`)]:TIMEOUT:_default' \ '--format=[Output format (defaults to text)]:FORMAT:((text\:"Human-readable text output (default)" json\:"Single-record JSON envelope (snake_case)"))' \ '--remote=[Git remote whose URL feeds provider detection (default\: \`origin\`)]:REMOTE:_default' \ '--provider=[Override auto-detected provider]:PROVIDER:(github gitlab)' \ '--repo=[Override the repo slug (\`owner/name\`). When absent it is derived from the remote URL]:owner/name:_default' \ +'--no-merge[Stop after \`pr.wait-checks\` — do not promote to ready or merge]' \ +'--allow-non-default-base[Allow merges where the PR'\''s base is not the repo'\''s default branch]' \ '--dry-run[Render the backend command that would run, without invoking it. The envelope'\''s \`data.plan\` carries the exact argv]' \ '-h[Print help (see more with '\''--help'\'')]' \ '--help[Print help (see more with '\''--help'\'')]' \ diff --git a/crates/forge-cli/src/cli.rs b/crates/forge-cli/src/cli.rs index 76538a6d..df574420 100644 --- a/crates/forge-cli/src/cli.rs +++ b/crates/forge-cli/src/cli.rs @@ -11,7 +11,7 @@ use std::time::Duration; use clap::{ArgAction, Args, Parser, Subcommand, ValueEnum}; use clap_complete::Shell; -use nils_common::cli_contract::{OutputFormat, emit_parse_error, exit, schema_version_for}; +use nils_common::cli_contract::{OutputFormat, emit_parse_error, exit}; use crate::error::ForgeError; use crate::ops; @@ -161,6 +161,13 @@ impl PrKindFlag { PrKindFlag::Bug => crate::validations::PrKind::Bug, } } + + pub fn as_str(self) -> &'static str { + match self { + PrKindFlag::Feature => "feature", + PrKindFlag::Bug => "bug", + } + } } /// `--state` filter shared by `pr list` and the close payload normaliser. @@ -421,7 +428,46 @@ pub enum PrCommand { /// Block until every required check reaches a terminal state. WaitChecks(PrWaitChecksArgs), /// End-to-end "open draft → CI green → ready → merge" macro. - Deliver, + Deliver(PrDeliverArgs), +} + +/// `pr deliver` arguments. Maps to +/// `forge-cli-ops-v1.yaml::operations.pr.deliver` inputs. +#[derive(Args, Debug, Clone)] +pub struct PrDeliverArgs { + /// PR / MR kind (selects branch-prefix rule). Required. + #[arg(long, value_enum)] + pub kind: PrKindFlag, + /// PR / MR title (required). + #[arg(long)] + pub title: String, + /// PR / MR body / description text. Mutually exclusive with `--body-file`. + #[arg(long, conflicts_with = "body_file")] + pub body: Option, + /// Read PR / MR body from a file. Use `-` to read from stdin. + #[arg(long = "body-file", value_name = "PATH")] + pub body_file: Option, + /// Source branch (defaults to the current branch). + #[arg(long)] + pub head: Option, + /// Target / base branch (defaults to the repo's default branch). + #[arg(long)] + pub base: Option, + /// Merge method (default: squash). + #[arg(long, value_enum, default_value_t = MergeMethodFlag::Squash)] + pub method: MergeMethodFlag, + /// Add a reviewer (repeatable). + #[arg(long = "reviewer", value_name = "USER")] + pub reviewers: Vec, + /// CI-wait budget before declaring `checks_timeout` (default `30m`). + #[arg(long, value_parser = parse_duration, default_value = "30m")] + pub timeout: Duration, + /// Stop after `pr.wait-checks` — do not promote to ready or merge. + #[arg(long = "no-merge", action = ArgAction::SetTrue)] + pub no_merge: bool, + /// Allow merges where the PR's base is not the repo's default branch. + #[arg(long = "allow-non-default-base", action = ArgAction::SetTrue)] + pub allow_non_default_base: bool, } /// `issue` subtree. @@ -584,6 +630,9 @@ pub fn dispatch(args: Vec) -> i32 { Some(Command::Pr(PrArgs { command: Some(PrCommand::Merge(args)), })) => ops::pr_merge::run(&global, args, format), + Some(Command::Pr(PrArgs { + command: Some(PrCommand::Deliver(args)), + })) => crate::macros::pr_deliver::run(&global, args, format), Some(Command::Issue(IssueArgs { command: Some(IssueCommand::Create(args)), })) => ops::issue_create::run(&global, args, format), @@ -613,13 +662,6 @@ pub fn dispatch(args: Vec) -> i32 { let _ = ::command().print_help(); return exit::USAGE; } - // Every other subcommand declared above is part of the v1 surface but - // not implemented in Sprint 1. The structured failure keeps callers on - // the contract instead of panicking on `todo!()`. - _ => Err(ForgeError::not_implemented( - schema_version_for(BINARY, "error", 1), - "subcommand not implemented in this sprint", - )), }; match result { @@ -818,6 +860,9 @@ mod tests { "create" => { argv.extend(["--title", "demo", "--kind", "feature", "--body", "x"]); } + "deliver" => { + argv.extend(["--kind", "feature", "--title", "demo"]); + } _ => {} } let result = parse(&argv); diff --git a/crates/forge-cli/src/lib.rs b/crates/forge-cli/src/lib.rs index 9a75bc04..941c0dd8 100644 --- a/crates/forge-cli/src/lib.rs +++ b/crates/forge-cli/src/lib.rs @@ -13,6 +13,7 @@ pub mod config; pub mod envelope; pub mod error; pub mod glab_version; +pub mod macros; pub mod ops; pub mod provider; pub mod validations; diff --git a/crates/forge-cli/src/macros/mod.rs b/crates/forge-cli/src/macros/mod.rs new file mode 100644 index 00000000..864e99c9 --- /dev/null +++ b/crates/forge-cli/src/macros/mod.rs @@ -0,0 +1,6 @@ +//! Macro orchestrators that compose multiple atoms into a single envelope. +//! +//! Spec / ops: `cli.forge-cli.pr.deliver.v1` is the only macro in v1; future +//! macros (e.g. `issue deliver`) will live here too. + +pub mod pr_deliver; diff --git a/crates/forge-cli/src/macros/pr_deliver.rs b/crates/forge-cli/src/macros/pr_deliver.rs new file mode 100644 index 00000000..cb7a9b28 --- /dev/null +++ b/crates/forge-cli/src/macros/pr_deliver.rs @@ -0,0 +1,687 @@ +//! `pr deliver` macro — the v1 lifecycle composition. +//! +//! Spec / ops: `cli.forge-cli.pr.deliver.v1`. Sequence per spec §"Macro: +//! pr deliver": +//! +//! ```text +//! auth.status → repo.view → pr.create → pr.wait-checks +//! → pr.ready (skip if --no-merge) +//! → pr.merge (skip if --no-merge) +//! ``` +//! +//! Each step's typed payload is captured via the atom's `compute` helper +//! (no subprocess re-spawn through a child binary) and appended to +//! `data.steps[]`. A failing step short-circuits — later steps are omitted +//! from `data.steps[]` and the macro's outer exit code equals the failing +//! atom's exit code (no remapping). + +use std::path::{Path, PathBuf}; + +use nils_common::cli_contract::{Envelope, EnvelopeError, OutputFormat, schema_version_for}; +use serde::Serialize; +use serde_json::Value; + +use crate::backend::{BackendCall, BackendProgram, BackendRunner, ProcessRunner}; +use crate::cli::{BINARY, GlobalFlags, PrCreateArgs, PrDeliverArgs, PrMergeArgs, PrWaitChecksArgs}; +use crate::config::ForgeConfig; +use crate::error::ForgeError; +use crate::ops::pr_create::{self, Environment}; +use crate::ops::pr_wait_checks::{Clock, SystemClock, WaitOutcome}; +use crate::ops::{auth_status, pr_merge, pr_ready, pr_wait_checks, repo_view}; +use crate::provider::{Provider, ProviderContext, detect, git_remote_url}; +use crate::validations::git_status_porcelain; + +pub const SCHEMA: &str = "pr.deliver"; +pub const SCHEMA_VERSION: u32 = 1; + +/// Composite envelope payload. +#[derive(Debug, Clone, Serialize)] +pub struct PrDeliverPayload { + pub kind: &'static str, + pub provider: &'static str, + pub pr: PrDeliverSummary, + pub steps: Vec, +} + +/// Summary of the resulting PR, mirroring the spec's `data.pr` shape. +#[derive(Debug, Clone, Serialize)] +pub struct PrDeliverSummary { + pub number: u64, + pub url: String, + pub merged: bool, + #[serde(skip_serializing_if = "Option::is_none")] + pub merge_sha: Option, +} + +/// One step in the macro sequence. `payload` carries the underlying atom's +/// typed data as a JSON value so consumers can grep through the chain +/// without depending on every atom's Rust type. +#[derive(Debug, Clone, Serialize)] +pub struct Step { + pub step: &'static str, + pub ok: bool, + pub schema_version: String, + pub payload: Value, +} + +/// Dry-run envelope. `plan_steps[]` enumerates every atom that would run +/// in this invocation; the macro never spawns a backend in dry-run. +#[derive(Debug, Clone, Serialize)] +pub struct PrDeliverDryRun { + pub provider: &'static str, + pub kind: &'static str, + pub plan_steps: Vec, + pub no_merge: bool, +} + +#[derive(Debug, Clone, Serialize)] +pub struct DryRunStep { + pub step: &'static str, + pub plan: Vec, +} + +pub fn run( + global: &GlobalFlags, + args: PrDeliverArgs, + format: OutputFormat, +) -> Result { + let runner = ProcessRunner; + let clock = SystemClock; + let workdir = std::env::current_dir().unwrap_or_else(|_| PathBuf::from(".")); + run_with(&runner, &clock, global, args, format, &workdir) +} + +pub fn run_with( + runner: &R, + clock: &C, + global: &GlobalFlags, + args: PrDeliverArgs, + format: OutputFormat, + workdir: &Path, +) -> Result { + let ctx = detect(global.provider_hint(), &global.remote, git_remote_url)?; + + if global.dry_run { + return Ok(emit_dry_run(&ctx, &args, format)); + } + + execute_sequence(runner, clock, global, &ctx, &args, format, workdir) +} + +fn execute_sequence( + runner: &R, + clock: &C, + global: &GlobalFlags, + ctx: &ProviderContext, + args: &PrDeliverArgs, + format: OutputFormat, + workdir: &Path, +) -> Result { + let mut steps: Vec = Vec::new(); + let mut merged = false; + let mut merge_sha: Option = None; + + // 1. auth.status + let auth_payload = match auth_status::compute(runner, global, git_remote_url) { + Ok(p) => p, + Err(err) => { + return Ok(emit_chain_failure(steps, args, ctx, None, &err, format)); + } + }; + steps.push(Step { + step: "auth_status", + ok: true, + schema_version: schema_version_for(BINARY, "auth.status", 1), + payload: to_value(&auth_payload), + }); + + // 2. repo.view + let repo_payload = match repo_view::compute(runner, ctx, global.repo.as_deref()) { + Ok(p) => p, + Err(err) => { + return Ok(emit_chain_failure(steps, args, ctx, None, &err, format)); + } + }; + steps.push(Step { + step: "repo_view", + ok: true, + schema_version: schema_version_for(BINARY, "repo.view", 1), + payload: to_value(&repo_payload), + }); + + // 3. pr.create + let create_args = build_create_args(args, &repo_payload.default_branch); + let env = Environment::production(); + let create_payload = match pr_create::compute(runner, global, &create_args, &env) { + Ok(p) => p, + Err(err) => { + return Ok(emit_chain_failure(steps, args, ctx, None, &err, format)); + } + }; + let pr_number = create_payload.number; + let pr_url = create_payload.url.clone(); + steps.push(Step { + step: "create", + ok: true, + schema_version: schema_version_for(BINARY, "pr.create", 1), + payload: to_value(&create_payload), + }); + + // 4. pr.wait-checks + let wait_args = PrWaitChecksArgs { + id: pr_number.to_string(), + timeout: args.timeout, + interval: std::time::Duration::from_secs(20), + required_only: true, + }; + match pr_wait_checks::compute(runner, clock, global, ctx, &wait_args) { + Ok(WaitOutcome::Success(snapshot)) => { + steps.push(Step { + step: "wait_checks", + ok: true, + schema_version: schema_version_for(BINARY, "pr.checks", 1), + payload: to_value(&snapshot), + }); + } + Ok(WaitOutcome::Failed(snapshot)) => { + let err = ForgeError::runtime_failure( + schema_version_for(BINARY, "pr.checks", 1), + "checks_failed", + "required checks did not reach success", + None, + ); + steps.push(Step { + step: "wait_checks", + ok: false, + schema_version: schema_version_for(BINARY, "pr.checks", 1), + payload: to_value(&snapshot), + }); + return Ok(emit_chain_failure( + steps, + args, + ctx, + Some((pr_number, pr_url)), + &err, + format, + )); + } + Ok(WaitOutcome::TimedOut(snapshot)) => { + let err = ForgeError::unavailable( + schema_version_for(BINARY, "pr.checks", 1), + "checks_timeout", + "deadline reached before required checks became terminal", + None, + ); + steps.push(Step { + step: "wait_checks", + ok: false, + schema_version: schema_version_for(BINARY, "pr.checks", 1), + payload: to_value(&snapshot), + }); + return Ok(emit_chain_failure( + steps, + args, + ctx, + Some((pr_number, pr_url)), + &err, + format, + )); + } + Err(err) => { + return Ok(emit_chain_failure( + steps, + args, + ctx, + Some((pr_number, pr_url)), + &err, + format, + )); + } + } + + if args.no_merge { + // Macro ends after wait-checks when --no-merge is set; outer exit + // code matches `pr.wait-checks` success (0). + return Ok(emit_success_envelope( + steps, args, ctx, pr_number, pr_url, merged, merge_sha, format, + )); + } + + // 5. pr.ready + let ready_payload = + match pr_ready::compute(runner, ctx, pr_number, workdir, git_status_porcelain) { + Ok(p) => p, + Err(err) => { + return Ok(emit_chain_failure( + steps, + args, + ctx, + Some((pr_number, pr_url)), + &err, + format, + )); + } + }; + steps.push(Step { + step: "ready", + ok: true, + schema_version: schema_version_for(BINARY, "pr.ready", 1), + payload: to_value(&ready_payload), + }); + + // 6. pr.merge + let merge_args = PrMergeArgs { + id: pr_number, + method: Some(args.method), + keep_branch: false, + allow_non_default_base: args.allow_non_default_base, + }; + let merge_payload = match pr_merge::compute(runner, global, &merge_args, workdir) { + Ok(p) => p, + Err(err) => { + return Ok(emit_chain_failure( + steps, + args, + ctx, + Some((pr_number, pr_url)), + &err, + format, + )); + } + }; + merge_sha = Some(merge_payload.merge_sha.clone()); + merged = true; + steps.push(Step { + step: "merge", + ok: true, + schema_version: schema_version_for(BINARY, "pr.merge", 1), + payload: to_value(&merge_payload), + }); + + Ok(emit_success_envelope( + steps, args, ctx, pr_number, pr_url, merged, merge_sha, format, + )) +} + +fn build_create_args(args: &PrDeliverArgs, default_branch: &str) -> PrCreateArgs { + PrCreateArgs { + head: args.head.clone(), + base: args + .base + .clone() + .or_else(|| Some(default_branch.to_string())), + title: args.title.clone(), + body: args.body.clone(), + body_file: args.body_file.clone(), + kind: args.kind, + no_draft: false, + reviewers: args.reviewers.clone(), + labels: Vec::new(), + } +} + +fn emit_dry_run(ctx: &ProviderContext, args: &PrDeliverArgs, format: OutputFormat) -> i32 { + let mut plan_steps = vec![ + DryRunStep { + step: "auth_status", + plan: BackendCall::new( + BackendProgram::for_provider(ctx.provider), + [ + std::ffi::OsString::from("auth"), + std::ffi::OsString::from("status"), + ], + ) + .plan_argv(), + }, + DryRunStep { + step: "repo_view", + plan: repo_view_dry_plan(ctx), + }, + DryRunStep { + step: "create", + plan: pr_create_dry_plan(ctx, args), + }, + DryRunStep { + step: "wait_checks", + plan: pr_wait_checks_dry_plan(ctx), + }, + ]; + if !args.no_merge { + plan_steps.push(DryRunStep { + step: "ready", + plan: pr_ready_dry_plan(ctx), + }); + plan_steps.push(DryRunStep { + step: "merge", + plan: pr_merge_dry_plan(ctx, args), + }); + } + let payload = PrDeliverDryRun { + provider: ctx.provider.as_str(), + kind: args.kind.as_str(), + plan_steps, + no_merge: args.no_merge, + }; + let envelope = Envelope::success(schema_version_for(BINARY, SCHEMA, SCHEMA_VERSION), payload); + write_envelope(&envelope, format); + nils_common::cli_contract::exit::SUCCESS +} + +fn repo_view_dry_plan(ctx: &ProviderContext) -> Vec { + let mut call = BackendCall::new( + BackendProgram::for_provider(ctx.provider), + match ctx.provider { + Provider::GitHub => vec![ + std::ffi::OsString::from("repo"), + std::ffi::OsString::from("view"), + std::ffi::OsString::from("--json"), + ], + Provider::GitLab => vec![ + std::ffi::OsString::from("repo"), + std::ffi::OsString::from("view"), + std::ffi::OsString::from("-F"), + std::ffi::OsString::from("json"), + ], + }, + ); + if let Provider::GitHub = ctx.provider { + call.argv.push(std::ffi::OsString::from( + "name,owner,defaultBranchRef,mergeCommitAllowed,squashMergeAllowed,rebaseMergeAllowed,url", + )); + } + call.plan_argv() +} + +fn pr_create_dry_plan(ctx: &ProviderContext, args: &PrDeliverArgs) -> Vec { + let head = args + .head + .clone() + .unwrap_or_else(|| "".into()); + let base = args.base.clone().unwrap_or_else(|| "".into()); + let mut argv = match ctx.provider { + Provider::GitHub => vec![ + "pr", + "create", + "--head", + &head, + "--base", + &base, + "--title", + &args.title, + "--draft", + ], + Provider::GitLab => vec![ + "mr", + "create", + "--source-branch", + &head, + "--target-branch", + &base, + "--title", + &args.title, + "--draft", + ], + }; + if args.body.is_some() { + argv.extend_from_slice(&["--body", ""]); + } else if args.body_file.is_some() { + argv.extend_from_slice(&["--body-file", ""]); + } + let mut out = vec![ + BackendProgram::for_provider(ctx.provider) + .default_executable() + .to_string(), + ]; + out.extend(argv.into_iter().map(String::from)); + out +} + +fn pr_wait_checks_dry_plan(ctx: &ProviderContext) -> Vec { + let prog = BackendProgram::for_provider(ctx.provider).default_executable(); + match ctx.provider { + Provider::GitHub => vec![ + prog.into(), + "pr".into(), + "checks".into(), + "".into(), + "--json".into(), + "name,state,conclusion,bucket,workflow,link,startedAt,completedAt,description,isRequired".into(), + ], + Provider::GitLab => vec![ + prog.into(), + "ci".into(), + "status".into(), + "-b".into(), + "".into(), + ], + } +} + +fn pr_ready_dry_plan(ctx: &ProviderContext) -> Vec { + let prog = BackendProgram::for_provider(ctx.provider).default_executable(); + match ctx.provider { + Provider::GitHub => vec![ + prog.into(), + "pr".into(), + "ready".into(), + "".into(), + ], + Provider::GitLab => vec![ + prog.into(), + "mr".into(), + "update".into(), + "".into(), + "--ready".into(), + ], + } +} + +fn pr_merge_dry_plan(ctx: &ProviderContext, args: &PrDeliverArgs) -> Vec { + let prog = BackendProgram::for_provider(ctx.provider).default_executable(); + let method = args.method.into_method(); + let mut out: Vec = match ctx.provider { + Provider::GitHub => vec![ + prog.into(), + "pr".into(), + "merge".into(), + "".into(), + format!("--{}", method.as_str()), + ], + Provider::GitLab => { + let mut v = vec![ + prog.to_string(), + "mr".into(), + "merge".into(), + "".into(), + ]; + if matches!(method, crate::config::MergeMethod::Squash) { + v.push("--squash".into()); + } + v + } + }; + let cfg = ForgeConfig::default(); + if cfg.resolve_delete_branch(None) { + match ctx.provider { + Provider::GitHub => out.push("--delete-branch".into()), + Provider::GitLab => out.push("--remove-source-branch".into()), + } + } + out +} + +#[allow(clippy::too_many_arguments)] +fn emit_success_envelope( + steps: Vec, + args: &PrDeliverArgs, + ctx: &ProviderContext, + number: u64, + url: String, + merged: bool, + merge_sha: Option, + format: OutputFormat, +) -> i32 { + let payload = PrDeliverPayload { + kind: args.kind.as_str(), + provider: ctx.provider.as_str(), + pr: PrDeliverSummary { + number, + url, + merged, + merge_sha, + }, + steps, + }; + let envelope = Envelope::success(schema_version_for(BINARY, SCHEMA, SCHEMA_VERSION), payload); + write_envelope(&envelope, format); + nils_common::cli_contract::exit::SUCCESS +} + +fn emit_chain_failure( + steps: Vec, + args: &PrDeliverArgs, + ctx: &ProviderContext, + pr: Option<(u64, String)>, + err: &ForgeError, + format: OutputFormat, +) -> i32 { + let (number, url) = pr.unwrap_or((0, String::new())); + let payload = PrDeliverPayload { + kind: args.kind.as_str(), + provider: ctx.provider.as_str(), + pr: PrDeliverSummary { + number, + url, + merged: false, + merge_sha: None, + }, + steps, + }; + let envelope = Envelope { + schema_version: schema_version_for(BINARY, SCHEMA, SCHEMA_VERSION), + ok: false, + data: Some(payload), + warnings: Vec::new(), + error: Some(EnvelopeError::new(err.kind(), err.to_string())), + }; + write_envelope(&envelope, format); + err.exit_code() +} + +fn write_envelope(envelope: &Envelope, format: OutputFormat) { + match format { + OutputFormat::Json => { + let serialized = + serde_json::to_string(envelope).unwrap_or_else(|_| String::from("{\"ok\":false}")); + println!("{serialized}"); + } + OutputFormat::Text => { + // Text mode: minimal one-liner so consumers know which step + // ended the macro. Detailed payload is JSON-only. + if envelope.ok + && let Some(payload) = envelope.data.as_ref() + && let Ok(value) = serde_json::to_value(payload) + && let Some(steps) = value.get("steps").and_then(|s| s.as_array()) + { + println!("delivered: {} steps", steps.len()); + } else if !envelope.ok + && let Some(err) = envelope.error.as_ref() + { + eprintln!("error: {}: {}", err.code, err.message); + } + } + } +} + +fn to_value(value: &T) -> Value { + serde_json::to_value(value).unwrap_or(Value::Null) +} + +#[cfg(test)] +mod tests { + use super::*; + use crate::cli::{MergeMethodFlag, PrKindFlag}; + use crate::provider::DetectionSource; + use pretty_assertions::assert_eq; + + fn ctx() -> ProviderContext { + ProviderContext { + provider: Provider::GitHub, + host: "github.com".into(), + source: DetectionSource::Flag, + } + } + + fn args(no_merge: bool) -> PrDeliverArgs { + PrDeliverArgs { + kind: PrKindFlag::Feature, + title: "demo".into(), + body: Some("## Summary\nx\n\n## Test plan\ny".into()), + body_file: None, + head: Some("feat/demo".into()), + base: Some("main".into()), + method: MergeMethodFlag::Squash, + reviewers: Vec::new(), + timeout: std::time::Duration::from_secs(30 * 60), + no_merge, + allow_non_default_base: false, + } + } + + #[test] + fn dry_run_full_chain_lists_six_steps() { + let format = OutputFormat::Json; + // Capture stdout would complicate this test; instead validate the + // plan_steps[] vector via the dry-run helper directly. + let mut plan_steps = Vec::new(); + let c = ctx(); + let a = args(false); + plan_steps.push(DryRunStep { + step: "auth_status", + plan: vec!["auth".into(), "status".into()], + }); + plan_steps.push(DryRunStep { + step: "repo_view", + plan: repo_view_dry_plan(&c), + }); + plan_steps.push(DryRunStep { + step: "create", + plan: pr_create_dry_plan(&c, &a), + }); + plan_steps.push(DryRunStep { + step: "wait_checks", + plan: pr_wait_checks_dry_plan(&c), + }); + plan_steps.push(DryRunStep { + step: "ready", + plan: pr_ready_dry_plan(&c), + }); + plan_steps.push(DryRunStep { + step: "merge", + plan: pr_merge_dry_plan(&c, &a), + }); + assert_eq!(plan_steps.len(), 6); + // Skip-when --no-merge collapses to 4 steps. + let _ = format; + } + + #[test] + fn no_merge_dry_plan_excludes_ready_and_merge() { + let c = ctx(); + let a = args(true); + let _plan = pr_merge_dry_plan(&c, &a); // smoke test + // The actual emission path filters ready/merge when no_merge is true; + // that branch is covered by the integration test asserting plan_steps + // length is 4 in that mode. + assert!(a.no_merge); + } + + #[test] + fn build_create_args_falls_back_to_repo_default_branch() { + let a = args(false); + let pc = build_create_args(&a, "main"); + assert_eq!(pc.base.as_deref(), Some("main")); + assert_eq!(pc.title, "demo"); + assert!(!pc.no_draft, "deliver always creates draft"); + } +} diff --git a/crates/forge-cli/src/ops/auth_status.rs b/crates/forge-cli/src/ops/auth_status.rs index cbf76bde..68ec5d8f 100644 --- a/crates/forge-cli/src/ops/auth_status.rs +++ b/crates/forge-cli/src/ops/auth_status.rs @@ -45,9 +45,9 @@ pub fn run_with Option>( remote_url_lookup: F, ) -> Result { let ctx = crate::provider::detect(global.provider_hint(), &global.remote, remote_url_lookup)?; - let call = build_call(&ctx); if global.dry_run { + let call = build_call(&ctx); let payload = DryRunPayload::new(ctx.provider, &call); return Ok(emit_success( schema_version_for(BINARY, SCHEMA, SCHEMA_VERSION), @@ -57,8 +57,7 @@ pub fn run_with Option>( )); } - let output = runner.run(&call)?; - let payload = parse_backend_output(&ctx, &output)?; + let payload = compute_with_ctx(runner, &ctx)?; Ok(emit_success( schema_version_for(BINARY, SCHEMA, SCHEMA_VERSION), payload, @@ -67,6 +66,26 @@ pub fn run_with Option>( )) } +/// Macro-facing entry point: compute the payload without emitting an +/// envelope. Used by `pr deliver` to capture each step's typed output. +pub fn compute Option>( + runner: &R, + global: &GlobalFlags, + remote_url_lookup: F, +) -> Result { + let ctx = crate::provider::detect(global.provider_hint(), &global.remote, remote_url_lookup)?; + compute_with_ctx(runner, &ctx) +} + +fn compute_with_ctx( + runner: &R, + ctx: &ProviderContext, +) -> Result { + let call = build_call(ctx); + let output = runner.run(&call)?; + parse_backend_output(ctx, &output) +} + fn build_call(ctx: &ProviderContext) -> BackendCall { let program = BackendProgram::for_provider(ctx.provider); let argv: Vec = vec![OsString::from("auth"), OsString::from("status")]; diff --git a/crates/forge-cli/src/ops/pr_create.rs b/crates/forge-cli/src/ops/pr_create.rs index 7ceed47e..2c35341d 100644 --- a/crates/forge-cli/src/ops/pr_create.rs +++ b/crates/forge-cli/src/ops/pr_create.rs @@ -200,17 +200,68 @@ pub fn run_with( })); } - // 5. Create + parse the URL the backend prints back. - let create_output = runner.run(&create_call)?; - let (number, url) = parse_create_output(&ctx, &create_output)?; + let payload = create_and_fetch(runner, &ctx, &create_call, kind)?; + Ok(emit_success(schema_ok(), payload, format, render_text)) +} - // 6. Re-fetch full metadata via the view follow-up so the envelope - // payload matches the spec's `data` shape exactly. - let view_call = build_view_call(&ctx, number); - let view_output = runner.run(&view_call)?; - let payload = parse_view_output(&ctx, &view_output, number, url, kind)?; +/// Macro-facing entry point: run the full validation chain + backend create + +/// view re-fetch and return the typed payload. The `env` argument supplies +/// the same hooks that `run_with` uses so the macro can inject test stubs. +pub fn compute( + runner: &R, + global: &GlobalFlags, + args: &PrCreateArgs, + env: &Environment<'_>, +) -> Result { + let ctx = detect(global.provider_hint(), &global.remote, |r| { + (env.remote_url)(r) + })?; + let head = match args.head.clone() { + Some(h) => h, + None => (env.current_branch)()?, + }; + let base = match args.base.clone() { + Some(b) => b, + None => (env.default_branch)(runner as &dyn BackendRunner, &ctx)?, + }; + let body = read_body(args.body.as_deref(), args.body_file.as_deref())?; + let prefix = branch_name(&head)?; + let kind: PrKind = args.kind.into_kind(); + branch_kind_matches(prefix, kind)?; + title_length(&args.title)?; + body_summary(&body, &env.headings)?; + body_test_plan(&body, &env.headings)?; + worktree_clean(&env.workdir, |w| (env.git_status)(w))?; + head_pushed(&env.workdir, |w| (env.head_state)(w))?; - Ok(emit_success(schema_ok(), payload, format, render_text)) + let draft = !args.no_draft; + let body_tempfile = write_body_tempfile(&body)?; + let body_path = body_tempfile.path().to_path_buf(); + let create_call = build_create_call( + &ctx, + &head, + &base, + &args.title, + &body, + &body_path, + draft, + &args.reviewers, + &args.labels, + ); + create_and_fetch(runner, &ctx, &create_call, kind) +} + +fn create_and_fetch( + runner: &R, + ctx: &ProviderContext, + create_call: &BackendCall, + kind: PrKind, +) -> Result { + let create_output = runner.run(create_call)?; + let (number, url) = parse_create_output(ctx, &create_output)?; + let view_call = build_view_call(ctx, number); + let view_output = runner.run(&view_call)?; + parse_view_output(ctx, &view_output, number, url, kind) } fn render_text(payload: &PrCreatePayload) { diff --git a/crates/forge-cli/src/ops/pr_merge.rs b/crates/forge-cli/src/ops/pr_merge.rs index 0eb77967..a7ffb7a8 100644 --- a/crates/forge-cli/src/ops/pr_merge.rs +++ b/crates/forge-cli/src/ops/pr_merge.rs @@ -102,11 +102,48 @@ pub fn run_with Option>( )); } + let payload = run_lockdown_chain(runner, global, &ctx, args, workdir, method, delete_branch)?; + Ok(emit_success( + schema_version_for(BINARY, SCHEMA, SCHEMA_VERSION), + payload, + format, + render_text, + )) +} + +/// Macro-facing entry point: run the entire lock-down chain + merge + +/// post-merge fetch and return the typed payload without emitting an +/// envelope. The macro is responsible for surfacing the result through the +/// composite `data.steps[]` envelope. +pub fn compute( + runner: &R, + global: &GlobalFlags, + args: &PrMergeArgs, + workdir: &std::path::Path, +) -> Result { + let ctx = detect(global.provider_hint(), &global.remote, git_remote_url)?; + let cfg = ForgeConfig::load_from(workdir, find_git_toplevel(workdir).as_deref()); + let method = cfg.resolve_merge_method(args.method.map(|m| m.into_method())); + let cfg_delete = cfg.resolve_delete_branch(None); + enforce_keep_branch_conflict(args.keep_branch, &cfg)?; + let delete_branch = if args.keep_branch { false } else { cfg_delete }; + run_lockdown_chain(runner, global, &ctx, args, workdir, method, delete_branch) +} + +fn run_lockdown_chain( + runner: &R, + global: &GlobalFlags, + ctx: &ProviderContext, + args: &PrMergeArgs, + workdir: &std::path::Path, + method: MergeMethod, + delete_branch: bool, +) -> Result { // Rule 4 — clean worktree. worktree_clean(workdir, git_status_porcelain)?; // Rule 7 + base discovery — fetch pr.view once and reuse. - let pr = fetch_pr_view(runner, &ctx, args.id)?; + let pr = fetch_pr_view(runner, ctx, args.id)?; if pr.draft { return Err(ForgeError::validation( schema_err(), @@ -117,7 +154,7 @@ pub fn run_with Option>( } // Rule 6 — default branch protection (unless explicitly overridden). - let repo = fetch_repo_view(runner, &ctx)?; + let repo = fetch_repo_view(runner, ctx)?; if !args.allow_non_default_base && pr.base != repo.default_branch { return Err(ForgeError::validation( schema_err(), @@ -135,16 +172,16 @@ pub fn run_with Option>( enforce_method_supported(method, &repo)?; // Rule 8 — TTL-zero required-check re-check. - ensure_required_checks_green(runner, global, &ctx, &args.id.to_string())?; + ensure_required_checks_green(runner, global, ctx, &args.id.to_string())?; // All gates clear — invoke the backend. - let merge_call = build_merge_call(&ctx, args.id, method, delete_branch); + let merge_call = build_merge_call(ctx, args.id, method, delete_branch); runner.run(&merge_call)?; // Post-merge re-fetch for merge_sha. - let merge_sha = fetch_merge_sha(runner, &ctx, args.id)?; + let merge_sha = fetch_merge_sha(runner, ctx, args.id)?; - let payload = PrMergePayload { + Ok(PrMergePayload { provider: ctx.provider.as_str(), number: pr.number, url: pr.url, @@ -153,14 +190,7 @@ pub fn run_with Option>( deleted_branch: delete_branch, base: pr.base, head: pr.head, - }; - - Ok(emit_success( - schema_version_for(BINARY, SCHEMA, SCHEMA_VERSION), - payload, - format, - render_text, - )) + }) } /// Build the backend merge invocation. diff --git a/crates/forge-cli/src/ops/pr_ready.rs b/crates/forge-cli/src/ops/pr_ready.rs index 4702a71d..39f13dd8 100644 --- a/crates/forge-cli/src/ops/pr_ready.rs +++ b/crates/forge-cli/src/ops/pr_ready.rs @@ -66,11 +66,7 @@ where |p| println!("would run: {plan}", plan = p.plan.join(" ")), )); } - let _ = runner.run(&call)?; - - let view_call = pr_view_call(&ctx, args.id); - let view_output = runner.run(&view_call)?; - let payload: PrViewPayload = pr_view::parse_view_output(&ctx, &view_output)?; + let payload = run_backend_and_fetch(runner, &ctx, args.id)?; Ok(emit_success( schema_version_for(BINARY, SCHEMA, SCHEMA_VERSION), payload, @@ -79,6 +75,34 @@ where )) } +/// Macro-facing entry point: validate worktree, mark ready, re-fetch view, +/// return the typed payload without emitting an envelope. Used by +/// `pr deliver` to capture this step's typed output. +pub fn compute( + runner: &R, + ctx: &ProviderContext, + id: u64, + workdir: &std::path::Path, + git_status: G, +) -> Result +where + R: BackendRunner, + G: FnOnce(&std::path::Path) -> Result, +{ + worktree_clean(workdir, git_status)?; + run_backend_and_fetch(runner, ctx, id) +} + +fn run_backend_and_fetch( + runner: &R, + ctx: &ProviderContext, + id: u64, +) -> Result { + let _ = runner.run(&build_ready_call(ctx, id))?; + let view_output = runner.run(&pr_view_call(ctx, id))?; + pr_view::parse_view_output(ctx, &view_output) +} + fn build_ready_call(ctx: &ProviderContext, id: u64) -> BackendCall { let program = BackendProgram::for_provider(ctx.provider); let argv: Vec = match ctx.provider { diff --git a/crates/forge-cli/src/ops/pr_wait_checks.rs b/crates/forge-cli/src/ops/pr_wait_checks.rs index 155736b5..bdcbf734 100644 --- a/crates/forge-cli/src/ops/pr_wait_checks.rs +++ b/crates/forge-cli/src/ops/pr_wait_checks.rs @@ -72,21 +72,76 @@ pub fn run_with Option>( return Ok(emit_dry_run(&ctx, args, &snapshot_args, format)); } + match poll_until_terminal_or_timeout(runner, clock, global, &ctx, args, &snapshot_args)? { + WaitOutcome::Success(snapshot) => Ok(emit_success( + schema_version_for(BINARY, SCHEMA, SCHEMA_VERSION), + snapshot, + format, + render_text, + )), + WaitOutcome::Failed(snapshot) => Ok(emit_failure( + snapshot, + "checks_failed", + "required checks did not reach success", + nils_common::cli_contract::exit::RUNTIME, + format, + )), + WaitOutcome::TimedOut(snapshot) => Ok(emit_timeout(snapshot, format)), + } +} + +/// Internal outcome enum for the polling loop; the macro consumes the same +/// data through [`compute`] without going through the envelope emitters. +pub enum WaitOutcome { + Success(PrChecksPayload), + Failed(PrChecksPayload), + TimedOut(PrChecksPayload), +} + +/// Macro-facing entry point: poll until terminal or timeout and return the +/// final snapshot tagged with its outcome. Caller (e.g. `pr deliver`) decides +/// whether the macro continues, short-circuits with `checks_failed`, or +/// short-circuits with `checks_timeout`. +pub fn compute( + runner: &R, + clock: &C, + global: &GlobalFlags, + ctx: &ProviderContext, + args: &PrWaitChecksArgs, +) -> Result { + let snapshot_args = PrChecksArgs { + id: args.id.clone(), + required_only: args.required_only, + }; + poll_until_terminal_or_timeout(runner, clock, global, ctx, args, &snapshot_args) +} + +fn poll_until_terminal_or_timeout( + runner: &R, + clock: &C, + global: &GlobalFlags, + ctx: &ProviderContext, + args: &PrWaitChecksArgs, + snapshot_args: &PrChecksArgs, +) -> Result { let timeout = args.timeout; let interval = args.interval; let start = clock.now(); let deadline = start + timeout; loop { - let snapshot = pr_checks::snapshot(runner, global, &ctx, &snapshot_args)?; + let snapshot = pr_checks::snapshot(runner, global, ctx, snapshot_args)?; let snapshot = with_duration(snapshot, ms_between(start, clock.now())); if is_terminal(&snapshot) { - return finalise(snapshot, format); + if snapshot.state == "success" { + return Ok(WaitOutcome::Success(snapshot)); + } + return Ok(WaitOutcome::Failed(snapshot)); } let now = clock.now(); if now >= deadline { let timed_out = with_duration(snapshot, ms_between(start, now)); - return Ok(emit_timeout(timed_out, format)); + return Ok(WaitOutcome::TimedOut(timed_out)); } let remaining = deadline.saturating_duration_since(now); let sleep_for = std::cmp::min(interval, remaining); @@ -111,28 +166,6 @@ fn is_terminal(snapshot: &PrChecksPayload) -> bool { snapshot.pending.is_empty() } -fn finalise(snapshot: PrChecksPayload, format: OutputFormat) -> Result { - // Distinguish success from failure based on the aggregated state. - let state = snapshot.state; - let is_success = state == "success"; - if is_success { - Ok(emit_success( - schema_version_for(BINARY, SCHEMA, SCHEMA_VERSION), - snapshot, - format, - render_text, - )) - } else { - Ok(emit_failure( - snapshot, - "checks_failed", - "required checks did not reach success", - nils_common::cli_contract::exit::RUNTIME, - format, - )) - } -} - fn emit_timeout(snapshot: PrChecksPayload, format: OutputFormat) -> i32 { emit_failure( snapshot, @@ -384,7 +417,7 @@ mod tests { } #[test] - fn failure_state_marks_terminal_and_finalise_returns_runtime_code() { + fn failure_state_marks_terminal_and_emit_failure_returns_runtime_code() { let snapshot = PrChecksPayload { provider: "github", state: "failure", @@ -400,7 +433,13 @@ mod tests { duration_ms: Some(123), }; assert!(is_terminal(&snapshot)); - let code = finalise(snapshot, OutputFormat::Json).unwrap(); + let code = emit_failure( + snapshot, + "checks_failed", + "required checks did not reach success", + nils_common::cli_contract::exit::RUNTIME, + OutputFormat::Json, + ); assert_eq!(code, nils_common::cli_contract::exit::RUNTIME); } diff --git a/crates/forge-cli/src/ops/repo_view.rs b/crates/forge-cli/src/ops/repo_view.rs index cfc05e57..a969d9e1 100644 --- a/crates/forge-cli/src/ops/repo_view.rs +++ b/crates/forge-cli/src/ops/repo_view.rs @@ -50,9 +50,9 @@ pub fn run_with Option>( remote_url_lookup: F, ) -> Result { let ctx = crate::provider::detect(global.provider_hint(), &global.remote, remote_url_lookup)?; - let call = build_call(&ctx, global.repo.as_deref()); if global.dry_run { + let call = build_call(&ctx, global.repo.as_deref()); let payload = DryRunPayload::new(ctx.provider, &call); return Ok(emit_success( schema_version_for(BINARY, SCHEMA, SCHEMA_VERSION), @@ -62,8 +62,7 @@ pub fn run_with Option>( )); } - let output = runner.run(&call)?; - let payload = parse_backend_output(&ctx, &output)?; + let payload = compute(runner, &ctx, global.repo.as_deref())?; Ok(emit_success( schema_version_for(BINARY, SCHEMA, SCHEMA_VERSION), payload, @@ -72,6 +71,19 @@ pub fn run_with Option>( )) } +/// Macro-facing entry point: compute the payload without emitting. Used by +/// `pr deliver` to capture the repo view step's typed output for the +/// composite envelope. +pub fn compute( + runner: &R, + ctx: &ProviderContext, + repo_override: Option<&str>, +) -> Result { + let call = build_call(ctx, repo_override); + let output = runner.run(&call)?; + parse_backend_output(ctx, &output) +} + fn build_call(ctx: &ProviderContext, repo_override: Option<&str>) -> BackendCall { let program = BackendProgram::for_provider(ctx.provider); let mut argv: Vec = Vec::new(); diff --git a/crates/forge-cli/tests/integration.rs b/crates/forge-cli/tests/integration.rs index 551e13b0..58e9c4c0 100644 --- a/crates/forge-cli/tests/integration.rs +++ b/crates/forge-cli/tests/integration.rs @@ -9,6 +9,7 @@ mod integration { mod pr_checks_github; mod pr_checks_gitlab; mod pr_create; + mod pr_deliver; mod pr_merge; mod pr_wait_checks; mod repo_view; diff --git a/crates/forge-cli/tests/integration/pr_deliver.rs b/crates/forge-cli/tests/integration/pr_deliver.rs new file mode 100644 index 00000000..45d9bf08 --- /dev/null +++ b/crates/forge-cli/tests/integration/pr_deliver.rs @@ -0,0 +1,162 @@ +//! Sprint 6 integration tests for the `pr deliver` macro. These pin the +//! dry-run plan envelope and the CLI surface — full-chain end-to-end with all +//! six atoms (auth_status / repo_view / create / wait_checks / ready / merge) +//! is exercised in Sprint 7's parity harness, where the fixture corpus +//! handles real-shaped responses for every atom. + +use pretty_assertions::assert_eq; + +use super::support::{StubEnv, parse_envelope, run_forge_cli}; + +const FORBIDDEN_STUB: &str = "#!/bin/sh\necho 'should not run during dry-run' >&2\nexit 99\n"; + +#[test] +fn pr_deliver_dry_run_lists_all_six_steps_in_order() { + let stub = StubEnv::new().gh_stub(FORBIDDEN_STUB); + let out = run_forge_cli( + &stub, + &[ + "--provider", + "github", + "--dry-run", + "--format", + "json", + "pr", + "deliver", + "--kind", + "feature", + "--title", + "demo", + "--body", + "## Summary\nx\n\n## Test plan\ny\n", + ], + ); + assert_eq!(out.code, 0, "stderr={}", out.stderr); + let env = parse_envelope(&out.stdout); + assert_eq!(env["schema_version"], "cli.forge-cli.pr.deliver.v1"); + let steps: Vec<&str> = env["data"]["plan_steps"] + .as_array() + .expect("plan_steps array") + .iter() + .map(|s| s["step"].as_str().unwrap_or("")) + .collect(); + assert_eq!( + steps, + vec![ + "auth_status", + "repo_view", + "create", + "wait_checks", + "ready", + "merge", + ] + ); + assert_eq!(env["data"]["no_merge"], false); + assert_eq!(env["data"]["kind"], "feature"); +} + +#[test] +fn pr_deliver_dry_run_no_merge_excludes_ready_and_merge() { + let stub = StubEnv::new().gh_stub(FORBIDDEN_STUB); + let out = run_forge_cli( + &stub, + &[ + "--provider", + "github", + "--dry-run", + "--format", + "json", + "pr", + "deliver", + "--kind", + "bug", + "--title", + "demo", + "--body", + "## Summary\nx\n\n## Test plan\ny\n", + "--no-merge", + ], + ); + assert_eq!(out.code, 0, "stderr={}", out.stderr); + let env = parse_envelope(&out.stdout); + let steps: Vec<&str> = env["data"]["plan_steps"] + .as_array() + .expect("plan_steps array") + .iter() + .map(|s| s["step"].as_str().unwrap_or("")) + .collect(); + assert_eq!( + steps, + vec!["auth_status", "repo_view", "create", "wait_checks"] + ); + assert_eq!(env["data"]["no_merge"], true); + assert_eq!(env["data"]["kind"], "bug"); +} + +#[test] +fn pr_deliver_dry_run_method_override_threads_through_merge_plan() { + let stub = StubEnv::new().gh_stub(FORBIDDEN_STUB); + let out = run_forge_cli( + &stub, + &[ + "--provider", + "github", + "--dry-run", + "--format", + "json", + "pr", + "deliver", + "--kind", + "feature", + "--title", + "demo", + "--body", + "## Summary\nx\n\n## Test plan\ny\n", + "--method", + "rebase", + ], + ); + assert_eq!(out.code, 0, "stderr={}", out.stderr); + let env = parse_envelope(&out.stdout); + let merge_plan = env["data"]["plan_steps"] + .as_array() + .unwrap() + .iter() + .find(|s| s["step"] == "merge") + .expect("merge step present")["plan"] + .as_array() + .unwrap() + .iter() + .map(|v| v.as_str().unwrap_or("").to_string()) + .collect::>(); + assert!( + merge_plan.iter().any(|s| s == "--rebase"), + "expected --rebase in merge plan, got {merge_plan:?}" + ); +} + +#[test] +fn pr_deliver_help_lists_every_documented_flag() { + let stub = StubEnv::new(); + let out = run_forge_cli(&stub, &["pr", "deliver", "--help"]); + assert_eq!(out.code, 0, "stderr={}", out.stderr); + for flag in [ + "--kind", + "--title", + "--body", + "--body-file", + "--head", + "--base", + "--method", + "--reviewer", + "--timeout", + "--no-merge", + "--allow-non-default-base", + ] { + assert!( + out.stdout.contains(flag), + "expected `{flag}` in help, got: {}", + out.stdout + ); + } +} From 4cf537afc1b84b337541a4f2a85198ec07d9a1d8 Mon Sep 17 00:00:00 2001 From: graysurf <10785178+graysurf@users.noreply.github.com> Date: Wed, 20 May 2026 10:37:42 +0800 Subject: [PATCH 2/3] docs(forge-cli): mark Sprint 6 pr.deliver macro complete in execution ledger - Update Task 6.1 / 6.2 to completed and refresh the session log so the ledger matches HEAD before opening the Sprint 6 PR; Sprint 7 (parity harness + exit-code matrix + fixture corpus) is the next milestone. --- .../forge-cli/forge-cli-execution-state.md | 76 ++++++++++--------- 1 file changed, 42 insertions(+), 34 deletions(-) diff --git a/docs/plans/forge-cli/forge-cli-execution-state.md b/docs/plans/forge-cli/forge-cli-execution-state.md index 56b1ba2d..6a880f57 100644 --- a/docs/plans/forge-cli/forge-cli-execution-state.md +++ b/docs/plans/forge-cli/forge-cli-execution-state.md @@ -7,46 +7,46 @@ - Execution window: 2026-05-19 → ongoing - Staged execution confirmation: not applicable (default-continue authorization: "默認就一直做下去") -- Current task: Sprint 5 review (PR pending) -- Next task: Task 6.1 +- Current task: Sprint 6 review (PR pending) +- Next task: Task 7.1 - Last updated: 2026-05-20 -- Branch/commit: `feat/forge-cli-v1-sprint5-issues` cut from - `origin/main@c049a1f` (Sprint 4 PR #393 merge); Sprint 5 production - code complete locally, PR pending push + CI +- Branch/commit: `feat/forge-cli-v1-sprint6-deliver` cut from + `origin/main@718b7dd` (Sprint 5 PR #395 merge); Sprint 6 macro and + per-atom compute helpers complete locally, PR pending push and CI - Source document: docs/plans/forge-cli/forge-cli-plan.md - Direct source-doc execution waiver: not applicable ## Task Ledger -| ID | Status | Task | Evidence | Notes | -| -------- | --------- | ----------------------------------------------------------- | ----------------------------------------- | -------------------------------------------------------------- | -| Task 1.1 | completed | Scaffold `crates/forge-cli/` package and workspace wiring | PR #381 (merged `eb92969`) | Sprint 1 | -| Task 1.2 | completed | Define clap command tree + global flags | PR #381 | Sprint 1 | -| Task 1.3 | completed | Implement provider detection | PR #381 | Sprint 1 | -| Task 1.4 | completed | Subprocess wrapper + envelope serializer + dry-run plumbing | PR #381 | Sprint 1 | -| Task 1.5 | completed | Implement `auth status` atom | PR #381 | Sprint 1 | -| Task 1.6 | completed | Implement `repo view` atom | PR #381 | Sprint 1 | -| Task 1.7 | completed | Sprint 1 exit-code matrix + workspace gate | PR #381 | Sprint 1 | -| Task 2.1 | completed | PR body / title / branch validation module | PR #388 (merged `856797c`) | Sprint 2 — `crates/forge-cli/src/validations.rs` | -| Task 2.2 | completed | `pr create` atom | PR #388 | Sprint 2 | -| Task 2.3 | completed | `pr view`, `pr list`, `pr close` atoms | PR #388 | Sprint 2 | -| Task 2.4 | completed | `pr edit`, `pr comment`, `pr ready` atoms | PR #388 | Sprint 2 | -| Task 3.1 | completed | `pr checks` GitHub backend | PR #390 (merged `7d93d1a`) | Sprint 3 | -| Task 3.2 | completed | `pr checks` GitLab text parser with version fail-fast | PR #390 | Pinned `glab` 1.45.x — `glab_version_unsupported` → UNAVAIL 69 | -| Task 3.3 | completed | `pr wait-checks` polling loop | PR #390 + fix `1071cc6` | `checks_failed` RUNTIME 1 / `checks_timeout` UNAVAIL 69 | -| Task 4.1 | completed | Merge-method resolution + `.forge-cli.toml` loader | commit `cf45459` | Sprint 4 — 12 lib tests covering loader walk + warning surface | -| Task 4.2 | completed | TTL-zero required-check re-check helper | commit `feae217` | Sprint 4 — 6 lib + 4 integration tests pin no-caching contract | -| Task 4.3 | completed | `pr merge` atom | commit `9094408` | Sprint 4 — full lock-down chain + 12 lib + 4 integration tests | -| Task 5.1 | completed | `issue create`, `issue view`, `issue close`, `issue reopen` | branch `feat/forge-cli-v1-sprint5-issues` | Sprint 5 — issue_view shared parser + 4 atoms + 9 integration | -| Task 5.2 | completed | `issue edit`, `issue comment` | branch `feat/forge-cli-v1-sprint5-issues` | Sprint 5 — partial mutation + body-file stdin | -| Task 6.1 | pending | `pr deliver` macro composition + step envelope | n/a | Sprint 6 | -| Task 6.2 | pending | Macro CLI surface + dry-run plan rendering | n/a | Sprint 6 | -| Task 7.1 | pending | Parity harness | n/a | Sprint 7 | -| Task 7.2 | pending | Exit-code matrix completion | n/a | Sprint 7 | -| Task 7.3 | pending | Fixture redaction audit | n/a | Sprint 7 | -| Task 8.1 | pending | `wrappers/forge-cli` + shell completions | n/a | Sprint 8 | -| Task 8.2 | pending | Homebrew tap formula update | n/a | Sprint 8 | -| Task 8.3 | pending | `nils-cli` minor bump + tag + tap formula bump | n/a | Sprint 8 | +| ID | Status | Task | Evidence | Notes | +| -------- | --------- | ----------------------------------------------------------- | ------------------------------------------ | -------------------------------------------------------------- | +| Task 1.1 | completed | Scaffold `crates/forge-cli/` package and workspace wiring | PR #381 (merged `eb92969`) | Sprint 1 | +| Task 1.2 | completed | Define clap command tree + global flags | PR #381 | Sprint 1 | +| Task 1.3 | completed | Implement provider detection | PR #381 | Sprint 1 | +| Task 1.4 | completed | Subprocess wrapper + envelope serializer + dry-run plumbing | PR #381 | Sprint 1 | +| Task 1.5 | completed | Implement `auth status` atom | PR #381 | Sprint 1 | +| Task 1.6 | completed | Implement `repo view` atom | PR #381 | Sprint 1 | +| Task 1.7 | completed | Sprint 1 exit-code matrix + workspace gate | PR #381 | Sprint 1 | +| Task 2.1 | completed | PR body / title / branch validation module | PR #388 (merged `856797c`) | Sprint 2 — `crates/forge-cli/src/validations.rs` | +| Task 2.2 | completed | `pr create` atom | PR #388 | Sprint 2 | +| Task 2.3 | completed | `pr view`, `pr list`, `pr close` atoms | PR #388 | Sprint 2 | +| Task 2.4 | completed | `pr edit`, `pr comment`, `pr ready` atoms | PR #388 | Sprint 2 | +| Task 3.1 | completed | `pr checks` GitHub backend | PR #390 (merged `7d93d1a`) | Sprint 3 | +| Task 3.2 | completed | `pr checks` GitLab text parser with version fail-fast | PR #390 | Pinned `glab` 1.45.x — `glab_version_unsupported` → UNAVAIL 69 | +| Task 3.3 | completed | `pr wait-checks` polling loop | PR #390 + fix `1071cc6` | `checks_failed` RUNTIME 1 / `checks_timeout` UNAVAIL 69 | +| Task 4.1 | completed | Merge-method resolution + `.forge-cli.toml` loader | commit `cf45459` | Sprint 4 — 12 lib tests covering loader walk + warning surface | +| Task 4.2 | completed | TTL-zero required-check re-check helper | commit `feae217` | Sprint 4 — 6 lib + 4 integration tests pin no-caching contract | +| Task 4.3 | completed | `pr merge` atom | commit `9094408` | Sprint 4 — full lock-down chain + 12 lib + 4 integration tests | +| Task 5.1 | completed | `issue create`, `issue view`, `issue close`, `issue reopen` | branch `feat/forge-cli-v1-sprint5-issues` | Sprint 5 — issue_view shared parser + 4 atoms + 9 integration | +| Task 5.2 | completed | `issue edit`, `issue comment` | branch `feat/forge-cli-v1-sprint5-issues` | Sprint 5 — partial mutation + body-file stdin | +| Task 6.1 | completed | `pr deliver` macro composition + step envelope | branch `feat/forge-cli-v1-sprint6-deliver` | Sprint 6 — atom compute helpers + WaitOutcome enum, 6-step seq | +| Task 6.2 | completed | Macro CLI surface + dry-run plan rendering | branch `feat/forge-cli-v1-sprint6-deliver` | Sprint 6 — 4 integration tests pin dry-run / no-merge / method | +| Task 7.1 | pending | Parity harness | n/a | Sprint 7 | +| Task 7.2 | pending | Exit-code matrix completion | n/a | Sprint 7 | +| Task 7.3 | pending | Fixture redaction audit | n/a | Sprint 7 | +| Task 8.1 | pending | `wrappers/forge-cli` + shell completions | n/a | Sprint 8 | +| Task 8.2 | pending | Homebrew tap formula update | n/a | Sprint 8 | +| Task 8.3 | pending | `nils-cli` minor bump + tag + tap formula bump | n/a | Sprint 8 | ## Validation @@ -102,3 +102,11 @@ sharing `issue_view::parse_view_output`. Test surface grows to 216 lib + 63 integration (was 197 + 54 at end of Sprint 4). PR pending push + CI green. +- 2026-05-20 — Sprint 5 merged via PR #395 (`718b7dd`); cut Sprint 6 + branch `feat/forge-cli-v1-sprint6-deliver` from updated + `origin/main`. Sprint 6 added `compute()` helpers to the six atoms + used by the macro (auth.status / repo.view / pr.create / + pr.wait-checks / pr.ready / pr.merge), introduced + `pr_wait_checks::WaitOutcome`, and landed the `pr deliver` macro at + `crates/forge-cli/src/macros/pr_deliver.rs`. Test surface grows to + 219 lib + 67 integration. PR pending push + CI green. From 59a25a6a062cb151759e74f5fef0309f470a060a Mon Sep 17 00:00:00 2001 From: graysurf <10785178+graysurf@users.noreply.github.com> Date: Wed, 20 May 2026 10:50:21 +0800 Subject: [PATCH 3/3] test(forge-cli): add pr.deliver full-chain integration tests - Add three integration tests that exercise the macro's execute_sequence end to end against a comprehensive gh stub: no-merge (4 steps), full chain with merge_sha (6 steps), and short-circuit on pr.create title_too_long (DATA 65). Covers the previously test-only dry-run path through to the real subprocess wiring so workspace coverage stays above the strict 85 percent threshold after Sprint 6's macro refactor. --- crates/forge-cli/tests/integration.rs | 1 + .../tests/integration/pr_deliver_chain.rs | 350 ++++++++++++++++++ 2 files changed, 351 insertions(+) create mode 100644 crates/forge-cli/tests/integration/pr_deliver_chain.rs diff --git a/crates/forge-cli/tests/integration.rs b/crates/forge-cli/tests/integration.rs index 58e9c4c0..a906089a 100644 --- a/crates/forge-cli/tests/integration.rs +++ b/crates/forge-cli/tests/integration.rs @@ -10,6 +10,7 @@ mod integration { mod pr_checks_gitlab; mod pr_create; mod pr_deliver; + mod pr_deliver_chain; mod pr_merge; mod pr_wait_checks; mod repo_view; diff --git a/crates/forge-cli/tests/integration/pr_deliver_chain.rs b/crates/forge-cli/tests/integration/pr_deliver_chain.rs new file mode 100644 index 00000000..59de74f3 --- /dev/null +++ b/crates/forge-cli/tests/integration/pr_deliver_chain.rs @@ -0,0 +1,350 @@ +//! Full-chain integration tests for the `pr deliver` macro. These exercise +//! the entire execute_sequence — auth.status / repo.view / pr.create / +//! pr.wait-checks / pr.ready / pr.merge — against a comprehensive gh stub +//! that branches on argv. The dry-run path is covered by the sibling +//! `pr_deliver.rs` module; this one pins the lock-step ordering and +//! short-circuit behaviour against real subprocess wiring. + +use std::fs; +use std::os::unix::fs::PermissionsExt; +use std::path::{Path, PathBuf}; +use std::process::Command; + +use pretty_assertions::assert_eq; +use tempfile::TempDir; + +use super::support::{CmdOutput, StubEnv, parse_envelope, run_forge_cli_in}; + +const FIXTURE_CREATE_STDOUT: &str = include_str!("../fixtures/github/pr_create/create_stdout.txt"); +const FIXTURE_CHECKS_JSON: &str = include_str!("../fixtures/github/pr_checks/all_success.json"); + +/// Full pr.view JSON used by every step that re-fetches the PR. The fixture +/// at `tests/fixtures/github/pr_create/view_response.json` was designed for +/// pr.create's bespoke parser and omits `state`/`mergeable`; the macro's +/// chain reuses `pr_view::parse_view_output` (via pr.ready / pr.merge) which +/// requires the full Sprint 2 shape, so we inline a complete payload here. +const FULL_PR_VIEW_JSON: &str = r#"{ + "number": 123, + "url": "https://github.com/sympoies/nils-cli/pull/123", + "state": "OPEN", + "isDraft": false, + "title": "feat: sample feature", + "headRefName": "feat/sample", + "baseRefName": "main", + "mergeable": "MERGEABLE", + "mergedAt": null, + "labels": [] +}"#; + +/// Build the canonical Sprint 2 git tempdir: clean worktree on `feat/sample` +/// tracking `origin/feat/sample` at the same SHA, with the remote URL set to +/// `https:///.git` so provider detection lands on GitHub. +fn make_git_repo() -> TempDir { + let tempdir = TempDir::new().expect("tempdir"); + let repo = tempdir.path().join("repo"); + fs::create_dir_all(&repo).expect("repo dir"); + + let git = |args: &[&str]| { + let out = Command::new("git") + .arg("-C") + .arg(&repo) + .args(args) + .output() + .expect("git spawn"); + if !out.status.success() { + panic!( + "git {args:?} failed: {}", + String::from_utf8_lossy(&out.stderr) + ); + } + out + }; + + git(&["init", "-q", "-b", "main"]); + git(&["config", "user.email", "test@example.com"]); + git(&["config", "user.name", "Tester"]); + git(&["config", "commit.gpgsign", "false"]); + fs::write(repo.join("README.md"), "init\n").expect("readme"); + git(&["add", "README.md"]); + git(&["commit", "-q", "-m", "initial"]); + git(&["checkout", "-q", "-b", "feat/sample"]); + git(&[ + "remote", + "add", + "origin", + "https://github.com/sympoies/nils-cli.git", + ]); + let head_sha = String::from_utf8(git(&["rev-parse", "HEAD"]).stdout) + .unwrap() + .trim() + .to_string(); + let upstream_ref = repo.join(".git/refs/remotes/origin/feat/sample"); + fs::create_dir_all(upstream_ref.parent().unwrap()).unwrap(); + fs::write(&upstream_ref, format!("{head_sha}\n")).unwrap(); + git(&[ + "branch", + "-q", + "--set-upstream-to=origin/feat/sample", + "feat/sample", + ]); + tempdir +} + +fn write_full_chain_stub(stub: &StubEnv) -> PathBuf { + let body = format!( + r#"#!/bin/sh +set -e +case "$1 $2" in + "auth status") + cat <<'EOF' 1>&2 +github.com + ✓ Logged in to github.com account graysurf (keyring) + - Token scopes: 'repo', 'read:org' +EOF + ;; + "repo view") + cat <<'EOF' +{{ + "name": "nils-cli", + "owner": {{ "login": "sympoies" }}, + "url": "https://github.com/sympoies/nils-cli", + "defaultBranchRef": {{ "name": "main" }}, + "mergeCommitAllowed": false, + "squashMergeAllowed": true, + "rebaseMergeAllowed": false +}} +EOF + ;; + "pr create") + cat <<'EOF' +{create} +EOF + ;; + "pr checks") + cat <<'EOF' +{checks} +EOF + ;; + "pr ready"|"pr merge") + : + ;; + "pr view") + # Distinguish the post-merge view (--json mergeCommit) from regular view. + case "$*" in + *"--json mergeCommit"*) + cat <<'EOF' +{{ "mergeCommit": {{ "oid": "abc123def456" }} }} +EOF + ;; + *) + cat <<'EOF' +{view} +EOF + ;; + esac + ;; + *) + echo "stub: unexpected gh args: $*" >&2 + exit 99 + ;; +esac +"#, + create = FIXTURE_CREATE_STDOUT, + view = FULL_PR_VIEW_JSON, + checks = FIXTURE_CHECKS_JSON, + ); + let path = stub.tempdir.path().join("gh"); + fs::write(&path, body).expect("write gh stub"); + let mut perm = fs::metadata(&path).expect("metadata").permissions(); + perm.set_mode(0o755); + fs::set_permissions(&path, perm).expect("chmod"); + path +} + +fn run_in_repo(stub: &StubEnv, repo: &Path, args: &[&str]) -> CmdOutput { + run_forge_cli_in(stub, args, Some(repo)) +} + +#[test] +fn pr_deliver_full_chain_no_merge_emits_four_steps_and_returns_success() { + let tempdir = make_git_repo(); + let repo_path = tempdir.path().join("repo"); + + let stub = StubEnv::new(); + let gh_path = write_full_chain_stub(&stub); + let stub = stub.env("FORGE_CLI_GH_BIN", gh_path.to_string_lossy()); + + let out = run_in_repo( + &stub, + &repo_path, + &[ + "--provider", + "github", + "--format", + "json", + "pr", + "deliver", + "--kind", + "feature", + "--title", + "feat: sample feature", + "--body", + "## Summary\n\nLand the new feature.\n\n## Test plan\n\nVerified.\n", + "--head", + "feat/sample", + "--base", + "main", + "--timeout", + "5s", + "--no-merge", + ], + ); + assert_eq!(out.code, 0, "stderr={}", out.stderr); + let envelope = parse_envelope(&out.stdout); + assert_eq!(envelope["schema_version"], "cli.forge-cli.pr.deliver.v1"); + assert_eq!(envelope["ok"], true); + assert_eq!(envelope["data"]["kind"], "feature"); + assert_eq!(envelope["data"]["provider"], "github"); + let steps: Vec<&str> = envelope["data"]["steps"] + .as_array() + .expect("steps array") + .iter() + .map(|s| s["step"].as_str().unwrap_or("")) + .collect(); + assert_eq!( + steps, + vec!["auth_status", "repo_view", "create", "wait_checks"], + "no-merge must stop after wait_checks" + ); + assert_eq!(envelope["data"]["pr"]["merged"], false); + assert!(envelope["data"]["pr"]["merge_sha"].is_null()); + assert_eq!(envelope["data"]["pr"]["number"], 123); +} + +#[test] +fn pr_deliver_full_chain_emits_all_six_steps_with_merge_sha() { + let tempdir = make_git_repo(); + let repo_path = tempdir.path().join("repo"); + + let stub = StubEnv::new(); + let gh_path = write_full_chain_stub(&stub); + let stub = stub.env("FORGE_CLI_GH_BIN", gh_path.to_string_lossy()); + + let out = run_in_repo( + &stub, + &repo_path, + &[ + "--provider", + "github", + "--format", + "json", + "pr", + "deliver", + "--kind", + "feature", + "--title", + "feat: sample feature", + "--body", + "## Summary\n\nLand the new feature.\n\n## Test plan\n\nVerified.\n", + "--head", + "feat/sample", + "--base", + "main", + "--timeout", + "5s", + ], + ); + assert_eq!(out.code, 0, "stdout={}\nstderr={}", out.stdout, out.stderr); + let envelope = parse_envelope(&out.stdout); + let steps: Vec<&str> = envelope["data"]["steps"] + .as_array() + .expect("steps array") + .iter() + .map(|s| s["step"].as_str().unwrap_or("")) + .collect(); + assert_eq!( + steps, + vec![ + "auth_status", + "repo_view", + "create", + "wait_checks", + "ready", + "merge", + ], + "full chain must include all six steps in order" + ); + assert_eq!(envelope["data"]["pr"]["merged"], true); + assert_eq!(envelope["data"]["pr"]["merge_sha"], "abc123def456"); + // Every step carries its atom's schema literal. + let schemas: Vec<&str> = envelope["data"]["steps"] + .as_array() + .unwrap() + .iter() + .map(|s| s["schema_version"].as_str().unwrap_or("")) + .collect(); + assert_eq!( + schemas, + vec![ + "cli.forge-cli.auth.status.v1", + "cli.forge-cli.repo.view.v1", + "cli.forge-cli.pr.create.v1", + "cli.forge-cli.pr.checks.v1", + "cli.forge-cli.pr.ready.v1", + "cli.forge-cli.pr.merge.v1", + ] + ); +} + +#[test] +fn pr_deliver_short_circuits_when_pr_create_validation_fails_with_data_65() { + // Title over 70 chars trips pr.create's title_length validation. The + // macro must surface DATA 65 (not a remapped runtime code) and omit + // later steps (wait_checks / ready / merge) from data.steps[]. + let tempdir = make_git_repo(); + let repo_path = tempdir.path().join("repo"); + + let stub = StubEnv::new(); + let gh_path = write_full_chain_stub(&stub); + let stub = stub.env("FORGE_CLI_GH_BIN", gh_path.to_string_lossy()); + + let long_title = "a".repeat(71); + let out = run_in_repo( + &stub, + &repo_path, + &[ + "--provider", + "github", + "--format", + "json", + "pr", + "deliver", + "--kind", + "feature", + "--title", + &long_title, + "--body", + "## Summary\n\nx\n\n## Test plan\n\ny\n", + "--head", + "feat/sample", + "--base", + "main", + ], + ); + assert_eq!( + out.code, 65, + "expected DATA 65 on title_too_long, stderr={}", + out.stderr + ); + let envelope = parse_envelope(&out.stdout); + assert_eq!(envelope["ok"], false); + assert_eq!(envelope["error"]["code"], "title_too_long"); + // Only auth_status + repo_view ran before the validation failed. + let steps: Vec<&str> = envelope["data"]["steps"] + .as_array() + .expect("steps array") + .iter() + .map(|s| s["step"].as_str().unwrap_or("")) + .collect(); + assert_eq!(steps, vec!["auth_status", "repo_view"]); +}