Skip to content

m5: CLI + main wiring (internal/cli + cmd/cascade)#10

Merged
oubiwann merged 7 commits into
mainfrom
m5/cli-main-wiring
May 7, 2026
Merged

m5: CLI + main wiring (internal/cli + cmd/cascade)#10
oubiwann merged 7 commits into
mainfrom
m5/cli-main-wiring

Conversation

@oubiwann

@oubiwann oubiwann commented May 7, 2026

Copy link
Copy Markdown
Contributor

What

M5 — wires M2 (pkg/golist) + M3 (pkg/depgraph) + M4 (pkg/changeset) into a usable CLI binary with a documented exit-code contract, signal-driven cancellation, and three function-variable seams that keep the io edges (git diff, go list, signal.NotifyContext) in-process testable.

The pipeline is literally four lines in internal/cli/cli.go:runPipeline:

pkgs, err := runGoListWrapper(ctx, cfg.tags, []string{"./..."}, golist.WithDir(cfg.root))
if err != nil { return nil, err }
g := depgraph.Build(pkgs)
seeds := changeset.Resolve(changedFiles, pkgs, changeset.WithModuleRoot(cfg.root))
return g.RevDepClosure(seeds), nil

This is the structural verification of M4 retro's "trio composes without adapter code" claim — runPipeline is the entire adapter.

Where

  • internal/cli/ (new package) — doc.go, cli.go (Run + parseFlags + loadChangeSet + runPipeline + mapError + signal handling), errors.go (*GitDiffError + ErrGitDiffFailed mirroring pkg/golist/errors.go shape), seam.go (runGitDiff + defaultRunGitDiff + classifyGitDiffError), three test files (cli_test.go external public-API, seam_test.go internal seam-driven, helpers_test.go shared).
  • cmd/cascade/main.go — shrunk from 55 lines to 16 lines; one-line delegation to cli.Run.
  • cmd/cascade/main_test.go — M1 in-process run() tests retired (those scenarios live in internal/cli/cli_test.go now); preserves TestCascadeBinaryVersion (F-16); adds TestCascadeBinaryHelp (F-15) + TestCascadeBinaryEndToEnd (F-17).
  • scripts/coverage-check.shinternal/cli added at index 4 with threshold 100; comment block updated.
  • README.md — CLI-usage section gains a Flag reference table (7 rows) and an Exit codes table (6 rows: 0/1/2/3/4/5).
  • docs/dev/0012-implementation-retrospective-m5-cli-main-wiring.md — closing retro walking F-1..F-22, with CDC review notes already folded in (CDC pass approved this PR pre-merge).

How to verify

make check-all                                           # build + lint + test + coverage gate
go test -race -count=10 -run TestRun_PipelineIntegration ./internal/cli   # F-12 determinism
go test -race -run TestRun_ContextCancellation ./internal/cli             # F-11 cancellation
bash scripts/coverage-check.sh                           # F-13: 5/5 packages at 100%
go test -run TestCascadeBinary ./cmd/cascade             # F-15/F-16/F-17 layer-2

# Specific subtests:
go test -run TestRun_GitDiffFails ./internal/cli         # F-7: exit 2
go test -run TestRun_GoListFails ./internal/cli          # F-8: exit 3
go test -run TestRun_StdinChangedFiles ./internal/cli    # F-9: stdin mode
go test -run TestRun_EmptyResult ./internal/cli          # F-10: empty → exit 0

Disclosed plan-additions vs spec (folded into retro)

The plan added two seams to internal/cli beyond the spec's runGitDiff:

  1. runGoListWrapper = golist.Run. pkg/golist's own runGoList seam is unexported within the package and not reachable from internal/cli's tests. Wrapping golist.Run in an internal seam variable is the structural fix.
  2. signalContext = signal.NotifyContext. Stdlib signal.NotifyContext cannot be cancelled from inside a single-process test without sending real OS signals. The seam lets TestRun_ContextCancellation substitute a pre-cancelled context-creator and exercise the exit-5 path deterministically.

Both follow the established var name = realImpl package-level convention used by M2 (runGoList) and M4 (getCwd). Five seams across the codebase total. CDC's review (folded into the retro pre-merge) ruled both well-justified — structural necessity for in-process testability, not scope creep.

Abridged ledger (full version in docs/dev/0012-implementation-retrospective-m5-cli-main-wiring.md)

