Forge-cli v1 Sprint 7: parity harness, exit-code matrix, fixture lint

.agents/skills/nils-cli-verify-required-checks/scripts/nils-cli-verify-required-checks.sh

@@ -119,6 +119,7 @@ run bash scripts/ci/docs-hygiene-audit.sh --strict
 run bash scripts/ci/markdownlint-audit.sh --strict
 run bash scripts/ci/plan-bundle-validate.sh --strict
 run bash scripts/ci/cli-output-contract-lint.sh --strict
+run bash scripts/ci/forge-cli-fixture-lint.sh --strict
 if [[ "$docs_only" -eq 1 ]]; then
   echo "ok: docs-only nils-cli checks passed"
   exit 0

crates/forge-cli/tests/fixtures/README.md

@@ -0,0 +1,33 @@
+# forge-cli test fixtures
+
+This tree holds the canonical `gh` / `glab` responses every forge-cli integration test replays against the stub binaries injected via `FORGE_CLI_GH_BIN` / `FORGE_CLI_GLAB_BIN`. The fixtures are split per-op (e.g. `pr_create/`, `pr_checks/`) and per-provider (`github/`, `gitlab/`).
+
+## Redaction policy
+
+Token-shaped strings MUST NOT appear anywhere under this tree. `scripts/ci/forge-cli-fixture-lint.sh` runs in CI's docs-only lane and fails the build on any match for these patterns:
+
+| Pattern (regex)             | Source                                 |
+| --------------------------- | -------------------------------------- |
+| `gh[ps]_[A-Za-z0-9_]{16,}`  | GitHub personal / server tokens        |
+| `ghr_[A-Za-z0-9_]{16,}`     | GitHub refresh tokens                  |
+| `gho_[A-Za-z0-9_]{16,}`     | GitHub OAuth tokens                    |
+| `glpat-[A-Za-z0-9_-]{16,}`  | GitLab personal access tokens          |
+| `Bearer [A-Za-z0-9._-]{16,}`| Generic bearer auth headers (e.g. JWT) |
+
+Replace every occurrence with `<redacted-token>` (or `<redacted-jwt>` for bearer-style headers). When you need a token-shaped placeholder to exercise downstream parsing without tripping the lint, use shorter shapes (under 16 characters) — they're below the lint's threshold and stay obviously synthetic.
+
+## Adding a new fixture
+
+1. Drop the file under `crates/forge-cli/tests/fixtures/<provider>/<op>/<name>.<ext>`.
+2. Confirm the placeholder values use the same redaction markers as existing fixtures.
+3. Run `bash scripts/ci/forge-cli-fixture-lint.sh` locally before committing.
+4. Reference the fixture from the matching integration test via `include_str!(...)`.
+
+## Negative regression test
+
+`crates/forge-cli/tests/integration/fixture_lint.rs` writes a synthetic
+GitHub-token-shaped string (e.g. `ghp_` followed by 20+ alphanumerics) into a
+tempdir and re-invokes the lint script against that tempdir. The lint must
+exit non-zero and surface the file path + line in stderr; the test asserts
+both. The actual token-shaped string is constructed in code so this README
+never contains one itself.

crates/forge-cli/tests/integration.rs

@@ -5,7 +5,10 @@ mod integration {
     mod auth_status;
     mod cli;
     mod exit_codes;
+    mod exit_codes_full;
+    mod fixture_lint;
     mod issue_atoms;
+    mod parity;
     mod pr_checks_github;
     mod pr_checks_gitlab;
     mod pr_create;

crates/forge-cli/tests/integration/exit_codes_full.rs

@@ -0,0 +1,150 @@
+//! Sprint 7 Task 7.2 — full exit-code matrix.
+//!
+//! Spec: `forge-cli-spec-v1` §"Exit code map" + §"Lock-down policy". Every
+//! row in those two tables MUST have at least one assertion here. Each test
+//! references the constant name from `nils_common::cli_contract::exit`, never
+//! the numeric value, so a future re-number of the sysexits table doesn't
+//! drift this matrix silently.
+
+use forge_cli::error::ForgeError;
+use nils_common::cli_contract::exit;
+use pretty_assertions::assert_eq;
+
+const SCHEMA: &str = "cli.forge-cli.error.v1";
+
+// ---------------------------------------------------------------------------
+// Spec §"Exit code map" — one assertion per row.
+// ---------------------------------------------------------------------------
+
+#[test]
+fn exit_constant_runtime_is_used_by_backend_error() {
+    let err = ForgeError::backend_error(SCHEMA, "backend non-zero", None);
+    assert_eq!(err.exit_code(), exit::RUNTIME);
+    assert_eq!(err.kind(), "backend_error");
+}
+
+#[test]
+fn exit_constant_usage_is_used_by_provider_unsupported() {
+    let err = ForgeError::provider_unsupported(SCHEMA, "unknown host", None);
+    assert_eq!(err.exit_code(), exit::USAGE);
+    assert_eq!(err.kind(), "provider_unsupported");
+}
+
+#[test]
+fn exit_constant_data_is_used_by_validation() {
+    // DATA 65 covers every lock-down rule violation; the next group below
+    // pins each documented `error.kind` to the same exit code.
+    let err = ForgeError::validation(SCHEMA, "branch_name_invalid", "bad branch", None);
+    assert_eq!(err.exit_code(), exit::DATA);
+}
+
+#[test]
+fn exit_constant_unavailable_is_used_by_backend_missing() {
+    let err = ForgeError::backend_missing(SCHEMA, "gh not found", None);
+    assert_eq!(err.exit_code(), exit::UNAVAILABLE);
+    assert_eq!(err.kind(), "backend_missing");
+}
+
+#[test]
+fn exit_constant_unavailable_is_used_by_backend_unauthenticated() {
+    let err = ForgeError::backend_unauthenticated(SCHEMA, "auth required", None);
+    assert_eq!(err.exit_code(), exit::UNAVAILABLE);
+    assert_eq!(err.kind(), "backend_unauthenticated");
+}
+
+#[test]
+fn exit_constant_software_is_used_by_software_error() {
+    let err = ForgeError::software(SCHEMA, "invariant blew", None);
+    assert_eq!(err.exit_code(), exit::SOFTWARE);
+    assert_eq!(err.kind(), "software_error");
+}
+
+#[test]
+fn exit_constant_software_is_used_by_not_implemented() {
+    let err = ForgeError::not_implemented(SCHEMA, "subcommand not implemented");
+    assert_eq!(err.exit_code(), exit::SOFTWARE);
+    assert_eq!(err.kind(), "not_implemented");
+}
+
+// ---------------------------------------------------------------------------
+// Spec §"Lock-down policy" — one assertion per documented `error.kind`.
+// Every row maps to DATA 65 except checks_failed (RUNTIME 1) and
+// checks_timeout / glab_version_unsupported (UNAVAILABLE 69) which the spec
+// keeps adjacent to the table notes.
+// ---------------------------------------------------------------------------
+
+const LOCKDOWN_DATA_KINDS: &[&str] = &[
+    "branch_name_invalid",
+    "branch_kind_mismatch",
+    "body_missing_summary",
+    "body_missing_test_plan",
+    "title_too_long",
+    "dirty_worktree",
+    "head_not_pushed",
+    "default_branch_protected",
+    "draft_merge_refused",
+    "checks_pending",
+    "merge_method_unsupported",
+    "keep_branch_conflict",
+];
+
+#[test]
+fn every_lockdown_data_kind_maps_to_data_65() {
+    for kind in LOCKDOWN_DATA_KINDS {
+        let err = ForgeError::validation(SCHEMA, kind, "x", None);
+        assert_eq!(
+            err.exit_code(),
+            exit::DATA,
+            "kind={kind} should map to DATA"
+        );
+        assert_eq!(err.kind(), *kind, "kind round-trip for {kind}");
+    }
+}
+
+#[test]
+fn checks_failed_maps_to_runtime_1() {
+    let err = ForgeError::runtime_failure(SCHEMA, "checks_failed", "ci failed", None);
+    assert_eq!(err.exit_code(), exit::RUNTIME);
+    assert_eq!(err.kind(), "checks_failed");
+}
+
+#[test]
+fn checks_timeout_maps_to_unavailable_69() {
+    let err = ForgeError::unavailable(SCHEMA, "checks_timeout", "deadline reached", None);
+    assert_eq!(err.exit_code(), exit::UNAVAILABLE);
+    assert_eq!(err.kind(), "checks_timeout");
+}
+
+#[test]
+fn glab_version_unsupported_maps_to_unavailable_69() {
+    let err = ForgeError::unavailable(SCHEMA, "glab_version_unsupported", "upgrade", None);
+    assert_eq!(err.exit_code(), exit::UNAVAILABLE);
+    assert_eq!(err.kind(), "glab_version_unsupported");
+}
+
+// ---------------------------------------------------------------------------
+// Spec-stability invariant: the six exit constants the binary may emit are
+// the ones documented in the spec's Exit code map, in numeric order.
+// ---------------------------------------------------------------------------
+
+#[test]
+fn binary_only_emits_documented_exit_constants() {
+    // No literals — every value comes from nils_common::cli_contract::exit.
+    let canonical = [
+        ("SUCCESS", exit::SUCCESS),
+        ("RUNTIME", exit::RUNTIME),
+        ("USAGE", exit::USAGE),
+        ("DATA", exit::DATA),
+        ("UNAVAILABLE", exit::UNAVAILABLE),
+        ("SOFTWARE", exit::SOFTWARE),
+    ];
+    // All six are distinct.
+    let mut codes: Vec<i32> = canonical.iter().map(|(_, c)| *c).collect();
+    codes.sort();
+    codes.dedup();
+    assert_eq!(
+        codes.len(),
+        6,
+        "exit constants must be six distinct values, got {canonical:?}"
+    );
+}

crates/forge-cli/tests/integration/fixture_lint.rs

@@ -0,0 +1,83 @@
+//! Sprint 7 Task 7.3 — synthetic regression for the fixture redaction lint.
+//!
+//! Spec: `forge-cli-spec-v1` §"Security and redaction expectations". The
+//! lint script lives at `scripts/ci/forge-cli-fixture-lint.sh`; this test
+//! plants a fake `ghp_aaaaaaaaaaaaaaaaaaaa` string into a tempdir and runs
+//! the script against that root, asserting the script fails (non-zero exit)
+//! and reports the file path + line in stderr.
+
+use std::fs;
+use std::process::Command;
+
+use tempfile::TempDir;
+
+#[test]
+fn fixture_lint_catches_planted_ghp_token() {
+    let tempdir = TempDir::new().expect("tempdir");
+    let bad_file = tempdir.path().join("planted.json");
+    fs::write(&bad_file, r#"{ "token": "ghp_aaaaaaaaaaaaaaaaaaaa" }"#)
+        .expect("write planted fixture");
+
+    let script = std::env::current_dir()
+        .expect("cwd")
+        .ancestors()
+        .find(|p| p.join("scripts/ci/forge-cli-fixture-lint.sh").is_file())
+        .expect("locate lint script root (cargo test from any depth)")
+        .join("scripts/ci/forge-cli-fixture-lint.sh");
+
+    let output = Command::new("bash")
+        .arg(&script)
+        .arg(tempdir.path())
+        .output()
+        .expect("spawn fixture lint");
+
+    let stderr = String::from_utf8_lossy(&output.stderr);
+    assert!(
+        !output.status.success(),
+        "expected non-zero exit on planted token; stderr={stderr}"
+    );
+    assert!(
+        stderr.contains("planted.json"),
+        "expected the planted filename in stderr, got: {stderr}"
+    );
+    assert!(
+        stderr.contains("ghp_aaaaaaaaaaaaaaaaaaaa"),
+        "expected the offending match echoed in stderr, got: {stderr}"
+    );
+}
+
+#[test]
+fn fixture_lint_passes_on_clean_tempdir() {
+    let tempdir = TempDir::new().expect("tempdir");
+    fs::write(
+        tempdir.path().join("clean.json"),
+        r#"{ "token": "<redacted-token>" }"#,
+    )
+    .expect("write clean fixture");
+    fs::write(
+        tempdir.path().join("clean.txt"),
+        "Bearer short\n", // under the 16-char threshold, must not match
+    )
+    .expect("write short bearer fixture");
+
+    let script = std::env::current_dir()
+        .expect("cwd")
+        .ancestors()
+        .find(|p| p.join("scripts/ci/forge-cli-fixture-lint.sh").is_file())
+        .expect("locate lint script root")
+        .join("scripts/ci/forge-cli-fixture-lint.sh");
+
+    let output = Command::new("bash")
+        .arg(&script)
+        .arg(tempdir.path())
+        .output()
+        .expect("spawn fixture lint");
+
+    let stdout = String::from_utf8_lossy(&output.stdout);
+    assert!(
+        output.status.success(),
+        "expected zero exit on clean tempdir; stdout={stdout}, stderr={}",
+        String::from_utf8_lossy(&output.stderr)
+    );
+    assert!(stdout.starts_with("PASS:"), "stdout={stdout}");
+}