From 1882283f6817e40dde73b38de36b1e1718f9654e Mon Sep 17 00:00:00 2001 From: LeadGoEngineer Date: Sun, 10 May 2026 18:34:24 -0400 Subject: [PATCH 1/3] feat(compat): scaffold cross-CLI conformance library + dates bundle MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adds `compat/` as a stdlib-only Go module (`github.com/quantcli/common/compat`) that every `*-export-cli` can import to machine-attest its conformance to CONTRACT.md. The first bundle, `compat/dates`, pins down four assertions tied to CONTRACT §3 and §5: - `--help` documents `--since` and `--until` and exits 0. - An invalid `--since` value exits non-zero with stderr-only error. - `--help` is hermetic (no network, even with proxies forced unreachable). - A flag-validation failure is hermetic for the same reason. What is intentionally NOT yet machine-attested: the local-midnight semantics of `--since 2026-04-15`. Asserting that black-box requires either a `--print-resolved-window`-style affordance on every CLI (a substantive contract change, out of scope here) or per-upstream recorded HTTP fixtures (heavy). A follow-up issue will propose the affordance. The library is self-tested in `quantcli/common`'s CI against a tiny contract-compliant stub in `compat/internal/stubcli`, so a regression in the library itself is caught here before any real exporter wires it up. Docs: - `CONTRACT.md` gains an §7 "Conformance" section and an Attestation column on the Status table (machine vs human). - `CONTRIBUTING.md` replaces the "harness is being scaffolded" caveat with the real integration recipe. - `compat/README.md` is the exporter-facing how-to. CI: - Adds a new `compat` job to `.github/workflows/ci.yml` that runs `go vet` and `go test` inside the compat module when `compat/go.mod` is present. The existing `go` job (which detects a root-level `go.mod`) is untouched, so this is additive. Refs CONTRACT.md §3, §5, §7. Refs QUA-8. Co-Authored-By: Paperclip --- .github/workflows/ci.yml | 29 +++++ CONTRACT.md | 33 +++--- CONTRIBUTING.md | 12 ++- compat/README.md | 80 ++++++++++++++ compat/compat.go | 120 +++++++++++++++++++++ compat/dates/dates.go | 185 ++++++++++++++++++++++++++++++++ compat/dates/dates_test.go | 36 +++++++ compat/go.mod | 3 + compat/internal/stubcli/main.go | 65 +++++++++++ 9 files changed, 547 insertions(+), 16 deletions(-) create mode 100644 compat/README.md create mode 100644 compat/compat.go create mode 100644 compat/dates/dates.go create mode 100644 compat/dates/dates_test.go create mode 100644 compat/go.mod create mode 100644 compat/internal/stubcli/main.go diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index f1d091b..0333494 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -44,3 +44,32 @@ jobs: - name: go test if: steps.detect.outputs.has_go == 'true' run: go test ./... + + compat: + name: compat module (vet + self-test) + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v4 + - id: detect + name: detect compat module + run: | + if [ -f compat/go.mod ]; then + echo "has_compat=true" >> "$GITHUB_OUTPUT" + else + echo "has_compat=false" >> "$GITHUB_OUTPUT" + echo "::notice::No compat/go.mod present; skipping compat job." + fi + - uses: actions/setup-go@v5 + if: steps.detect.outputs.has_compat == 'true' + with: + go-version-file: compat/go.mod + # compat is stdlib-only; no module cache needed. + cache: false + - name: go vet (compat) + if: steps.detect.outputs.has_compat == 'true' + working-directory: compat + run: go vet ./... + - name: go test (compat self-test) + if: steps.detect.outputs.has_compat == 'true' + working-directory: compat + run: go test ./... diff --git a/CONTRACT.md b/CONTRACT.md index 54d118d..5ea7a8f 100644 --- a/CONTRACT.md +++ b/CONTRACT.md @@ -6,17 +6,18 @@ The contract exists so that, once you have used one of these CLIs, the others fe ## Status -| Section | crono-export | liftoff-export | withings-export | -|---|---|---|---| -| Repo naming (`{service}-export-cli`) | ✓ | ✓ | ✓ | -| Timezone policy | ✓ | ✓ | ✓ | -| Date flags (`--since` / `--until`) | ✓ | ✓ | ✓ | -| Markdown-default output | ✓ | ✓ | ✓ | -| Single `--format` flag | ✓ | ✓ | ✓ | -| `auth status` subcommand | ✓ | ✓ | ✓ | -| `prime` subcommand | ✓ | ✓ | ✓ | - -All sections shipped across all three CLIs (April 25, 2026). +| Section | crono-export | liftoff-export | withings-export | Attestation | +|---|---|---|---|---| +| Repo naming (`{service}-export-cli`) | ✓ | ✓ | ✓ | human | +| Timezone policy | ✓ | ✓ | ✓ | human | +| Date flags (`--since` / `--until`) — surface | ✓ | ✓ | ✓ | **machine** ([`compat/dates`](compat/README.md)) | +| Date flags — local-midnight semantics | ✓ | ✓ | ✓ | human | +| Markdown-default output | ✓ | ✓ | ✓ | human | +| Single `--format` flag | ✓ | ✓ | ✓ | human | +| `auth status` subcommand | ✓ | ✓ | ✓ | human | +| `prime` subcommand | ✓ | ✓ | ✓ | human | + +All sections shipped across all three CLIs (April 25, 2026). The "Attestation" column tracks whether a contract section is verified by an automated [compat test](compat/README.md) on every PR or only by human review at merge time. Rows marked "human" are candidates for promotion to "machine" as the compat library grows. --- @@ -98,7 +99,15 @@ GOTCHAS non-obvious pitfalls Prime is short. It is not a man page. If it grows past one terminal screen, something belongs in this contract instead. -## 7. Versioning and releases +## 7. Conformance (the compat library) + +Conformance to this contract is verified by [`compat/`](compat/README.md), a small black-box Go test library that lives in this repo and is imported by every `*-export-cli` from its own CI. The current bundles: + +- [`compat/dates`](compat/README.md) — pins down §3: that `--since` / `--until` are documented in `--help`, that an invalid value exits non-zero with stderr-only error, and that flag handling makes no network request (per §5). + +A new exporter is not "in" the family until its CI runs at least the `dates` bundle green. Existing exporters that have not yet wired up the bundle are tracked in the Status table's Attestation column. + +## 8. Versioning and releases Semantic versioning. User-visible bug fix → patch. New subcommand or flag → minor. Removed/renamed flag → major. Releases cut via `gh release create` against the relevant tag; goreleaser builds binaries for darwin/linux/windows × amd64/arm64 and publishes the cask to `quantcli/homebrew-tap`. diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index d0ee15f..a54999e 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -69,14 +69,18 @@ Keep the per-CLI PR (the one that adds the CLI to the table) trivial and reviewa ## Compat tests -The contract is only as honest as the test that proves three CLIs behave the same. The compat-test expectation: +The contract is only as honest as the test that proves three CLIs behave the same. The harness lives in [`compat/`](compat/README.md) as its own Go module (`github.com/quantcli/common/compat`); each exporter imports it and runs the relevant bundles against its own built binary in CI. -- Anyone changing `CONTRACT.md` is also expected to update or add tests in this repo that exercise the contract against the released binaries of every `*-export-cli`. The tests live in `compat/` (one suite per contract section: dates, formats, auth, prime). -- The tests run pinned versions of each CLI binary, drive them with the same input fixtures, and assert that observable output (stdout, stderr, exit code, JSON shape) matches the spec. They are deliberately black-box: the goal is to catch divergence, not to test internals. +Rules: + +- Anyone changing `CONTRACT.md` is also expected to update or add tests under `compat/` that exercise the new behavior against every `*-export-cli`. +- The harness is deliberately black-box: it shells out to the binary and asserts on stdout, stderr, and exit code only. It must not import a CLI's internal packages. +- One subpackage per contract section (`compat/dates`, future `compat/formats`, `compat/auth`, `compat/prime`). Each exposes a single entry point — `RunContract(t, runner)` — that exporters call from one build-tagged `_test.go` file. - A PR that changes the contract without touching `compat/` is incomplete. Either update the tests in the same PR or open a follow-up issue and link it from the PR body before merging — the Lead Go Engineer holds the line on this. - Compat tests run in CI on every PR and on `main`. A failing compat test on `main` means at least one shipped CLI no longer matches the contract, and that's a release-blocker incident, not a flake. +- The Status table in `CONTRACT.md` distinguishes **machine-attested** rows (covered by `compat/`) from **human-attested** rows (still verified by reviewer judgment). Promoting a row from human to machine attestation is itself a worthwhile PR. -The `compat/` harness is being scaffolded — until it lands, document the test you *would* write in the PR body so the expectation stays visible. +**Bar for a new exporter:** the exporter's CI must build its binary and run `dates.RunContract` against it green. See [`compat/README.md`](compat/README.md) for the one-file integration pattern. ## License and sign-off diff --git a/compat/README.md b/compat/README.md new file mode 100644 index 0000000..54e2668 --- /dev/null +++ b/compat/README.md @@ -0,0 +1,80 @@ +# `compat/` — cross-CLI conformance library + +This module is the machine-attested half of [`CONTRACT.md`](../CONTRACT.md). Every `*-export-cli` is expected to wire it into CI so the contract's "✓" status table stops being purely human-attested. + +It is a Go module (`github.com/quantcli/common/compat`) under the `compat/` subdirectory of `quantcli/common`. Stdlib only. + +## Design at a glance + +- **Black-box.** The library never imports a CLI's internal packages. It shells out to the binary and asserts on stdout, stderr, and exit code. +- **One subpackage per contract section.** Currently only `dates/` (CONTRACT §2–§3). `formats/`, `auth/`, `prime/` are expected to follow. +- **Exporters consume via a build-tagged `_test.go`.** No production import; compat tests do not ship in the released binary. +- **Hermetic by default.** `compat.Runner` runs the binary with an empty environment unless callers opt into specific variables. Subtests that need to assert "no network call" set proxy env vars to an unreachable address. + +## Usage from an exporter + +Add a single file in your repo (e.g. `compat_test.go`) gated behind a build tag so it does not run as part of the default `go test ./...`: + +```go +//go:build compat + +package main_test + +import ( + "os" + "testing" + + "github.com/quantcli/common/compat" + "github.com/quantcli/common/compat/dates" +) + +func TestContractDates(t *testing.T) { + bin := os.Getenv("EXPORT_CLI_BIN") + if bin == "" { + t.Skip("EXPORT_CLI_BIN not set; skipping compat suite") + } + dates.RunContract(t, compat.Runner{Binary: bin}) +} +``` + +Then in your CI workflow: + +```yaml +- name: build + run: go build -o /tmp/cli . +- name: compat tests + env: + EXPORT_CLI_BIN: /tmp/cli + run: go test -tags=compat ./... +``` + +The exporter does not need a separate `go.mod` for compat tests — the standard `require github.com/quantcli/common/compat vX.Y.Z` line in the exporter's existing `go.mod` is enough. + +## What `dates.RunContract` covers today + +| Subtest | Contract | What it asserts | +|---|---|---| +| `HelpDocumentsDateFlags` | §3 | `--help` mentions `--since` and `--until` and exits 0. | +| `InvalidSinceValueFails` | §3, §4 | `--since obviously-not-a-date` exits non-zero, writes to stderr, leaves stdout empty. | +| `HelpIsHermetic` | §5 | `--help` succeeds with all HTTP proxies pointed at an unreachable address. | +| `FlagValidationIsHermetic` | §5 | A parse failure also produces no successful outbound request. | + +## What it does NOT cover yet + +The actual local-midnight semantics of `--since 2026-04-15` (the harmonization that just landed across crono/liftoff/withings) is still **human-attested** in the status table. Asserting it black-box requires either: + +- a `--print-resolved-window`-style affordance on every CLI (a substantive contract change, intentionally out of scope of the first compat-test cut), or +- per-upstream recorded HTTP fixtures (heavy, and pinned to specific API shapes). + +When that affordance lands, the test belongs here as `dates.LocalMidnightSemantics`. + +## Adding a new contract test + +1. Decide which CONTRACT.md section it pins down. If no subpackage exists for that section, create one (`compat/
/`). +2. Write the assertion as a function that takes `*testing.T` and `compat.Runner`. +3. Wire it into the section's `RunContract`. Subtests are `t.Run`-scoped so one failure does not mask the rest. +4. Update this README's table and `CONTRACT.md`'s status-attestation note in the same PR. + +## Self-test + +This module has its own test that runs the suite against a stub CLI in `internal/stubcli/`. The stub is intentionally narrow — it exists so `go test ./...` from this module's root proves the library compiles and the assertions fire correctly, without depending on any of the real export-CLIs. Failures in the self-test mean the library has a bug; failures in an exporter's compat test mean the exporter drifted from the contract. diff --git a/compat/compat.go b/compat/compat.go new file mode 100644 index 0000000..c87101e --- /dev/null +++ b/compat/compat.go @@ -0,0 +1,120 @@ +// Package compat is the cross-CLI conformance test library for the +// quantcli export-CLI contract. Each *-export-cli imports the subpackages +// (e.g. compat/dates) and runs them against its own built binary in CI. +// +// The library is deliberately black-box: it shells out to the binary under +// test and asserts on stdout, stderr, and exit code. It never imports a +// CLI's internal packages. Adding a new contract test means adding a new +// subpackage here; every exporter then picks it up by adding a one-line +// test entry point. +// +// See CONTRACT.md in the parent repository for the surface this library +// pins down. +package compat + +import ( + "bytes" + "context" + "errors" + "fmt" + "os/exec" + "testing" + "time" +) + +// Runner invokes a single *-export-cli binary in a controlled environment. +// A zero-value Runner is not usable; Binary must be set to an absolute path. +type Runner struct { + // Binary is the absolute path to the export-cli binary under test. + Binary string + + // Env is the environment passed to the binary. If nil, an empty + // environment is used. Tests should set this explicitly so behavior + // does not depend on whatever happens to be in the CI runner's + // environment (notably PATH, HOME, TZ, and any *_TOKEN credentials). + Env []string + + // Timeout is the per-invocation timeout. Zero means 10 seconds. + Timeout time.Duration +} + +// Result captures everything observable about one CLI invocation. All +// compat-test assertions operate on these three fields. +type Result struct { + Stdout []byte + Stderr []byte + ExitCode int +} + +// StdoutString returns Stdout as a string. +func (r Result) StdoutString() string { return string(r.Stdout) } + +// StderrString returns Stderr as a string. +func (r Result) StderrString() string { return string(r.Stderr) } + +// Run invokes the binary with the given args and returns its observable +// output. A non-zero exit code is NOT returned as an error — compat tests +// frequently assert on non-zero exits, so the caller decides what counts +// as a failure. ctx cancellation, process-start failure, and timeouts are +// returned as errors. +func (r Runner) Run(ctx context.Context, args ...string) (Result, error) { + if r.Binary == "" { + return Result{}, errors.New("compat: Runner.Binary is empty") + } + timeout := r.Timeout + if timeout == 0 { + timeout = 10 * time.Second + } + runCtx, cancel := context.WithTimeout(ctx, timeout) + defer cancel() + + cmd := exec.CommandContext(runCtx, r.Binary, args...) + // Default to an empty env so tests are hermetic. Callers opt into + // passing TZ, HOME, etc. via Runner.Env. + if r.Env != nil { + cmd.Env = append([]string(nil), r.Env...) + } else { + cmd.Env = []string{} + } + + var stdout, stderr bytes.Buffer + cmd.Stdout = &stdout + cmd.Stderr = &stderr + + err := cmd.Run() + res := Result{Stdout: stdout.Bytes(), Stderr: stderr.Bytes()} + if err == nil { + res.ExitCode = 0 + return res, nil + } + var exitErr *exec.ExitError + if errors.As(err, &exitErr) { + res.ExitCode = exitErr.ExitCode() + return res, nil + } + if errors.Is(runCtx.Err(), context.DeadlineExceeded) { + return res, fmt.Errorf("compat: %s timed out after %s", r.Binary, timeout) + } + return res, fmt.Errorf("compat: failed to run %s: %w", r.Binary, err) +} + +// MustRun is the testing-helper equivalent of Run: it fails the test on +// any non-exit-code error (timeout, missing binary, etc.) and returns the +// Result on success. +func (r Runner) MustRun(t *testing.T, args ...string) Result { + t.Helper() + res, err := r.Run(context.Background(), args...) + if err != nil { + t.Fatalf("compat: %v", err) + } + return res +} + +// WithEnv returns a copy of r with environment variable KEY=VALUE pairs +// appended. Useful for setting TZ on a per-test basis without mutating +// the receiver. +func (r Runner) WithEnv(kv ...string) Runner { + out := r + out.Env = append(append([]string(nil), r.Env...), kv...) + return out +} diff --git a/compat/dates/dates.go b/compat/dates/dates.go new file mode 100644 index 0000000..861a2e6 --- /dev/null +++ b/compat/dates/dates.go @@ -0,0 +1,185 @@ +// Package dates is the compat test bundle for §2 (timezone policy) and §3 +// (date flags) of CONTRACT.md. +// +// What is machine-attested here: +// +// - The CLI documents `--since` and `--until` in its top-level help. +// - An invalid `--since` value produces a non-zero exit with an error +// on stderr and an empty stdout. +// - A flag-validation failure does not perform a network request. +// - `--help` exits zero and does not perform a network request. +// +// What is intentionally NOT machine-attested yet: +// +// - The actual local-midnight semantics of `--since 2026-04-15` (i.e. +// "the window starts at local midnight, not UTC midnight"). Asserting +// this black-box requires either a `--print-resolved-window` (or +// equivalent) affordance on every CLI, or a recorded HTTP fixture per +// upstream. Both are out of scope for the first compat-test cut and +// tracked separately in quantcli/common. +// +// Exporter usage: +// +// //go:build compat +// package mycli_compat_test +// +// import ( +// "os" +// "testing" +// "github.com/quantcli/common/compat" +// "github.com/quantcli/common/compat/dates" +// ) +// +// func TestContractDates(t *testing.T) { +// bin := os.Getenv("EXPORT_CLI_BIN") +// if bin == "" { t.Skip("EXPORT_CLI_BIN not set") } +// dates.RunContract(t, compat.Runner{Binary: bin}) +// } +package dates + +import ( + "context" + "strings" + "testing" + + "github.com/quantcli/common/compat" +) + +// RunContract runs the full date-flag contract test bundle against r. +// It is the only function exporters are expected to call. +// +// Each assertion is a t.Run subtest, so a failure in one does not mask +// the others. The bundle is safe to run in parallel with other compat +// suites; individual subtests are not marked parallel because they +// shell out to the same binary and the OS may serialize them anyway. +func RunContract(t *testing.T, r compat.Runner) { + t.Helper() + if r.Binary == "" { + t.Fatal("dates: compat.Runner.Binary is empty") + } + + t.Run("HelpDocumentsDateFlags", func(t *testing.T) { + helpDocumentsDateFlags(t, r) + }) + t.Run("InvalidSinceValueFails", func(t *testing.T) { + invalidSinceValueFails(t, r) + }) + t.Run("HelpIsHermetic", func(t *testing.T) { + helpIsHermetic(t, r) + }) + t.Run("FlagValidationIsHermetic", func(t *testing.T) { + flagValidationIsHermetic(t, r) + }) +} + +// helpDocumentsDateFlags asserts that the CLI documents `--since` and +// `--until` somewhere in its top-level `--help` output. This is the +// minimum binding between the contract and the binary: an exporter that +// quietly drops one of the flags will fail this test. +func helpDocumentsDateFlags(t *testing.T, r compat.Runner) { + t.Helper() + res := r.MustRun(t, "--help") + if res.ExitCode != 0 { + t.Fatalf("--help exited %d, want 0; stderr=%q", res.ExitCode, res.StderrString()) + } + // Some CLIs route global flag docs through stderr or through a + // subcommand-listing screen, so we check both streams. The contract + // only requires that the flags are documented and exit code is zero. + combined := res.StdoutString() + "\n" + res.StderrString() + for _, flag := range []string{"--since", "--until"} { + if !strings.Contains(combined, flag) { + t.Errorf("--help output does not mention %s; got stdout=%q stderr=%q", + flag, res.StdoutString(), res.StderrString()) + } + } +} + +// invalidSinceValueFails asserts that a malformed `--since` value causes +// the CLI to exit non-zero with an error on stderr and an empty stdout. +// +// The flag is passed to whatever subcommand the CLI puts first; we use a +// known-bad value so any subcommand that accepts `--since` will reject +// it at parse time. If a CLI accepts `--since` only on subcommands, the +// integrator overrides this test by passing their own subcommand name +// via a future SubcommandHint hook — for now the contract requires that +// at least one entry point rejects malformed dates. +func invalidSinceValueFails(t *testing.T, r compat.Runner) { + t.Helper() + // "obviously-not-a-date" should never parse as a keyword, absolute + // date, or relative duration under any contract-compliant CLI. + args := []string{"--since", "obviously-not-a-date", "--until", "obviously-not-a-date"} + res, err := r.Run(context.Background(), args...) + if err != nil { + t.Fatalf("run failed: %v", err) + } + if res.ExitCode == 0 { + t.Errorf("invalid --since accepted (exit 0); stdout=%q stderr=%q", + res.StdoutString(), res.StderrString()) + } + if len(res.Stdout) != 0 { + // CONTRACT §4: stdout is "data only". A flag-validation error + // must not contaminate the data stream. + t.Errorf("invalid --since produced stdout output (contract §4 violation): %q", + res.StdoutString()) + } + if strings.TrimSpace(res.StderrString()) == "" { + t.Errorf("invalid --since produced no stderr message") + } +} + +// helpIsHermetic asserts that `--help` makes no outbound network request. +// We enforce this by pointing every common proxy env var at an +// unreachable address. If the CLI tries to hit the network at all, it +// will fail or hang — the latter is caught by the Runner timeout. +// +// CONTRACT §5: "A CLI run with --help or with a flag-validation failure +// must not make network requests." +func helpIsHermetic(t *testing.T, r compat.Runner) { + t.Helper() + res := r.WithEnv(noNetworkEnv()...).MustRun(t, "--help") + if res.ExitCode != 0 { + t.Errorf("--help exited %d with no-network env; stderr=%q", res.ExitCode, res.StderrString()) + } +} + +// flagValidationIsHermetic is the other half of CONTRACT §5: a parse +// failure must not have already opened a connection. +func flagValidationIsHermetic(t *testing.T, r compat.Runner) { + t.Helper() + args := []string{"--since", "obviously-not-a-date", "--until", "obviously-not-a-date"} + res, err := r.WithEnv(noNetworkEnv()...).Run(context.Background(), args...) + if err != nil { + t.Fatalf("run failed under no-network env: %v", err) + } + if res.ExitCode == 0 { + t.Errorf("invalid --since accepted under no-network env (exit 0); stderr=%q", + res.StderrString()) + } + // If the CLI dialed out before parsing, stderr would typically + // contain a proxy or DNS error rather than a parse error. We don't + // pattern-match the message (each CLI phrases it differently) — we + // only require that the process did not stall and did exit non-zero, + // which is already covered above. The proxy variables exist as a + // belt-and-suspenders: any HTTP client honoring them will fail + // loudly instead of silently reaching the real upstream. +} + +// noNetworkEnv returns a slice of KEY=VALUE strings that point common +// HTTP proxy variables at an address guaranteed not to resolve. Any +// Go HTTP client built on net/http will honor at least HTTP_PROXY and +// HTTPS_PROXY; we set the lowercase variants too for libraries that +// reach into env directly. +func noNetworkEnv() []string { + const unreachable = "http://127.0.0.1:1" + return []string{ + "HTTP_PROXY=" + unreachable, + "HTTPS_PROXY=" + unreachable, + "http_proxy=" + unreachable, + "https_proxy=" + unreachable, + "NO_PROXY=", + "no_proxy=", + // Force a deterministic timezone so CLIs that read TZ on + // startup behave predictably across runners. + "TZ=UTC", + } +} diff --git a/compat/dates/dates_test.go b/compat/dates/dates_test.go new file mode 100644 index 0000000..3ddda90 --- /dev/null +++ b/compat/dates/dates_test.go @@ -0,0 +1,36 @@ +package dates_test + +import ( + "os/exec" + "path/filepath" + "runtime" + "testing" + + "github.com/quantcli/common/compat" + "github.com/quantcli/common/compat/dates" +) + +// TestRunContract_AgainstStub builds the in-tree stub binary and runs +// the compat suite against it. This is the library's own gate: if the +// stub (which is contract-compliant by construction) starts failing +// these tests, the library has a bug, not the stub. +func TestRunContract_AgainstStub(t *testing.T) { + bin := buildStub(t) + dates.RunContract(t, compat.Runner{Binary: bin}) +} + +// buildStub compiles the stub CLI into a temp directory and returns the +// absolute path. It uses `go build` rather than relying on a checked-in +// binary so the test is reproducible across platforms. +func buildStub(t *testing.T) string { + t.Helper() + out := filepath.Join(t.TempDir(), "stubcli") + if runtime.GOOS == "windows" { + out += ".exe" + } + cmd := exec.Command("go", "build", "-o", out, "github.com/quantcli/common/compat/internal/stubcli") + if output, err := cmd.CombinedOutput(); err != nil { + t.Fatalf("go build stubcli failed: %v\n%s", err, output) + } + return out +} diff --git a/compat/go.mod b/compat/go.mod new file mode 100644 index 0000000..5408261 --- /dev/null +++ b/compat/go.mod @@ -0,0 +1,3 @@ +module github.com/quantcli/common/compat + +go 1.21 diff --git a/compat/internal/stubcli/main.go b/compat/internal/stubcli/main.go new file mode 100644 index 0000000..988dcee --- /dev/null +++ b/compat/internal/stubcli/main.go @@ -0,0 +1,65 @@ +// Command stubcli is a tiny contract-compliant stand-in used by the +// compat library's own tests. It is NOT a usable export-cli. It exists +// only so the dates compat suite can run against a known-good binary in +// quantcli/common's CI, proving the library itself works end-to-end +// before any real exporter wires it up. +// +// Two behaviors: +// - `--help`: prints a help string mentioning --since and --until, exits 0. +// - anything else: parses --since/--until; rejects "obviously-not-a-date" +// with a stderr message and exit code 2. +// +// It never makes a network request. +package main + +import ( + "flag" + "fmt" + "os" +) + +func main() { + if len(os.Args) > 1 && os.Args[1] == "--help" { + fmt.Fprintln(os.Stdout, "stubcli — contract-compliant test stand-in") + fmt.Fprintln(os.Stdout) + fmt.Fprintln(os.Stdout, " --since VALUE inclusive lower bound (local date)") + fmt.Fprintln(os.Stdout, " --until VALUE inclusive upper bound (local date)") + os.Exit(0) + } + + fs := flag.NewFlagSet("stubcli", flag.ContinueOnError) + fs.SetOutput(os.Stderr) + since := fs.String("since", "", "inclusive lower bound") + until := fs.String("until", "", "inclusive upper bound") + if err := fs.Parse(os.Args[1:]); err != nil { + os.Exit(2) + } + + if !validDate(*since) { + fmt.Fprintf(os.Stderr, "error: invalid value for --since: %q\n", *since) + os.Exit(2) + } + if !validDate(*until) { + fmt.Fprintf(os.Stderr, "error: invalid value for --until: %q\n", *until) + os.Exit(2) + } + // Real export-CLIs would emit data here. The stub stays silent; + // no compat test in this package exercises the data path. +} + +// validDate is intentionally narrow: it accepts only what the stub needs +// to fail the compat suite's "obviously-not-a-date" probe. It is not a +// faithful implementation of CONTRACT.md §3 parsing. +func validDate(s string) bool { + if s == "" { + return false + } + switch s { + case "today", "yesterday": + return true + } + if len(s) == 10 && s[4] == '-' && s[7] == '-' { + return true // YYYY-MM-DD shape; we do not validate the calendar. + } + return false +} From 66df1474c84ef7a9eccc3663218a08434a1ba08f Mon Sep 17 00:00:00 2001 From: LeadGoEngineer Date: Sun, 10 May 2026 18:48:17 -0400 Subject: [PATCH 2/3] feat(compat): add Subcommands affordance to Runner for cobra-based CLIs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The original Runner shape assumed --since/--until lived on the root binary. crono is cobra-based — date flags live on `biometrics`, `exercises`, `nutrition`, `servings`, `notes` — so the dates suite red-x'd HelpDocumentsDateFlags against it. Liftoff and Withings will hit the same pattern. Changes: - Runner gains a Subcommands []string field declaring the subcommands under which the contract surface lives, plus a WithSubcommand(name) composition helper that prepends the subcommand to argv on Run. - dates.RunContract dispatches per-subcommand when Subcommands is non-empty, scoping each pass under `subcommand=NAME/...` so any single-subcommand regression surfaces as a named failure rather than masking the rest. Flat CLIs leave Subcommands empty and the bundle runs against the root binary unchanged. - stubcli grows a second mode (STUBCLI_MODE=cobra) that mirrors a cobra-based CLI: root --help lists subcommands without mentioning the date flags, and only `biometrics` carries --since/--until. The cobra-mode self-test fails fast if the Runner ever stops prepending the subcommand. - New compat/internal/argecho helper and compat_test.go give the WithSubcommand prepend a focused unit test against a binary that just echoes os.Args. - README + CONTRIBUTING document the affordance and the cobra-style integration shape. Addresses LeadGoEngineer review on PR #6 — the "first machine-attested contract test" should not ship with a known gap that prevents its only consumer from going green. Refs CONTRACT.md §3. Co-Authored-By: Paperclip --- CONTRIBUTING.md | 1 + compat/README.md | 27 ++++++++- compat/compat.go | 48 +++++++++++++++- compat/compat_test.go | 98 +++++++++++++++++++++++++++++++++ compat/dates/dates.go | 49 +++++++++++++---- compat/dates/dates_test.go | 30 ++++++++-- compat/internal/argecho/main.go | 17 ++++++ compat/internal/stubcli/main.go | 96 +++++++++++++++++++++++++++++--- 8 files changed, 340 insertions(+), 26 deletions(-) create mode 100644 compat/compat_test.go create mode 100644 compat/internal/argecho/main.go diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index a54999e..f38ee6f 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -76,6 +76,7 @@ Rules: - Anyone changing `CONTRACT.md` is also expected to update or add tests under `compat/` that exercise the new behavior against every `*-export-cli`. - The harness is deliberately black-box: it shells out to the binary and asserts on stdout, stderr, and exit code only. It must not import a CLI's internal packages. - One subpackage per contract section (`compat/dates`, future `compat/formats`, `compat/auth`, `compat/prime`). Each exposes a single entry point — `RunContract(t, runner)` — that exporters call from one build-tagged `_test.go` file. +- Cobra-based exporters whose contract surface lives on subcommands set `compat.Runner.Subcommands`; section bundles dispatch per-subcommand under a `subcommand=NAME/...` subtree. Flat CLIs leave the field empty and the bundle runs against the root binary. - A PR that changes the contract without touching `compat/` is incomplete. Either update the tests in the same PR or open a follow-up issue and link it from the PR body before merging — the Lead Go Engineer holds the line on this. - Compat tests run in CI on every PR and on `main`. A failing compat test on `main` means at least one shipped CLI no longer matches the contract, and that's a release-blocker incident, not a flake. - The Status table in `CONTRACT.md` distinguishes **machine-attested** rows (covered by `compat/`) from **human-attested** rows (still verified by reviewer judgment). Promoting a row from human to machine attestation is itself a worthwhile PR. diff --git a/compat/README.md b/compat/README.md index 54e2668..2ada722 100644 --- a/compat/README.md +++ b/compat/README.md @@ -37,7 +37,28 @@ func TestContractDates(t *testing.T) { } ``` -Then in your CI workflow: +### Cobra-based CLIs: date flags on subcommands + +When `--since`/`--until` live on subcommands (the crono / liftoff / withings pattern), set `Subcommands` and the suite will dispatch per-subcommand: + +```go +func TestContractDates(t *testing.T) { + bin := os.Getenv("EXPORT_CLI_BIN") + if bin == "" { + t.Skip("EXPORT_CLI_BIN not set; skipping compat suite") + } + dates.RunContract(t, compat.Runner{ + Binary: bin, + Subcommands: []string{ + "biometrics", "exercises", "nutrition", "servings", "notes", + }, + }) +} +``` + +Each subcommand is verified under a `subcommand=NAME/...` subtree, so a regression in any single one fails as a named subtest instead of masking the rest. + +### CI workflow ```yaml - name: build @@ -59,6 +80,8 @@ The exporter does not need a separate `go.mod` for compat tests — the standard | `HelpIsHermetic` | §5 | `--help` succeeds with all HTTP proxies pointed at an unreachable address. | | `FlagValidationIsHermetic` | §5 | A parse failure also produces no successful outbound request. | +When `compat.Runner.Subcommands` is set, every row above runs once per declared subcommand under `subcommand=NAME/...`. + ## What it does NOT cover yet The actual local-midnight semantics of `--since 2026-04-15` (the harmonization that just landed across crono/liftoff/withings) is still **human-attested** in the status table. Asserting it black-box requires either: @@ -78,3 +101,5 @@ When that affordance lands, the test belongs here as `dates.LocalMidnightSemanti ## Self-test This module has its own test that runs the suite against a stub CLI in `internal/stubcli/`. The stub is intentionally narrow — it exists so `go test ./...` from this module's root proves the library compiles and the assertions fire correctly, without depending on any of the real export-CLIs. Failures in the self-test mean the library has a bug; failures in an exporter's compat test mean the exporter drifted from the contract. + +The stub has two modes (`STUBCLI_MODE=flat` and `STUBCLI_MODE=cobra`). The flat-mode self-test exercises the original Runner shape; the cobra-mode self-test exercises `Subcommands`-based dispatch. In cobra mode, the stub's root `--help` deliberately omits `--since/--until`, so the cobra-mode self-test fails fast if `compat.Runner` ever stops prepending the subcommand. There is also a focused unit test for `Runner.WithSubcommand` using an `argecho` helper that just prints `os.Args`. diff --git a/compat/compat.go b/compat/compat.go index c87101e..e5441df 100644 --- a/compat/compat.go +++ b/compat/compat.go @@ -36,6 +36,29 @@ type Runner struct { // Timeout is the per-invocation timeout. Zero means 10 seconds. Timeout time.Duration + + // Subcommands declares the subcommands under which the contract + // surface lives — for CLIs (typically cobra-based) where flags like + // --since and --until are attached to data-producing subcommands + // rather than the root binary. Examples: crono's `biometrics`, + // `exercises`, `nutrition`, `servings`, `notes` each accept their + // own --since/--until. + // + // Empty means the surface is on the root binary; section bundles + // invoke the binary directly. Non-empty means each section's + // RunContract iterates the list and verifies the contract once per + // subcommand via t.Run, so a regression in any single subcommand + // surfaces as a named subtest failure rather than masking the rest. + // + // The Runner itself does not look at this field; section bundles + // (e.g. compat/dates) read it and dispatch via WithSubcommand. + Subcommands []string + + // subcommand, when non-empty, is prepended to args on every Run + // call. Set via WithSubcommand; section bundles use it to dispatch + // per-subcommand. Callers do not need to set it directly — set + // Subcommands instead and let the bundle compose the dispatch. + subcommand string } // Result captures everything observable about one CLI invocation. All @@ -68,7 +91,11 @@ func (r Runner) Run(ctx context.Context, args ...string) (Result, error) { runCtx, cancel := context.WithTimeout(ctx, timeout) defer cancel() - cmd := exec.CommandContext(runCtx, r.Binary, args...) + fullArgs := args + if r.subcommand != "" { + fullArgs = append([]string{r.subcommand}, args...) + } + cmd := exec.CommandContext(runCtx, r.Binary, fullArgs...) // Default to an empty env so tests are hermetic. Callers opt into // passing TZ, HOME, etc. via Runner.Env. if r.Env != nil { @@ -118,3 +145,22 @@ func (r Runner) WithEnv(kv ...string) Runner { out.Env = append(append([]string(nil), r.Env...), kv...) return out } + +// WithSubcommand returns a copy of r whose Run prepends sub as the +// first command-line argument. Section bundles use this internally to +// dispatch per-subcommand when Runner.Subcommands is non-empty; +// integrators normally set Subcommands and let the bundle do it. +// +// Calling WithSubcommand again replaces (not stacks) the previous +// value; nested subcommand paths are out of scope for the current +// contract. +func (r Runner) WithSubcommand(sub string) Runner { + out := r + out.subcommand = sub + return out +} + +// Subcommand returns the subcommand that Run will prepend to args, or +// the empty string if none is set. Section bundles use this in subtest +// names so failures point at the offending subcommand. +func (r Runner) Subcommand() string { return r.subcommand } diff --git a/compat/compat_test.go b/compat/compat_test.go new file mode 100644 index 0000000..ade8d91 --- /dev/null +++ b/compat/compat_test.go @@ -0,0 +1,98 @@ +package compat_test + +import ( + "context" + "os/exec" + "path/filepath" + "runtime" + "strings" + "testing" + "time" + + "github.com/quantcli/common/compat" +) + +// TestWithSubcommand_PrependsArg checks that WithSubcommand causes Run +// to emit the subcommand as argv[1] in front of the caller's args. +// We use a small Go program that echoes its os.Args back so the assertion +// is independent of any system command's flag semantics. +func TestWithSubcommand_PrependsArg(t *testing.T) { + bin := buildArgEcho(t) + r := compat.Runner{Binary: bin}.WithSubcommand("biometrics") + + res, err := r.Run(context.Background(), "--help") + if err != nil { + t.Fatalf("run: %v", err) + } + if res.ExitCode != 0 { + t.Fatalf("exit %d; stderr=%q", res.ExitCode, res.StderrString()) + } + got := strings.TrimRight(res.StdoutString(), "\n") + want := "biometrics\n--help" + if got != want { + t.Errorf("argv mismatch:\n got:\n%s\nwant:\n%s", got, want) + } +} + +// TestRun_NoSubcommandPassthrough checks that the default zero +// subcommand leaves args untouched. +func TestRun_NoSubcommandPassthrough(t *testing.T) { + bin := buildArgEcho(t) + r := compat.Runner{Binary: bin} + + res, err := r.Run(context.Background(), "--help") + if err != nil { + t.Fatalf("run: %v", err) + } + got := strings.TrimRight(res.StdoutString(), "\n") + if got != "--help" { + t.Errorf("argv mismatch: got %q want %q", got, "--help") + } +} + +// TestWithSubcommand_DoesNotMutateReceiver asserts that WithSubcommand +// returns a copy and leaves the parent runner unchanged. Section +// bundles rely on this when iterating Subcommands. +func TestWithSubcommand_DoesNotMutateReceiver(t *testing.T) { + bin := buildArgEcho(t) + parent := compat.Runner{Binary: bin} + _ = parent.WithSubcommand("biometrics") + if parent.Subcommand() != "" { + t.Errorf("parent.Subcommand() = %q; want empty after WithSubcommand on copy", parent.Subcommand()) + } +} + +// TestRun_TimeoutReturnsError exercises the timeout branch so the +// Runner's error contract (non-zero exit is not an error; timeout is) +// stays load-bearing. +func TestRun_TimeoutReturnsError(t *testing.T) { + sleeper, err := exec.LookPath("sleep") + if err != nil { + t.Skip("sleep not on PATH; skipping timeout exercise") + } + r := compat.Runner{Binary: sleeper, Timeout: 50 * time.Millisecond} + _, runErr := r.Run(context.Background(), "5") + if runErr == nil { + t.Fatal("expected timeout error, got nil") + } + if !strings.Contains(runErr.Error(), "timed out") { + t.Errorf("expected timeout error, got: %v", runErr) + } +} + +// buildArgEcho compiles the tiny argecho helper into a temp dir. It is +// the simplest possible echo-args binary: it prints each os.Args[1:] +// entry on its own line to stdout. We build it instead of relying on +// /bin/echo so the test works on any GOOS. +func buildArgEcho(t *testing.T) string { + t.Helper() + out := filepath.Join(t.TempDir(), "argecho") + if runtime.GOOS == "windows" { + out += ".exe" + } + cmd := exec.Command("go", "build", "-o", out, "github.com/quantcli/common/compat/internal/argecho") + if output, err := cmd.CombinedOutput(); err != nil { + t.Fatalf("go build argecho: %v\n%s", err, output) + } + return out +} diff --git a/compat/dates/dates.go b/compat/dates/dates.go index 861a2e6..1277811 100644 --- a/compat/dates/dates.go +++ b/compat/dates/dates.go @@ -3,7 +3,10 @@ // // What is machine-attested here: // -// - The CLI documents `--since` and `--until` in its top-level help. +// - The CLI documents `--since` and `--until` in the help output of +// whatever entry point owns them — the root binary for flat CLIs, +// or each declared subcommand for cobra-based CLIs (set via +// compat.Runner.Subcommands). // - An invalid `--since` value produces a non-zero exit with an error // on stderr and an empty stdout. // - A flag-validation failure does not perform a network request. @@ -52,12 +55,37 @@ import ( // the others. The bundle is safe to run in parallel with other compat // suites; individual subtests are not marked parallel because they // shell out to the same binary and the OS may serialize them anyway. +// +// If r.Subcommands is non-empty, RunContract iterates the list and +// runs the four assertions once per subcommand under a +// "subcommand=NAME" t.Run group. This is how cobra-based CLIs (e.g. +// crono's `biometrics`, `exercises`, `nutrition`, `servings`, `notes`) +// attest the contract on every data-producing subcommand. If empty, +// the assertions run once against the root binary — the right shape +// for CLIs that put --since/--until at the top level. func RunContract(t *testing.T, r compat.Runner) { t.Helper() if r.Binary == "" { t.Fatal("dates: compat.Runner.Binary is empty") } + if len(r.Subcommands) == 0 { + runContractOne(t, r) + return + } + for _, sub := range r.Subcommands { + sub := sub + t.Run("subcommand="+sub, func(t *testing.T) { + runContractOne(t, r.WithSubcommand(sub)) + }) + } +} + +// runContractOne runs the four date-flag assertions against a single +// invocation surface — either the root binary (when r has no +// subcommand prefix) or a specific subcommand of it. +func runContractOne(t *testing.T, r compat.Runner) { + t.Helper() t.Run("HelpDocumentsDateFlags", func(t *testing.T) { helpDocumentsDateFlags(t, r) }) @@ -73,9 +101,11 @@ func RunContract(t *testing.T, r compat.Runner) { } // helpDocumentsDateFlags asserts that the CLI documents `--since` and -// `--until` somewhere in its top-level `--help` output. This is the -// minimum binding between the contract and the binary: an exporter that -// quietly drops one of the flags will fail this test. +// `--until` somewhere in the `--help` output of the configured entry +// point — root binary or subcommand, depending on how the integrator +// configured compat.Runner. This is the minimum binding between the +// contract and the binary: an exporter that quietly drops one of the +// flags will fail this test. func helpDocumentsDateFlags(t *testing.T, r compat.Runner) { t.Helper() res := r.MustRun(t, "--help") @@ -97,12 +127,11 @@ func helpDocumentsDateFlags(t *testing.T, r compat.Runner) { // invalidSinceValueFails asserts that a malformed `--since` value causes // the CLI to exit non-zero with an error on stderr and an empty stdout. // -// The flag is passed to whatever subcommand the CLI puts first; we use a -// known-bad value so any subcommand that accepts `--since` will reject -// it at parse time. If a CLI accepts `--since` only on subcommands, the -// integrator overrides this test by passing their own subcommand name -// via a future SubcommandHint hook — for now the contract requires that -// at least one entry point rejects malformed dates. +// We use a known-bad value so any entry point that accepts `--since` +// will reject it at parse time. For cobra-based CLIs whose date flags +// live on subcommands, the integrator sets compat.Runner.Subcommands; +// RunContract then dispatches via WithSubcommand and this assertion +// runs once per declared subcommand. func invalidSinceValueFails(t *testing.T, r compat.Runner) { t.Helper() // "obviously-not-a-date" should never parse as a keyword, absolute diff --git a/compat/dates/dates_test.go b/compat/dates/dates_test.go index 3ddda90..e0ab7f1 100644 --- a/compat/dates/dates_test.go +++ b/compat/dates/dates_test.go @@ -10,15 +10,35 @@ import ( "github.com/quantcli/common/compat/dates" ) -// TestRunContract_AgainstStub builds the in-tree stub binary and runs -// the compat suite against it. This is the library's own gate: if the -// stub (which is contract-compliant by construction) starts failing -// these tests, the library has a bug, not the stub. -func TestRunContract_AgainstStub(t *testing.T) { +// TestRunContract_AgainstStubFlat builds the in-tree stub binary in its +// default (flat) mode and runs the compat suite against it. This is the +// library's own gate for the original Runner shape: if the stub (which +// is contract-compliant by construction) starts failing these tests, +// the library has a bug, not the stub. +func TestRunContract_AgainstStubFlat(t *testing.T) { bin := buildStub(t) dates.RunContract(t, compat.Runner{Binary: bin}) } +// TestRunContract_AgainstStubSubcommand runs the same suite against +// the stub in cobra mode, with a Subcommands declaration so the Runner +// dispatches per-subcommand. In cobra mode the stub's root --help does +// NOT mention --since/--until — only `biometrics --help` does — so this +// test fails fast if compat.Runner ever stops prepending the +// subcommand. +// +// This is the gate that pins down the contract for crono / liftoff / +// withings, whose date flags live on cobra subcommands. +func TestRunContract_AgainstStubSubcommand(t *testing.T) { + bin := buildStub(t) + r := compat.Runner{ + Binary: bin, + Env: []string{"STUBCLI_MODE=cobra"}, + Subcommands: []string{"biometrics"}, + } + dates.RunContract(t, r) +} + // buildStub compiles the stub CLI into a temp directory and returns the // absolute path. It uses `go build` rather than relying on a checked-in // binary so the test is reproducible across platforms. diff --git a/compat/internal/argecho/main.go b/compat/internal/argecho/main.go new file mode 100644 index 0000000..cbaf26f --- /dev/null +++ b/compat/internal/argecho/main.go @@ -0,0 +1,17 @@ +// Command argecho prints each of its os.Args[1:] entries on its own +// line to stdout, then exits zero. It is used by compat_test.go to +// verify how Runner constructs argv (notably the WithSubcommand +// prepend behavior) without depending on any system command's flag +// parsing. +package main + +import ( + "fmt" + "os" +) + +func main() { + for _, a := range os.Args[1:] { + fmt.Println(a) + } +} diff --git a/compat/internal/stubcli/main.go b/compat/internal/stubcli/main.go index 988dcee..c233922 100644 --- a/compat/internal/stubcli/main.go +++ b/compat/internal/stubcli/main.go @@ -4,12 +4,25 @@ // quantcli/common's CI, proving the library itself works end-to-end // before any real exporter wires it up. // -// Two behaviors: -// - `--help`: prints a help string mentioning --since and --until, exits 0. -// - anything else: parses --since/--until; rejects "obviously-not-a-date" -// with a stderr message and exit code 2. +// stubcli has two modes, selected by the STUBCLI_MODE env var: // -// It never makes a network request. +// - "" (default) / "flat": date flags live on the root binary, +// mirroring single-purpose CLIs. Root --help mentions --since and +// --until and exits 0. +// - "cobra": date flags live on a `biometrics` subcommand, mirroring +// cobra-based exporters (crono, liftoff, withings). Root --help +// lists subcommands but does NOT mention --since/--until; passing +// --since to the root binary fails. `biometrics --help` mentions +// them and the parse path is the same. +// +// The two modes let the dates compat suite self-test both Runner +// shapes: a flat Runner against the default mode, and a Runner with +// Subcommands=["biometrics"] against cobra mode. If the subcommand +// dispatch in compat.Runner regresses, the cobra-mode self-test fails +// at HelpDocumentsDateFlags because root --help no longer carries the +// flags. +// +// stubcli never makes a network request. package main import ( @@ -18,20 +31,85 @@ import ( "os" ) +const cobraSubcommand = "biometrics" + func main() { - if len(os.Args) > 1 && os.Args[1] == "--help" { - fmt.Fprintln(os.Stdout, "stubcli — contract-compliant test stand-in") + mode := os.Getenv("STUBCLI_MODE") + args := os.Args[1:] + + switch mode { + case "", "flat": + runFlat(args) + case "cobra": + runCobra(args) + default: + fmt.Fprintf(os.Stderr, "error: unknown STUBCLI_MODE=%q\n", mode) + os.Exit(2) + } +} + +// runFlat is the original stubcli behavior: --since and --until parse +// at the root binary. Used by the flat-mode self-test. +func runFlat(args []string) { + if len(args) > 0 && args[0] == "--help" { + fmt.Fprintln(os.Stdout, "stubcli — contract-compliant test stand-in (flat mode)") + fmt.Fprintln(os.Stdout) + fmt.Fprintln(os.Stdout, " --since VALUE inclusive lower bound (local date)") + fmt.Fprintln(os.Stdout, " --until VALUE inclusive upper bound (local date)") + os.Exit(0) + } + parseDateFlags("stubcli", args) +} + +// runCobra is the subcommand-style behavior: root --help lists +// subcommands without mentioning the date flags; only the named +// subcommand owns --since/--until. Mirrors how cobra-based CLIs (e.g. +// crono biometrics, liftoff workouts) expose the contract surface. +func runCobra(args []string) { + if len(args) == 0 { + fmt.Fprintln(os.Stderr, "error: subcommand required") + os.Exit(2) + } + + switch args[0] { + case "--help": + // Root help intentionally omits --since/--until. This is what + // proves the subcommand dispatch is real: if compat.Runner + // silently drops the subcommand prefix, HelpDocumentsDateFlags + // runs against this output and fails. + fmt.Fprintln(os.Stdout, "stubcli — contract-compliant test stand-in (cobra mode)") + fmt.Fprintln(os.Stdout) + fmt.Fprintln(os.Stdout, "Available subcommands:") + fmt.Fprintln(os.Stdout, " "+cobraSubcommand+" sample data subcommand (owns --since/--until)") + os.Exit(0) + case cobraSubcommand: + // fall through to subcommand-arg handling + default: + fmt.Fprintf(os.Stderr, "error: unknown subcommand %q\n", args[0]) + os.Exit(2) + } + + subArgs := args[1:] + if len(subArgs) > 0 && subArgs[0] == "--help" { + fmt.Fprintln(os.Stdout, "stubcli "+cobraSubcommand+" — date-flag-owning subcommand") fmt.Fprintln(os.Stdout) fmt.Fprintln(os.Stdout, " --since VALUE inclusive lower bound (local date)") fmt.Fprintln(os.Stdout, " --until VALUE inclusive upper bound (local date)") os.Exit(0) } + parseDateFlags("stubcli "+cobraSubcommand, subArgs) +} - fs := flag.NewFlagSet("stubcli", flag.ContinueOnError) +// parseDateFlags is the shared --since/--until validator. It is used +// by both modes so the parse-error behavior (non-zero exit, stderr +// message, empty stdout) is identical regardless of where the flags +// live. +func parseDateFlags(progName string, args []string) { + fs := flag.NewFlagSet(progName, flag.ContinueOnError) fs.SetOutput(os.Stderr) since := fs.String("since", "", "inclusive lower bound") until := fs.String("until", "", "inclusive upper bound") - if err := fs.Parse(os.Args[1:]); err != nil { + if err := fs.Parse(args); err != nil { os.Exit(2) } From b07b0d70ee59ea442e80a5cd3b6fed94973d3607 Mon Sep 17 00:00:00 2001 From: LeadGoEngineer Date: Sun, 10 May 2026 18:49:26 -0400 Subject: [PATCH 3/3] fix(compat): return error on timeout instead of nil MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit exec.CommandContext kills the process when the deadline expires, and that kill surfaces as an *exec.ExitError on the signal path. The deadline check was after the ExitError check, so timeouts silently returned (res, nil) with a -1 exit code — masking hangs as clean non-zero exits. The package doc already promised timeouts return errors; this just lines the code up with the contract. Surfaced by the new TestRun_TimeoutReturnsError in compat_test.go. Co-Authored-By: Paperclip --- compat/compat.go | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/compat/compat.go b/compat/compat.go index e5441df..ff7c065 100644 --- a/compat/compat.go +++ b/compat/compat.go @@ -114,14 +114,20 @@ func (r Runner) Run(ctx context.Context, args ...string) (Result, error) { res.ExitCode = 0 return res, nil } + // Timeout check first. exec.CommandContext kills the process when + // the deadline expires, which surfaces as an *exec.ExitError on the + // signal path. The package contract promises a non-nil error on + // timeout, so we must detect that case before falling through to + // the ExitError handler — otherwise a hung CLI looks like a clean + // non-zero exit to the caller. + if errors.Is(runCtx.Err(), context.DeadlineExceeded) { + return res, fmt.Errorf("compat: %s timed out after %s", r.Binary, timeout) + } var exitErr *exec.ExitError if errors.As(err, &exitErr) { res.ExitCode = exitErr.ExitCode() return res, nil } - if errors.Is(runCtx.Err(), context.DeadlineExceeded) { - return res, fmt.Errorf("compat: %s timed out after %s", r.Binary, timeout) - } return res, fmt.Errorf("compat: failed to run %s: %w", r.Binary, err) }