ID Criterion Status Evidence
F-1 internal/cli/doc.go exists with // Package cli done First comment line preserved.
F-2 cli.Run signature matches spec done func Run(args []string, stdin io.Reader, stdout, stderr io.Writer) int — exact.
F-3 *GitDiffError + ErrGitDiffFailed exported with documented fields done All three fields (Cmd, ExitCode, Stderr) godoc'd; sentinel + Error/Is/Unwrap mirror pkg/golist.
F-4 cmd/cascade/main.go is a one-liner delegating to cli.Run done 16 lines (< 20); contains os.Exit(cli.Run(os.Args[1:], os.Stdin, os.Stdout, os.Stderr)).
F-5 Flag parsing covers required + optional done 4 named subtests pass: unknown_flag, unexpected_positional, missing_base_no_changed_files, missing_base_with_only_tags.
F-6 Pipeline integration test passes against synthetic packages done Synthetic seams; real depgraph.Build + changeset.Resolve; output ex/pkga\nex/pkgb\n sorted.
F-7 git diff failure → exit 2 done Seam returns *exec.ExitError + git stderr; Run returns 2; stderr propagates.
F-8 go list failure → exit 3 done golist seam returns *golist.ExitError; Run returns 3.
F-9 Stdin mode works correctly done Stdin " pkga/a.go \n\n \n" trimmed, blanks skipped → [ex/pkga].
F-10 Empty result → exit 0 done Stdin README.md\n (non-Go); changeset.Resolve skips; affected empty; exit 0.
F-11 Context cancellation handled cleanly done Pre-cancelled signalContext seam; -race clean; exit 5.
F-12 Output sorted + deduplicated, deterministic done -count=10 clean; inline sort.StringsAreSorted per subtest.
F-13 Per-package coverage gate at 100% on internal/cli done 5/5 cascade packages at 100% (golist, depgraph, changeset, internal/project, internal/cli).
F-14 No non-stdlib imports beyond cascade pkg/* done go list -m all | wc -l = 1.
F-15 cascade --help prints flag reference + exit-code table done TestCascadeBinaryHelp asserts presence of all 7 flags + 6 exit-code lines on stdout.
F-16 cascade --version prints injected ldflags metadata done TestCascadeBinaryVersion (M1 carry-over) green.
F-17 End-to-end pipeline against sample module done TestCascadeBinaryEndToEnd exec's binary against pkg/golist/testdata/sample-module/; stdin = pkga/a.go; affected set contains example.test/sample/pkga + example.test/sample/pkgb.
F-18 go install …@<sha> succeeds deferred Requires merged commit on main (proxy.golang.org indexes only merged refs). Re-entry: post-merge follow-up commit appending output to retro.
F-19 Manual sanity check on a real Go module deferred Same merge-prerequisite as F-18. M2-F-18-pattern follow-up.
F-20 README updated with usage + exit code table done Flag reference (7 rows) + Exit codes table (6 rows); grep -c 'exit code' returns 1.
F-21 go doc internal/cli renders cleanly done Package overview + role + io-edge note + full exit-code contract; all exported names DC-01/DC-02 compliant.
F-22 Closing report names guides + IDs done Substrate section enumerates 7 guides; cites EH-01/04/07/08/15/16/18, CC-08/09/11/13, AP-04/06/10/11/12/13/15, API-42, TE-01..15/43, DC-01/02.

Total rows: 22. Done in PR: 20. Deferred to post-merge follow-up: 2 (F-18, F-19). No-op: 0. Iteration count: 1.

Notable findings

  • runPipeline is structurally four operations (5 lines counting the error-handling line). Verification of M4 retro's "trio composes without adapter code" claim at the integration layer.
  • Five-seam pattern across the codebase is now textbook. pkg/golist:runGoList, pkg/changeset:getCwd, internal/cli:{runGitDiff, runGoListWrapper, signalContext}. Established convention; worth a CONTRIBUTING.md mention in M6 release-prep.
  • OUT-1 (M3's lint-cache fix) caught a gofmt issue immediately (struct-literal trailing-comment alignment in cli_test.go). Fifth milestone-use; battle-tested.
  • CC self-check protocol now validated across 5 milestones (M3, refactor, M4, M5; M2 informally). Zero softpedals across the run; discipline settled.
  • mapError's cancellation-first ordering is the right call (CDC observation): a SIGINT received mid-go-list maps to exit 5 (cancelled), not exit 3 (go list failed). Doc comment on the function makes the precedence explicit.

CDC review

CDC pass already run pre-merge (folded into the retro at commit d83d505). Closure recommendation: M5 is mergeable.

Checklist

  • make check passes locally (race tests + lint cold-cache + 99.7% overall coverage + per-package gate at 100% across 5 packages)
  • New/changed exported symbols have godoc comments (Run, GitDiffError, ErrGitDiffFailed all DC-01/DC-02 compliant)
  • Tests added/updated for behavior changes (Layer 1 in-process at 100% coverage; Layer 2 binary smoke for F-15/F-16/F-17)
  • No public-API breakage (internal/cli is a new package; cmd/cascade/main.go shrinks but doesn't change observable behaviour beyond what M5 specs)
  • If this PR closes a milestone ledger, each row's planned evidence text matches the criterion text (CC self-check + CDC independent re-read both clean)

Breaking change?

No for cascade's CLI users — cascade --version output unchanged from M1; new flags (--base, --head, --tags, --changed-files, --root, --help) are additive. No for library consumers — this PR adds new pipeline wiring under internal/cli; the existing public packages (pkg/golist, pkg/depgraph, pkg/changeset) are untouched.

🤖 Generated with Claude Code

oubiwann and others added 7 commits May 7, 2026 12:13
Wires M2 (pkg/golist) + M3 (pkg/depgraph) + M4 (pkg/changeset) into
a usable CLI binary with a documented exit-code contract, signal-
driven cancellation, and three function-variable seams that keep the
io edges (git diff, go list, signal.NotifyContext) in-process testable.

Surface (internal/cli, not exported beyond the cascade module):
- Run(args []string, stdin io.Reader, stdout, stderr io.Writer) int —
  the testable entry point; cmd/cascade/main is now a one-liner that
  delegates here.
- *GitDiffError + ErrGitDiffFailed — typed-error pair mirroring
  pkg/golist's *ExitError + ErrGoListFailed shape.

Pipeline (literally four lines in runPipeline):
  pkgs, err := runGoListWrapper(ctx, cfg.tags, []string{"./..."}, ...)
  g := depgraph.Build(pkgs)
  seeds := changeset.Resolve(changedFiles, pkgs, ...)
  return g.RevDepClosure(seeds), nil

This is the structural verification of M4 retro's "trio composes
without adapter code" claim — runPipeline is the entire adapter.

Three internal seams (function-variable pattern, third milestone-use
of M2's runGoList template, fifth occurrence in the codebase):
- runGitDiff = defaultRunGitDiff (the spec'd seam for the new io edge)
- runGoListWrapper = golist.Run (plan addition: golist's own seam is
  unexported within pkg/golist and not reachable from internal/cli's
  tests; this wrapper makes pipeline-integration tests in seam_test.go
  drive synthetic []golist.Package without real go list)
- signalContext = signal.NotifyContext (plan addition: stdlib signal
  context can't be cancelled in-test without sending real OS signals;
  this seam lets TestRun_ContextCancellation inject a pre-cancelled
  context to exercise exit 5 deterministically)

CLI flag set + exit code contract:
- --tags, --base, --head, --changed-files, --root, --version, --help
- 0=success, 1=flag/input error, 2=git diff failed, 3=go list failed,
  4=internal logic error (should never occur), 5=cancelled/interrupted
- helpText is an inline const; "// keep in sync with README" comment
  marks the pragmatic single-source-of-truth (Q2 lean, accepted).
- --help routes to stdout (Q4 GNU lean); -h shorthand triggers
  flag.ErrHelp + stdlib's auto-Usage to stderr.
- Q5 lean accepted: new exit code 5 for cancellation/interrupt.

Tests: 100% statement coverage on internal/cli (all five cascade
packages now at 100%). Layer 1 unit tests in internal/cli/{cli_test.go,
seam_test.go} drive every branch via the three seams. Layer 2 binary
smoke tests in cmd/cascade/main_test.go (TestCascadeBinaryVersion,
TestCascadeBinaryHelp, TestCascadeBinaryEndToEnd) build the binary
with -ldflags injection and exercise --version, --help, and the full
pipeline against pkg/golist/testdata/sample-module. F-18 (go install
@<sha>) and F-19 (manual sanity check on real Go module) deferred to
post-merge follow-ups (verifiable only against a merged commit).

scripts/coverage-check.sh: PACKAGES gains internal/cli at index 4 with
threshold 100. Comment block updated.

README.md CLI usage section: full flag reference (7 rows) + exit code
table (6 rows: 0/1/2/3/4/5) + a closing line on CI-workflow exit-code
discrimination. F-20 verify (grep -c 'exit code') returns 1.

Spec/plan: docs/dev/0011-implementation-plan-m5-cli-main-wiring.md

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
20 of 22 ledger rows close in the M5 PR; F-18 (go install @<sha>) and
F-19 (manual sanity check on real Go module) are deferred to post-
merge follow-ups since both verify against a merged commit on main.
Both deferrals carry documented re-entry conditions per
LEDGER_DISCIPLINE.md.

Notable findings carried into the retro:
- Function-variable seam pattern is now used 5 times across the
  codebase (pkg/golist runGoList, pkg/changeset getCwd, internal/cli
  runGitDiff + runGoListWrapper + signalContext). Settled discipline.
- M4 retro's "trio composes without adapter code" claim is
  structurally verified at the integration layer: runPipeline is
  literally four lines.
- Plan added two seams beyond the spec (runGoListWrapper,
  signalContext) for testability reasons that the spec didn't
  account for. Documented as plan-additions, not scope creep.
- 100% coverage on second try; no speculative-branch tests.
- CC self-check protocol now validated across 5 milestones (M3,
  refactor, M4, M5; plus M2 informally). Discipline settled.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
CDC pass run against m5/cli-main-wiring head c45291e + retro 9792d5f.
Replaces the "Pending..." placeholder in §"CDC review notes" with the
full independent-verification report:

- Direct re-reads on F-1, F-2 (Run signature), F-3 (GitDiffError +
  ErrGitDiffFailed at exact line numbers), F-4 (main.go one-liner),
  F-13 (PACKAGES array), F-14 (imports), F-20 (README tables), F-22
  (substrate enumeration).
- Toolchain rows (F-5..F-12, F-15..F-17, F-21) accepted on CI green
  + local-run evidence in the row walk.
- Two deferrals (F-18, F-19) classified as structurally valid, not
  softpedals: proxy.golang.org doesn't index unmerged refs; both
  rows have explicit re-entry conditions per LEDGER_DISCIPLINE.md.
- Two new seams beyond spec (runGoListWrapper, signalContext) ruled
  well-justified: structural necessity for in-process testability.
- mapError's cancellation-first ordering called out as the right
  call (a SIGINT mid-go-list should map to exit 5, not exit 3).

CDC findings: zero softpedals, zero silent drops, row count
22-declared / 22-accounted (20 closed, 2 deferred-with-condition).
Closure recommendation: M5 is mergeable.

Three engineering observations carried forward:
- errFlagOrInput as a category-error wrapper is reusable.
- GNU-convention --help routing + shared helpText const + "keep in
  sync with README" comment is the right pragmatic single-source
  pattern.
- Five-seam pattern is now textbook; worth a CONTRIBUTING.md mention
  in M6 release-prep.

Forward-looking note (not blocking M5): seam-pattern documentation
in CONTRIBUTING.md as a small standalone fix in the M6 window.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@oubiwann oubiwann merged commit 78cc7ae into main May 7, 2026
3 checks passed
oubiwann added a commit that referenced this pull request May 7, 2026
Closes the two formally-deferred ledger rows from M5 PR #10:

F-18 — `go install github.com/geomyidia/cascade/cmd/cascade@78cc7ae`
       via proxy.golang.org succeeded; resolves to pseudo-version
       v0.0.0-20260507180335-78cc7ae2dac9. Installed binary's
       --version + --help output captured (3.8MB darwin/arm64).
       Pseudo-version-extraction logic from M1.5 threads the commit
       SHA + build date through ReadBuildInfo into --version output.

F-19 — Cascade-against-cascade real-codebase sanity check (M2 F-18
       lineage; the gta-target codebase from M2 is private, this is
       the public-PR-safe substitute). Six cases:
       - golist root change → 5 packages affected (sorted)
       - changeset mid → 3 packages
       - cmd/cascade leaf → 1 package
       - internal/cli mid → 2 packages
       - empty stdin → empty output, exit 0
       - real --base/--head git diff → 134ms wall-clock end-to-end
       Every observed affected-set matches the hand-derived prediction.

Updated closure: M5 ledger reaches 22/22 done. Zero deferred at
final close; zero no-ops; zero open. The two-deferral pattern
(merge-prerequisite verification rows with explicit re-entry
conditions) worked exactly as LEDGER_DISCIPLINE.md documents.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant