From d17a11edbe648ff920edb10ce4f2f1243f0e1663 Mon Sep 17 00:00:00 2001 From: Rick Liu Date: Thu, 16 Jul 2026 13:53:31 +0100 Subject: [PATCH 1/8] refactor(generate): generalize source abstraction Rename PRInfo to ChangeSet, DiffProvider to ChangeSource, and ParsePRRef to ParseSourceRef. Pure rename with no behavior change, preparing for commit and snapshot sources alongside PR/MR generation. Co-Authored-By: Claude Fable 5 --- pkg/generate/generate.go | 6 +- pkg/generate/generate_test.go | 90 ++++++++++++++-------------- pkg/generate/provider.go | 20 +++---- pkg/generate/provider_github.go | 14 ++--- pkg/generate/provider_gitlab.go | 14 ++--- pkg/generate/provider_gitlab_test.go | 2 +- pkg/generate/types.go | 4 +- 7 files changed, 75 insertions(+), 75 deletions(-) diff --git a/pkg/generate/generate.go b/pkg/generate/generate.go index 37e28a0..23288ed 100644 --- a/pkg/generate/generate.go +++ b/pkg/generate/generate.go @@ -33,7 +33,7 @@ type Options struct { // Run generates a loom module from a PR/MR. func Run(ctx context.Context, opts Options, logger *slog.Logger) error { // 1. Detect provider and parse reference. - provider, diffProvider, err := ParsePRRef(opts.Ref, logger) + provider, diffProvider, err := ParseSourceRef(opts.Ref, logger) if err != nil { return err } @@ -42,7 +42,7 @@ func Run(ctx context.Context, opts Options, logger *slog.Logger) error { // 2. Fetch PR/MR diff. logger.Info("fetching PR/MR data", "ref", opts.Ref, "provider", provider) - prInfo, err := diffProvider.FetchDiff(ctx, opts.Ref, token, logger) + prInfo, err := diffProvider.Fetch(ctx, opts.Ref, token, logger) if err != nil { return fmt.Errorf("fetching diff: %w", err) } @@ -81,7 +81,7 @@ type generatedModule struct { patchFiles map[string][]byte // relative path (under __functions/patches/) -> content } -func buildModule(pr *PRInfo, name string, params map[string]string, logger *slog.Logger) *generatedModule { +func buildModule(pr *ChangeSet, name string, params map[string]string, logger *slog.Logger) *generatedModule { mod := &generatedModule{ loomFile: config.LoomFile{ APIVersion: "loom.rickliujh.github.io/v1beta1", diff --git a/pkg/generate/generate_test.go b/pkg/generate/generate_test.go index c6eb182..0168f3f 100644 --- a/pkg/generate/generate_test.go +++ b/pkg/generate/generate_test.go @@ -13,7 +13,7 @@ import ( ) func TestBuildModule_AddedFiles(t *testing.T) { - pr := &PRInfo{ + pr := &ChangeSet{ Title: "Onboard payments", BaseBranch: "main", HeadBranch: "feat/payments", @@ -62,7 +62,7 @@ func TestBuildModule_AddedFiles(t *testing.T) { } func TestBuildModule_ModifiedYAML(t *testing.T) { - pr := &PRInfo{ + pr := &ChangeSet{ Title: "Update config", Provider: "github", Files: []FileChange{ @@ -96,7 +96,7 @@ func TestBuildModule_ModifiedYAML(t *testing.T) { } func TestBuildModule_DeletedFiles(t *testing.T) { - pr := &PRInfo{ + pr := &ChangeSet{ Title: "Cleanup", Provider: "github", Files: []FileChange{ @@ -117,7 +117,7 @@ func TestBuildModule_DeletedFiles(t *testing.T) { } func TestBuildModule_DefaultGitOps(t *testing.T) { - pr := &PRInfo{ + pr := &ChangeSet{ Title: "Add feature", BaseBranch: "main", HeadBranch: "feat/add-feature", @@ -158,7 +158,7 @@ func TestBuildModule_DefaultGitOps(t *testing.T) { } func TestBuildModule_DefaultGitOps_ParameterizesTarget(t *testing.T) { - pr := &PRInfo{ + pr := &ChangeSet{ Title: "Onboard payments", BaseBranch: "main", HeadBranch: "feat/onboard-payments", @@ -254,7 +254,7 @@ func TestEmitModule(t *testing.T) { // --- G3: Module name derivation priority --- func TestBuildModule_NamePriority_ExplicitName(t *testing.T) { - pr := &PRInfo{ + pr := &ChangeSet{ Title: "Some PR Title", Provider: "github", Files: []FileChange{{Type: ChangeAdded, Path: "f.yaml", NewContent: []byte("x: 1")}}, @@ -266,7 +266,7 @@ func TestBuildModule_NamePriority_ExplicitName(t *testing.T) { } func TestBuildModule_NamePriority_SlugifiedTitle(t *testing.T) { - pr := &PRInfo{ + pr := &ChangeSet{ Title: "Add Payment Service", Provider: "github", Files: []FileChange{{Type: ChangeAdded, Path: "f.yaml", NewContent: []byte("x: 1")}}, @@ -320,9 +320,9 @@ func TestSlugify_TruncatesAt60(t *testing.T) { } } -// --- PD1/PD2: Provider detection via ParsePRRef --- +// --- PD1/PD2: Provider detection via ParseSourceRef --- -func TestParsePRRef_GitHub(t *testing.T) { +func TestParseSourceRef_GitHub(t *testing.T) { tests := []struct { ref string provider string @@ -335,21 +335,21 @@ func TestParsePRRef_GitHub(t *testing.T) { {"github:owner/repo#1", "github"}, } for _, tt := range tests { - provider, dp, err := ParsePRRef(tt.ref, testLogger()) + provider, dp, err := ParseSourceRef(tt.ref, testLogger()) if err != nil { - t.Errorf("ParsePRRef(%q): %v", tt.ref, err) + t.Errorf("ParseSourceRef(%q): %v", tt.ref, err) continue } if provider != tt.provider { - t.Errorf("ParsePRRef(%q) provider = %q, want %q", tt.ref, provider, tt.provider) + t.Errorf("ParseSourceRef(%q) provider = %q, want %q", tt.ref, provider, tt.provider) } if dp == nil { - t.Errorf("ParsePRRef(%q) returned nil DiffProvider", tt.ref) + t.Errorf("ParseSourceRef(%q) returned nil ChangeSource", tt.ref) } } } -func TestParsePRRef_RejectsBadURLs(t *testing.T) { +func TestParseSourceRef_RejectsBadURLs(t *testing.T) { // These contain "/pull/" or "/-/merge_requests/" as substrings but // are not valid PR/MR URLs — the strict regex should reject them. badRefs := []string{ @@ -358,14 +358,14 @@ func TestParsePRRef_RejectsBadURLs(t *testing.T) { "this-has-/pull/42-in-the-middle", } for _, ref := range badRefs { - _, _, err := ParsePRRef(ref, testLogger()) + _, _, err := ParseSourceRef(ref, testLogger()) if err == nil { - t.Errorf("ParsePRRef(%q) should have been rejected", ref) + t.Errorf("ParseSourceRef(%q) should have been rejected", ref) } } } -func TestParsePRRef_GitLab(t *testing.T) { +func TestParseSourceRef_GitLab(t *testing.T) { tests := []struct { ref string provider string @@ -378,21 +378,21 @@ func TestParsePRRef_GitLab(t *testing.T) { {"gitlab:group/repo!5", "gitlab"}, } for _, tt := range tests { - provider, dp, err := ParsePRRef(tt.ref, testLogger()) + provider, dp, err := ParseSourceRef(tt.ref, testLogger()) if err != nil { - t.Errorf("ParsePRRef(%q): %v", tt.ref, err) + t.Errorf("ParseSourceRef(%q): %v", tt.ref, err) continue } if provider != tt.provider { - t.Errorf("ParsePRRef(%q) provider = %q, want %q", tt.ref, provider, tt.provider) + t.Errorf("ParseSourceRef(%q) provider = %q, want %q", tt.ref, provider, tt.provider) } if dp == nil { - t.Errorf("ParsePRRef(%q) returned nil DiffProvider", tt.ref) + t.Errorf("ParseSourceRef(%q) returned nil ChangeSource", tt.ref) } } } -func TestParsePRRef_UnknownProvider(t *testing.T) { +func TestParseSourceRef_UnknownProvider(t *testing.T) { refs := []string{ "https://example.com/repo/changes/1", "bitbucket:owner/repo#1", @@ -400,9 +400,9 @@ func TestParsePRRef_UnknownProvider(t *testing.T) { "", } for _, ref := range refs { - _, _, err := ParsePRRef(ref, testLogger()) + _, _, err := ParseSourceRef(ref, testLogger()) if err == nil { - t.Errorf("ParsePRRef(%q) expected error for unknown provider", ref) + t.Errorf("ParseSourceRef(%q) expected error for unknown provider", ref) } } } @@ -522,7 +522,7 @@ func TestParseGitLabMRRef_Errors(t *testing.T) { // --- FC1: Added files — single newFiles operation from root --- func TestBuildModule_AddedFiles_SingleNewFilesOp(t *testing.T) { - pr := &PRInfo{ + pr := &ChangeSet{ Title: "Add multiple dirs", Provider: "github", Files: []FileChange{ @@ -551,7 +551,7 @@ func TestBuildModule_AddedFiles_SingleNewFilesOp(t *testing.T) { } func TestBuildModule_AddedFiles_SourceIsDot(t *testing.T) { - pr := &PRInfo{ + pr := &ChangeSet{ Title: "Root files", Provider: "github", Files: []FileChange{ @@ -574,7 +574,7 @@ func TestBuildModule_AddedFiles_SourceIsDot(t *testing.T) { } func TestBuildModule_AddedFiles_NilContentSkipped(t *testing.T) { - pr := &PRInfo{ + pr := &ChangeSet{ Title: "Nil content", Provider: "github", Files: []FileChange{ @@ -596,7 +596,7 @@ func TestBuildModule_AddedFiles_NilContentSkipped(t *testing.T) { // --- FC2: Modified files — YAML with SMP only; others skipped with warning --- func TestBuildModule_ModifiedNonYAML_Skipped(t *testing.T) { - pr := &PRInfo{ + pr := &ChangeSet{ Title: "Update JSON", Provider: "github", Files: []FileChange{ @@ -620,7 +620,7 @@ func TestBuildModule_ModifiedNonYAML_Skipped(t *testing.T) { } func TestBuildModule_ModifiedYAML_NoOldContent_Skipped(t *testing.T) { - pr := &PRInfo{ + pr := &ChangeSet{ Title: "Update YAML without old", Provider: "github", Files: []FileChange{ @@ -644,7 +644,7 @@ func TestBuildModule_ModifiedYAML_NoOldContent_Skipped(t *testing.T) { } func TestBuildModule_ModifiedYAML_InvalidYAML_Skipped(t *testing.T) { - pr := &PRInfo{ + pr := &ChangeSet{ Title: "Bad YAML", Provider: "github", Files: []FileChange{ @@ -668,7 +668,7 @@ func TestBuildModule_ModifiedYAML_InvalidYAML_Skipped(t *testing.T) { } func TestBuildModule_ModifiedYAML_YmlExtension(t *testing.T) { - pr := &PRInfo{ + pr := &ChangeSet{ Title: "Update yml", Provider: "github", Files: []FileChange{ @@ -692,7 +692,7 @@ func TestBuildModule_ModifiedYAML_YmlExtension(t *testing.T) { } func TestBuildModule_ModifiedYAML_NilNewContent_Skipped(t *testing.T) { - pr := &PRInfo{ + pr := &ChangeSet{ Title: "No new content", Provider: "github", Files: []FileChange{ @@ -717,7 +717,7 @@ func TestBuildModule_ModifiedYAML_NilNewContent_Skipped(t *testing.T) { // --- FC3: Deleted files → shell rm --- func TestBuildModule_DeletedFiles_Parameterized(t *testing.T) { - pr := &PRInfo{ + pr := &ChangeSet{ Title: "Remove payments config", Provider: "github", Files: []FileChange{ @@ -744,7 +744,7 @@ func TestBuildModule_DeletedFiles_Parameterized(t *testing.T) { // --- FC4: Renamed files → mv, then SMP patch if content also changed --- func TestBuildModule_RenamedFiles_MvOnly_NoOldContent(t *testing.T) { - pr := &PRInfo{ + pr := &ChangeSet{ Title: "Rename file", Provider: "github", Files: []FileChange{ @@ -788,7 +788,7 @@ func TestBuildModule_RenamedFiles_MvOnly_NoOldContent(t *testing.T) { } func TestBuildModule_RenamedFiles_MvAndPatch(t *testing.T) { - pr := &PRInfo{ + pr := &ChangeSet{ Title: "Rename and modify", Provider: "github", Files: []FileChange{ @@ -844,7 +844,7 @@ func TestBuildModule_RenamedFiles_MvAndPatch(t *testing.T) { } func TestBuildModule_RenamedFiles_NonYAML_ContentChanged(t *testing.T) { - pr := &PRInfo{ + pr := &ChangeSet{ Title: "Rename non-YAML", Provider: "github", Files: []FileChange{ @@ -873,7 +873,7 @@ func TestBuildModule_RenamedFiles_NonYAML_ContentChanged(t *testing.T) { func TestBuildModule_RenamedFiles_YAML_IdenticalContent(t *testing.T) { content := []byte("key: value\nother: data") - pr := &PRInfo{ + pr := &ChangeSet{ Title: "Pure rename YAML", Provider: "github", Files: []FileChange{ @@ -901,7 +901,7 @@ func TestBuildModule_RenamedFiles_YAML_IdenticalContent(t *testing.T) { } func TestBuildModule_RenamedFiles_Parameterized(t *testing.T) { - pr := &PRInfo{ + pr := &ChangeSet{ Title: "Rename payments", Provider: "github", Files: []FileChange{ @@ -927,7 +927,7 @@ func TestBuildModule_RenamedFiles_Parameterized(t *testing.T) { } func TestBuildModule_RenamedFiles_PureRename_NoNewContent(t *testing.T) { - pr := &PRInfo{ + pr := &ChangeSet{ Title: "Pure rename", Provider: "github", Files: []FileChange{ @@ -955,7 +955,7 @@ func TestBuildModule_RenamedFiles_PureRename_NoNewContent(t *testing.T) { // --- PM5/GO3/GO4: GitOps metadata parameterization --- func TestBuildModule_GitOps_ParameterizesCommitAndPR(t *testing.T) { - pr := &PRInfo{ + pr := &ChangeSet{ Title: "onboard payments service", Body: "Adding payments to the platform", BaseBranch: "main", @@ -1034,7 +1034,7 @@ func TestToSSHURL(t *testing.T) { // --- E2: loom.yaml structure conformance --- func TestBuildModule_LoomFileStructure(t *testing.T) { - pr := &PRInfo{ + pr := &ChangeSet{ Title: "test", Provider: "github", BaseBranch: "main", @@ -1058,7 +1058,7 @@ func TestBuildModule_LoomFileStructure(t *testing.T) { // --- PM6: All params declared as required --- func TestBuildModule_ParamDefs_AllRequired(t *testing.T) { - pr := &PRInfo{ + pr := &ChangeSet{ Title: "test", Provider: "github", Files: []FileChange{{Type: ChangeAdded, Path: "f.yaml", NewContent: []byte("x: 1")}}, @@ -1082,7 +1082,7 @@ func TestBuildModule_ParamDefs_AllRequired(t *testing.T) { // --- E3: Operation ordering --- func TestBuildModule_OperationOrdering(t *testing.T) { - pr := &PRInfo{ + pr := &ChangeSet{ Title: "Full change set", Provider: "github", BaseBranch: "main", @@ -1152,7 +1152,7 @@ func TestSanitizeFilename(t *testing.T) { // --- SMP5: Patch file path in operation --- func TestBuildModule_PatchFilePath(t *testing.T) { - pr := &PRInfo{ + pr := &ChangeSet{ Title: "Patch test", Provider: "github", Files: []FileChange{ @@ -1343,7 +1343,7 @@ func TestEmitModule_LoomYAMLRoundTrip(t *testing.T) { // --- Mixed change types in a single PR --- func TestBuildModule_MixedChangeTypes(t *testing.T) { - pr := &PRInfo{ + pr := &ChangeSet{ Title: "Mixed changes", Provider: "github", Files: []FileChange{ diff --git a/pkg/generate/provider.go b/pkg/generate/provider.go index aaca12b..67f60f4 100644 --- a/pkg/generate/provider.go +++ b/pkg/generate/provider.go @@ -10,9 +10,9 @@ import ( "strings" ) -// DiffProvider fetches file changes from a PR/MR. -type DiffProvider interface { - FetchDiff(ctx context.Context, ref string, token string, logger *slog.Logger) (*PRInfo, error) +// ChangeSource fetches file changes from a PR/MR. +type ChangeSource interface { + Fetch(ctx context.Context, ref string, token string, logger *slog.Logger) (*ChangeSet, error) } // Regex patterns for URL-based provider detection. @@ -24,8 +24,8 @@ var ( gitlabMRURLPattern = regexp.MustCompile(`^https?://[^/]+/.+/-/merge_requests/\d+/?$`) ) -// ParsePRRef parses a PR/MR URL and returns the provider type and a -// DiffProvider suitable for fetching PR/MR data. +// ParseSourceRef parses a PR/MR URL and returns the provider type and a +// ChangeSource suitable for fetching PR/MR data. // // Short-form references (unambiguous, preferred for self-hosted instances): // - github:owner/repo#123 @@ -37,27 +37,27 @@ var ( // // For self-hosted instances where URL patterns may be ambiguous, use the // short-form prefix (github: or gitlab:) to explicitly specify the provider. -func ParsePRRef(ref string, logger *slog.Logger) (provider string, _ DiffProvider, _ error) { +func ParseSourceRef(ref string, logger *slog.Logger) (provider string, _ ChangeSource, _ error) { logger.Debug("parsing PR/MR reference", "ref", ref) // Short-form references — unambiguous, checked first. if strings.HasPrefix(ref, "github:") { logger.Debug("detected GitHub short-form reference") - return "github", &GitHubDiffProvider{}, nil + return "github", &GitHubSource{}, nil } if strings.HasPrefix(ref, "gitlab:") { logger.Debug("detected GitLab short-form reference") - return "gitlab", &GitLabDiffProvider{}, nil + return "gitlab", &GitLabSource{}, nil } // URL-based detection with strict pattern matching. if githubPRURLPattern.MatchString(ref) { logger.Debug("detected GitHub PR URL") - return "github", &GitHubDiffProvider{}, nil + return "github", &GitHubSource{}, nil } if gitlabMRURLPattern.MatchString(ref) { logger.Debug("detected GitLab MR URL") - return "gitlab", &GitLabDiffProvider{}, nil + return "gitlab", &GitLabSource{}, nil } return "", nil, fmt.Errorf("cannot detect provider from reference %q; use a full URL or prefix with github: or gitlab:", ref) diff --git a/pkg/generate/provider_github.go b/pkg/generate/provider_github.go index e78c4a9..3ea0c39 100644 --- a/pkg/generate/provider_github.go +++ b/pkg/generate/provider_github.go @@ -16,10 +16,10 @@ import ( "golang.org/x/oauth2" ) -// GitHubDiffProvider fetches PR diffs from GitHub. -type GitHubDiffProvider struct{} +// GitHubSource fetches PR diffs from GitHub. +type GitHubSource struct{} -func (p *GitHubDiffProvider) FetchDiff(ctx context.Context, ref, token string, logger *slog.Logger) (*PRInfo, error) { +func (p *GitHubSource) Fetch(ctx context.Context, ref, token string, logger *slog.Logger) (*ChangeSet, error) { owner, repo, number, err := parseGitHubPRRef(ref) if err != nil { return nil, err @@ -59,7 +59,7 @@ func githubRepoURL(ref, owner, repo string) string { return fmt.Sprintf("https://github.com/%s/%s.git", owner, repo) } -func (p *GitHubDiffProvider) fetchAPI(ctx context.Context, owner, repo string, number int, repoURL, token string, logger *slog.Logger) (*PRInfo, error) { +func (p *GitHubSource) fetchAPI(ctx context.Context, owner, repo string, number int, repoURL, token string, logger *slog.Logger) (*ChangeSet, error) { logger.Debug("creating GitHub API client", "owner", owner, "repo", repo) ts := oauth2.StaticTokenSource(&oauth2.Token{AccessToken: token}) tc := oauth2.NewClient(ctx, ts) @@ -72,7 +72,7 @@ func (p *GitHubDiffProvider) fetchAPI(ctx context.Context, owner, repo string, n } logger.Debug("PR metadata fetched", "title", pr.GetTitle(), "head", pr.GetHead().GetRef(), "base", pr.GetBase().GetRef()) - info := &PRInfo{ + info := &ChangeSet{ Title: pr.GetTitle(), Body: pr.GetBody(), BaseBranch: pr.GetBase().GetRef(), @@ -185,7 +185,7 @@ func fetchFileAtRef(ctx context.Context, client *github.Client, owner, repo, pat return []byte(content), nil } -func (p *GitHubDiffProvider) fetchCLI(ctx context.Context, owner, repo string, number int, repoURL string, logger *slog.Logger) (*PRInfo, error) { +func (p *GitHubSource) fetchCLI(ctx context.Context, owner, repo string, number int, repoURL string, logger *slog.Logger) (*ChangeSet, error) { nwo := owner + "/" + repo // Fetch PR metadata including commit SHAs for resilience against branch deletion. @@ -220,7 +220,7 @@ func (p *GitHubDiffProvider) fetchCLI(ctx context.Context, owner, repo string, n } logger.Debug("using refs for file content (gh CLI)", "headRef", headRef, "baseRef", baseRef) - info := &PRInfo{ + info := &ChangeSet{ Title: prMeta.Title, Body: prMeta.Body, BaseBranch: prMeta.BaseRefName, diff --git a/pkg/generate/provider_gitlab.go b/pkg/generate/provider_gitlab.go index fd4007d..0929dc0 100644 --- a/pkg/generate/provider_gitlab.go +++ b/pkg/generate/provider_gitlab.go @@ -13,10 +13,10 @@ import ( gogitlab "gitlab.com/gitlab-org/api/client-go" ) -// GitLabDiffProvider fetches MR diffs from GitLab. -type GitLabDiffProvider struct{} +// GitLabSource fetches MR diffs from GitLab. +type GitLabSource struct{} -func (p *GitLabDiffProvider) FetchDiff(ctx context.Context, ref, token string, logger *slog.Logger) (*PRInfo, error) { +func (p *GitLabSource) Fetch(ctx context.Context, ref, token string, logger *slog.Logger) (*ChangeSet, error) { baseURL, projectPath, mrIID, err := parseGitLabMRRef(ref) if err != nil { return nil, err @@ -47,7 +47,7 @@ func (p *GitLabDiffProvider) FetchDiff(ctx context.Context, ref, token string, l return nil, fmt.Errorf("GitLab API failed: %w; glab CLI is not available", apiErr) } -func (p *GitLabDiffProvider) fetchAPI(ctx context.Context, baseURL, projectPath string, mrIID int64, token string, logger *slog.Logger) (*PRInfo, error) { +func (p *GitLabSource) fetchAPI(ctx context.Context, baseURL, projectPath string, mrIID int64, token string, logger *slog.Logger) (*ChangeSet, error) { logger.Debug("creating GitLab API client", "baseURL", baseURL, "project", projectPath) client, err := gogitlab.NewClient(token, gogitlab.WithBaseURL(baseURL)) if err != nil { @@ -80,7 +80,7 @@ func (p *GitLabDiffProvider) fetchAPI(ctx context.Context, baseURL, projectPath } logger.Debug("using refs for file content", "headRef", headRef, "baseRef", baseRef) - info := &PRInfo{ + info := &ChangeSet{ Title: mr.Title, Body: mr.Description, BaseBranch: mr.TargetBranch, @@ -156,7 +156,7 @@ func (p *GitLabDiffProvider) fetchAPI(ctx context.Context, baseURL, projectPath return info, nil } -func (p *GitLabDiffProvider) fetchCLI(ctx context.Context, baseURL, projectPath string, mrIID int64, logger *slog.Logger) (*PRInfo, error) { +func (p *GitLabSource) fetchCLI(ctx context.Context, baseURL, projectPath string, mrIID int64, logger *slog.Logger) (*ChangeSet, error) { encodedProject := url.PathEscape(projectPath) // Extract hostname from baseURL for --hostname flag. @@ -213,7 +213,7 @@ func (p *GitLabDiffProvider) fetchCLI(ctx context.Context, baseURL, projectPath } logger.Debug("using refs for file content (CLI)", "headRef", headRef, "baseRef", baseRef) - info := &PRInfo{ + info := &ChangeSet{ Title: mrMeta.Title, Body: mrMeta.Description, BaseBranch: mrMeta.TargetBranch, diff --git a/pkg/generate/provider_gitlab_test.go b/pkg/generate/provider_gitlab_test.go index c354d99..ec5a1b9 100644 --- a/pkg/generate/provider_gitlab_test.go +++ b/pkg/generate/provider_gitlab_test.go @@ -2,7 +2,7 @@ package generate import "testing" -// Regression: the glab CLI fallback used to build PRInfo without RepoURL, +// Regression: the glab CLI fallback used to build ChangeSet without RepoURL, // leaving the generated module's target.url empty. Both fetch paths must // derive the same clone URL. func TestGitlabRepoURL(t *testing.T) { diff --git a/pkg/generate/types.go b/pkg/generate/types.go index a3753f9..8294bb3 100644 --- a/pkg/generate/types.go +++ b/pkg/generate/types.go @@ -34,8 +34,8 @@ type FileChange struct { OldContent []byte // full file content before the change (for modified) } -// PRInfo holds metadata about a PR/MR. -type PRInfo struct { +// ChangeSet holds metadata about a PR/MR. +type ChangeSet struct { Title string Body string BaseBranch string From 14902552a00f626a9dc0710571f81bf744c9735f Mon Sep 17 00:00:00 2001 From: Rick Liu Date: Thu, 16 Jul 2026 13:57:23 +0100 Subject: [PATCH 2/8] feat(generate): compose multiple sources into one net changeset Accept multiple source references on the CLI and squash their changesets in the order given (spec rules CS1-CS5): per-file old content from the first source, new content from the last, with rename-chain tracking, continuity warnings, repo consistency checks, and metadata from the last source. Modules generated from sources without PR metadata degrade gracefully: missing head branch synthesizes loom/, missing provider omits the pr operation, missing repo URL omits spec.target. Adds specs/generate-sources.md describing the multi-source design (PRs, commit ranges, local snapshots). Co-Authored-By: Claude Fable 5 --- cmd/generate.go | 54 +++++-- pkg/generate/compose.go | 209 +++++++++++++++++++++++++ pkg/generate/compose_test.go | 274 +++++++++++++++++++++++++++++++++ pkg/generate/generate.go | 141 ++++++++++++----- pkg/generate/generate_test.go | 115 ++++++++++++-- pkg/generate/provider.go | 120 ++++++++++++--- specs/generate-sources.md | 276 ++++++++++++++++++++++++++++++++++ 7 files changed, 1101 insertions(+), 88 deletions(-) create mode 100644 pkg/generate/compose.go create mode 100644 pkg/generate/compose_test.go create mode 100644 specs/generate-sources.md diff --git a/cmd/generate.go b/cmd/generate.go index f82f656..e1b3410 100644 --- a/cmd/generate.go +++ b/cmd/generate.go @@ -10,33 +10,52 @@ var ( genOutput string genName string genTokenEnv string + genInclude []string + genExclude []string + genBase string ) var generateCmd = &cobra.Command{ - Use: "generate ", - Short: "Generate a loom module from a GitHub PR or GitLab MR", - Long: `Generate a reusable loom module by analyzing the file changes in an existing -pull request or merge request. Added files become templates, modified YAML files -become strategic merge patches, and concrete values you specify with -p are -replaced with template parameters. + Use: "generate [...]", + Short: "Generate a loom module from PRs/MRs, commits, or local files", + Long: `Generate a reusable loom module by analyzing file changes from one or more +sources. Added files become templates, modified YAML files become strategic +merge patches, and concrete values you specify with -p are replaced with +template parameters. -By default, files are generated into the current directory. Use -o to specify -a different output directory. +When multiple references are given they are composed in order (oldest first) +into a single net changeset — useful when the desired state was reached over +several PRs or follow-up commits. Supported references: - https://github.com/owner/repo/pull/123 - https://gitlab.com/group/repo/-/merge_requests/123 - github:owner/repo#123 - gitlab:group/repo!123`, - Args: cobra.ExactArgs(1), + PR/MR (provider API): + https://github.com/owner/repo/pull/123 + https://gitlab.com/group/repo/-/merge_requests/123 + github:owner/repo#123 + gitlab:group/repo!123 + Commit or commit range (git-native, any host): + github:owner/repo@abc1234 + github:owner/repo@abc1234...def5678 + git@bitbucket.org:owner/repo.git@abc1234...def5678 + https://github.com/owner/repo/commit/abc1234 + ./checkout@abc1234...def5678 + Current state of local files (with --include, optionally --base): + ./checkout /abs/path file:relative/path + +By default, files are generated into the current directory. Use -o to specify +a different output directory.`, + Args: cobra.MinimumNArgs(1), RunE: runGenerate, } func init() { generateCmd.Flags().StringArrayVarP(&genParams, "param", "p", nil, "Value to parameterize: key=value (can be repeated)") generateCmd.Flags().StringVarP(&genOutput, "output", "o", "", "Output directory (default: current directory)") - generateCmd.Flags().StringVarP(&genName, "name", "n", "", "Module name (default: derived from PR title)") - generateCmd.Flags().StringVar(&genTokenEnv, "token-env", "", "Env var holding the API token (default: GITHUB_TOKEN or GITLAB_TOKEN)") + generateCmd.Flags().StringVarP(&genName, "name", "n", "", "Module name (default: derived from PR title or commit subject)") + generateCmd.Flags().StringVar(&genTokenEnv, "token-env", "", "Env var holding the API token for PR/MR sources (default: GITHUB_TOKEN or GITLAB_TOKEN)") + generateCmd.Flags().StringArrayVar(&genInclude, "include", nil, "Glob of files to capture from a local path source (can be repeated; ** matches directories)") + generateCmd.Flags().StringArrayVar(&genExclude, "exclude", nil, "Glob of files to skip from a local path source (can be repeated)") + generateCmd.Flags().StringVar(&genBase, "base", "", "Git ref to diff a local path source against (default: capture files as-is)") rootCmd.AddCommand(generateCmd) } @@ -49,11 +68,14 @@ func runGenerate(cmd *cobra.Command, args []string) error { } opts := generate.Options{ - Ref: args[0], + Refs: args, Params: paramMap, OutputDir: genOutput, ModuleName: genName, TokenEnv: genTokenEnv, + Include: genInclude, + Exclude: genExclude, + Base: genBase, } return generate.Run(cmd.Context(), opts, logger) diff --git a/pkg/generate/compose.go b/pkg/generate/compose.go new file mode 100644 index 0000000..372327b --- /dev/null +++ b/pkg/generate/compose.go @@ -0,0 +1,209 @@ +package generate + +import ( + "bytes" + "fmt" + "log/slog" + "strings" +) + +// netChange accumulates the net effect of all changesets on one file. +type netChange struct { + origPath string // path in the state before the first change + currentPath string // path after the latest change + // existedBefore is true when the file was present before the first + // changeset that touched it (i.e. the first change was not an add). + existedBefore bool + oldContent []byte // content before the first change (nil if !existedBefore) + exists bool // present after the latest change + newContent []byte // content after the latest change (nil if !exists) +} + +// Compose squashes changesets, applied in order, into a single net ChangeSet. +// Per file, the old content comes from the first changeset that touched it +// and the new content from the last, so the result transforms the original +// base state directly into the final desired state. +func Compose(sets []*ChangeSet, logger *slog.Logger) (*ChangeSet, error) { + if len(sets) == 0 { + return nil, fmt.Errorf("no change sets to compose") + } + + // CS3: all sources with a repo URL must reference the same repository. + repoURL := "" + for _, cs := range sets { + if cs.RepoURL == "" { + continue + } + if repoURL == "" { + repoURL = cs.RepoURL + continue + } + if normalizeRepoURL(cs.RepoURL) != normalizeRepoURL(repoURL) { + return nil, fmt.Errorf("sources reference different repositories: %s vs %s", repoURL, cs.RepoURL) + } + } + + if len(sets) == 1 { + return sets[0], nil + } + + // CS1: fold changesets left to right, keyed by current path. + byPath := make(map[string]*netChange) + var order []*netChange // first-touch order, for deterministic output + + for i, cs := range sets { + for _, f := range cs.Files { + key := f.Path + if f.Type == ChangeRenamed { + key = f.OldPath + } + e, found := byPath[key] + if !found { + e = newNetChange(f) + byPath[e.currentPath] = e + order = append(order, e) + continue + } + + // CS2: continuity check — this source's view of the file before + // its change should match the state accumulated so far. + if e.exists && f.Type != ChangeAdded && f.OldContent != nil && e.newContent != nil && + !bytes.Equal(e.newContent, f.OldContent) { + logger.Warn("file changed between sources; composing anyway (manual review recommended)", + "file", key, "source", i+1) + } + + switch f.Type { + case ChangeAdded: + if e.exists { + logger.Warn("file re-added while already present; using later content", "file", key, "source", i+1) + } + e.exists = true + e.newContent = f.NewContent + case ChangeModified: + e.exists = true + e.newContent = f.NewContent + case ChangeDeleted: + e.exists = false + e.newContent = nil + case ChangeRenamed: + delete(byPath, key) + e.currentPath = f.Path + byPath[e.currentPath] = e + e.exists = true + if f.NewContent != nil { + e.newContent = f.NewContent + } + } + } + } + + var files []FileChange + for _, e := range order { + if f, ok := e.net(); ok { + files = append(files, f) + } + } + + // CS5: everything cancelled out. + if len(files) == 0 { + return nil, fmt.Errorf("no net file changes after composing sources") + } + + // CS4: metadata from the last source that has it — the later sources + // represent the desired state. + merged := &ChangeSet{Files: files} + for _, cs := range sets { + if cs.Title != "" { + merged.Title = cs.Title + } + if cs.Body != "" { + merged.Body = cs.Body + } + if cs.BaseBranch != "" { + merged.BaseBranch = cs.BaseBranch + } + if cs.HeadBranch != "" { + merged.HeadBranch = cs.HeadBranch + } + if cs.RepoURL != "" { + merged.RepoURL = cs.RepoURL + } + if cs.Provider != "" { + merged.Provider = cs.Provider + } + } + + return merged, nil +} + +func newNetChange(f FileChange) *netChange { + switch f.Type { + case ChangeAdded: + return &netChange{ + origPath: f.Path, currentPath: f.Path, + existedBefore: false, exists: true, newContent: f.NewContent, + } + case ChangeDeleted: + return &netChange{ + origPath: f.Path, currentPath: f.Path, + existedBefore: true, oldContent: f.OldContent, exists: false, + } + case ChangeRenamed: + return &netChange{ + origPath: f.OldPath, currentPath: f.Path, + existedBefore: true, oldContent: f.OldContent, exists: true, newContent: f.NewContent, + } + default: // ChangeModified + return &netChange{ + origPath: f.Path, currentPath: f.Path, + existedBefore: true, oldContent: f.OldContent, exists: true, newContent: f.NewContent, + } + } +} + +// net reduces the accumulated state to a single FileChange relative to the +// original base state. ok is false when the changes cancelled out. +func (e *netChange) net() (FileChange, bool) { + switch { + case !e.existedBefore && !e.exists: + // Added then deleted — nothing to do. + return FileChange{}, false + case !e.existedBefore: + return FileChange{Type: ChangeAdded, Path: e.currentPath, NewContent: e.newContent}, true + case !e.exists: + // Deleted — target the original path; the generated module operates + // on a repo at base state. + return FileChange{Type: ChangeDeleted, Path: e.origPath, OldContent: e.oldContent}, true + case e.origPath != e.currentPath: + return FileChange{ + Type: ChangeRenamed, Path: e.currentPath, OldPath: e.origPath, + OldContent: e.oldContent, NewContent: e.newContent, + }, true + case bytes.Equal(e.oldContent, e.newContent): + // Modified back to the original content — nothing to do. + return FileChange{}, false + default: + return FileChange{ + Type: ChangeModified, Path: e.currentPath, + OldContent: e.oldContent, NewContent: e.newContent, + }, true + } +} + +// normalizeRepoURL reduces a repository URL to host/path form so that HTTPS, +// SSH, and scp-like spellings of the same repository compare equal. +func normalizeRepoURL(u string) string { + s := strings.ToLower(strings.TrimSpace(u)) + s = strings.TrimSuffix(s, "/") + s = strings.TrimSuffix(s, ".git") + for _, p := range []string{"https://", "http://", "ssh://", "git://"} { + s = strings.TrimPrefix(s, p) + } + // user@host:path or user@host/path + if i := strings.Index(s, "@"); i >= 0 { + s = s[i+1:] + } + s = strings.Replace(s, ":", "/", 1) + return strings.TrimSuffix(s, "/") +} diff --git a/pkg/generate/compose_test.go b/pkg/generate/compose_test.go new file mode 100644 index 0000000..32e5e09 --- /dev/null +++ b/pkg/generate/compose_test.go @@ -0,0 +1,274 @@ +package generate + +import ( + "bytes" + "log/slog" + "strings" + "testing" +) + +func mustCompose(t *testing.T, sets []*ChangeSet) *ChangeSet { + t.Helper() + merged, err := Compose(sets, testLogger()) + if err != nil { + t.Fatalf("Compose: %v", err) + } + return merged +} + +func findFile(t *testing.T, cs *ChangeSet, path string) FileChange { + t.Helper() + for _, f := range cs.Files { + if f.Path == path { + return f + } + } + t.Fatalf("file %q not found in composed changeset: %+v", path, cs.Files) + return FileChange{} +} + +// CS1: old content from the first source, new content from the last. +func TestCompose_CS1_ModifyThenModify(t *testing.T) { + merged := mustCompose(t, []*ChangeSet{ + {Files: []FileChange{{Type: ChangeModified, Path: "f.yaml", OldContent: []byte("v: 1"), NewContent: []byte("v: 2")}}}, + {Files: []FileChange{{Type: ChangeModified, Path: "f.yaml", OldContent: []byte("v: 2"), NewContent: []byte("v: 3")}}}, + }) + + if len(merged.Files) != 1 { + t.Fatalf("expected 1 net change, got %d", len(merged.Files)) + } + f := merged.Files[0] + if f.Type != ChangeModified { + t.Errorf("expected modified, got %v", f.Type) + } + if string(f.OldContent) != "v: 1" || string(f.NewContent) != "v: 3" { + t.Errorf("expected v:1 → v:3, got %q → %q", f.OldContent, f.NewContent) + } +} + +// CS2: a mid-chain source whose old content disagrees with the accumulated +// state signals interleaved out-of-band commits — warn, don't block. +func TestCompose_CS2_WarnsOnDiscontinuity(t *testing.T) { + var buf bytes.Buffer + logger := slog.New(slog.NewTextHandler(&buf, &slog.HandlerOptions{Level: slog.LevelWarn})) + + merged, err := Compose([]*ChangeSet{ + {Files: []FileChange{{Type: ChangeModified, Path: "f.yaml", OldContent: []byte("v: 1"), NewContent: []byte("v: 2")}}}, + // Old content v:9 does not match the accumulated v:2. + {Files: []FileChange{{Type: ChangeModified, Path: "f.yaml", OldContent: []byte("v: 9"), NewContent: []byte("v: 3")}}}, + }, logger) + if err != nil { + t.Fatal(err) + } + + if !strings.Contains(buf.String(), "changed between sources") { + t.Errorf("expected continuity warning, got log: %s", buf.String()) + } + // Still composes: last-wins on new content. + f := merged.Files[0] + if string(f.OldContent) != "v: 1" || string(f.NewContent) != "v: 3" { + t.Errorf("expected v:1 → v:3, got %q → %q", f.OldContent, f.NewContent) + } +} + +func TestCompose_AddThenModify(t *testing.T) { + merged := mustCompose(t, []*ChangeSet{ + {Files: []FileChange{{Type: ChangeAdded, Path: "f.yaml", NewContent: []byte("v: 1")}}}, + {Files: []FileChange{{Type: ChangeModified, Path: "f.yaml", OldContent: []byte("v: 1"), NewContent: []byte("v: 2")}}}, + }) + + f := findFile(t, merged, "f.yaml") + if f.Type != ChangeAdded { + t.Errorf("add+modify should net to added, got %v", f.Type) + } + if string(f.NewContent) != "v: 2" { + t.Errorf("expected latest content, got %q", f.NewContent) + } +} + +func TestCompose_AddThenDelete_Drops(t *testing.T) { + merged := mustCompose(t, []*ChangeSet{ + {Files: []FileChange{ + {Type: ChangeAdded, Path: "temp.yaml", NewContent: []byte("t: 1")}, + {Type: ChangeAdded, Path: "keep.yaml", NewContent: []byte("k: 1")}, + }}, + {Files: []FileChange{{Type: ChangeDeleted, Path: "temp.yaml", OldContent: []byte("t: 1")}}}, + }) + + if len(merged.Files) != 1 { + t.Fatalf("expected only keep.yaml to survive, got %+v", merged.Files) + } + if merged.Files[0].Path != "keep.yaml" { + t.Errorf("expected keep.yaml, got %q", merged.Files[0].Path) + } +} + +// Delete-then-re-add nets to modified so YAML stays SMP-able. +func TestCompose_DeleteThenAdd_NetsModified(t *testing.T) { + merged := mustCompose(t, []*ChangeSet{ + {Files: []FileChange{{Type: ChangeDeleted, Path: "f.yaml", OldContent: []byte("v: 1")}}}, + {Files: []FileChange{{Type: ChangeAdded, Path: "f.yaml", NewContent: []byte("v: 2")}}}, + }) + + f := findFile(t, merged, "f.yaml") + if f.Type != ChangeModified { + t.Errorf("delete+add should net to modified, got %v", f.Type) + } + if string(f.OldContent) != "v: 1" || string(f.NewContent) != "v: 2" { + t.Errorf("expected v:1 → v:2, got %q → %q", f.OldContent, f.NewContent) + } +} + +func TestCompose_ModifyThenDelete(t *testing.T) { + merged := mustCompose(t, []*ChangeSet{ + {Files: []FileChange{{Type: ChangeModified, Path: "f.yaml", OldContent: []byte("v: 1"), NewContent: []byte("v: 2")}}}, + {Files: []FileChange{{Type: ChangeDeleted, Path: "f.yaml", OldContent: []byte("v: 2")}}}, + }) + + f := findFile(t, merged, "f.yaml") + if f.Type != ChangeDeleted { + t.Errorf("modify+delete should net to deleted, got %v", f.Type) + } +} + +// Rename chains collapse: a→b, edit b, b→c becomes a single a→c rename. +func TestCompose_RenameChain(t *testing.T) { + merged := mustCompose(t, []*ChangeSet{ + {Files: []FileChange{{Type: ChangeRenamed, Path: "b.yaml", OldPath: "a.yaml", OldContent: []byte("v: 1"), NewContent: []byte("v: 1")}}}, + {Files: []FileChange{{Type: ChangeModified, Path: "b.yaml", OldContent: []byte("v: 1"), NewContent: []byte("v: 2")}}}, + {Files: []FileChange{{Type: ChangeRenamed, Path: "c.yaml", OldPath: "b.yaml", OldContent: []byte("v: 2"), NewContent: []byte("v: 2")}}}, + }) + + if len(merged.Files) != 1 { + t.Fatalf("expected 1 net change, got %+v", merged.Files) + } + f := merged.Files[0] + if f.Type != ChangeRenamed { + t.Fatalf("expected renamed, got %v", f.Type) + } + if f.OldPath != "a.yaml" || f.Path != "c.yaml" { + t.Errorf("expected a.yaml → c.yaml, got %q → %q", f.OldPath, f.Path) + } + if string(f.OldContent) != "v: 1" || string(f.NewContent) != "v: 2" { + t.Errorf("expected v:1 → v:2, got %q → %q", f.OldContent, f.NewContent) + } +} + +// Rename back to the original name with original content cancels out. +func TestCompose_RenameRoundTrip_Drops(t *testing.T) { + content := []byte("v: 1") + _, err := Compose([]*ChangeSet{ + {Files: []FileChange{{Type: ChangeRenamed, Path: "b.yaml", OldPath: "a.yaml", OldContent: content, NewContent: content}}}, + {Files: []FileChange{{Type: ChangeRenamed, Path: "a.yaml", OldPath: "b.yaml", OldContent: content, NewContent: content}}}, + }, testLogger()) + + // CS5: nothing left. + if err == nil || !strings.Contains(err.Error(), "no net file changes") { + t.Errorf("expected no-net-changes error, got %v", err) + } +} + +// CS5: a PR and its revert cancel out. +func TestCompose_CS5_RevertCancelsOut(t *testing.T) { + _, err := Compose([]*ChangeSet{ + {Files: []FileChange{{Type: ChangeModified, Path: "f.yaml", OldContent: []byte("v: 1"), NewContent: []byte("v: 2")}}}, + {Files: []FileChange{{Type: ChangeModified, Path: "f.yaml", OldContent: []byte("v: 2"), NewContent: []byte("v: 1")}}}, + }, testLogger()) + + if err == nil || !strings.Contains(err.Error(), "no net file changes") { + t.Errorf("expected no-net-changes error, got %v", err) + } +} + +// CS3: sources must reference the same repository; spelling may differ. +func TestCompose_CS3_RepoMismatch(t *testing.T) { + files := []FileChange{{Type: ChangeAdded, Path: "f.yaml", NewContent: []byte("x: 1")}} + _, err := Compose([]*ChangeSet{ + {RepoURL: "https://github.com/org/repo.git", Files: files}, + {RepoURL: "git@github.com:org/other.git", Files: files}, + }, testLogger()) + + if err == nil || !strings.Contains(err.Error(), "different repositories") { + t.Errorf("expected repo mismatch error, got %v", err) + } +} + +func TestCompose_RepoSpellingVariantsMatch(t *testing.T) { + merged := mustCompose(t, []*ChangeSet{ + {RepoURL: "https://github.com/org/repo.git", Files: []FileChange{{Type: ChangeAdded, Path: "a.yaml", NewContent: []byte("a: 1")}}}, + {RepoURL: "git@github.com:org/repo.git", Files: []FileChange{{Type: ChangeAdded, Path: "b.yaml", NewContent: []byte("b: 1")}}}, + }) + if len(merged.Files) != 2 { + t.Errorf("expected 2 files, got %d", len(merged.Files)) + } +} + +// CS4: metadata comes from the last source that has it. +func TestCompose_CS4_MetadataFromLastSource(t *testing.T) { + merged := mustCompose(t, []*ChangeSet{ + { + Title: "first PR", Body: "first body", BaseBranch: "main", HeadBranch: "feat/one", + RepoURL: "https://github.com/o/r.git", Provider: "github", + Files: []FileChange{{Type: ChangeAdded, Path: "a.yaml", NewContent: []byte("a: 1")}}, + }, + { + Title: "follow-up fix", // no HeadBranch (e.g. a commit source) + Files: []FileChange{{Type: ChangeAdded, Path: "b.yaml", NewContent: []byte("b: 1")}}, + }, + }) + + if merged.Title != "follow-up fix" { + t.Errorf("expected last title, got %q", merged.Title) + } + if merged.Body != "first body" { + t.Errorf("expected first body to survive (last source has none), got %q", merged.Body) + } + if merged.HeadBranch != "feat/one" { + t.Errorf("expected head branch from last PR source, got %q", merged.HeadBranch) + } + if merged.RepoURL != "https://github.com/o/r.git" || merged.Provider != "github" { + t.Errorf("expected repo metadata preserved, got %q / %q", merged.RepoURL, merged.Provider) + } +} + +func TestCompose_SinglePassthrough(t *testing.T) { + cs := &ChangeSet{ + Title: "only", + Files: []FileChange{{Type: ChangeAdded, Path: "f.yaml", NewContent: []byte("x: 1")}}, + } + merged := mustCompose(t, []*ChangeSet{cs}) + if merged != cs { + t.Error("single changeset should pass through unchanged") + } +} + +func TestCompose_UnrelatedFilesInFirstTouchOrder(t *testing.T) { + merged := mustCompose(t, []*ChangeSet{ + {Files: []FileChange{{Type: ChangeAdded, Path: "one.yaml", NewContent: []byte("1")}}}, + {Files: []FileChange{{Type: ChangeAdded, Path: "two.yaml", NewContent: []byte("2")}}}, + }) + if len(merged.Files) != 2 { + t.Fatalf("expected 2 files, got %d", len(merged.Files)) + } + if merged.Files[0].Path != "one.yaml" || merged.Files[1].Path != "two.yaml" { + t.Errorf("expected first-touch order, got %+v", merged.Files) + } +} + +func TestNormalizeRepoURL(t *testing.T) { + tests := []struct { + in string + want string + }{ + {"https://github.com/org/repo.git", "github.com/org/repo"}, + {"git@github.com:org/repo.git", "github.com/org/repo"}, + {"ssh://git@github.com/org/repo.git", "github.com/org/repo"}, + {"HTTPS://GitHub.com/Org/Repo", "github.com/org/repo"}, + {"https://gitlab.example.com/group/sub/repo.git", "gitlab.example.com/group/sub/repo"}, + } + for _, tt := range tests { + if got := normalizeRepoURL(tt.in); got != tt.want { + t.Errorf("normalizeRepoURL(%q) = %q, want %q", tt.in, got, tt.want) + } + } +} diff --git a/pkg/generate/generate.go b/pkg/generate/generate.go index 23288ed..71c8efd 100644 --- a/pkg/generate/generate.go +++ b/pkg/generate/generate.go @@ -16,8 +16,9 @@ import ( // Options configures module generation. type Options struct { - // Ref is the PR/MR URL or short reference. - Ref string + // Refs are the source references (PR/MR, commit, or local path), applied + // in the order given (oldest first). + Refs []string // Params maps param names to concrete values to parameterize. Params map[string]string // OutputDir is the directory to write the generated module. @@ -26,51 +27,91 @@ type Options struct { ModuleName string // TokenEnv is the name of the environment variable that holds the // GitHub personal access token or GitLab private token used to - // authenticate API requests when fetching PR/MR data. + // authenticate API requests when fetching PR/MR data. Commit and + // snapshot sources are git-native and do not use it. TokenEnv string + // Include, Exclude, and Base configure snapshot (local path) sources. + Include []string + Exclude []string + Base string } -// Run generates a loom module from a PR/MR. +// Run generates a loom module from one or more sources. func Run(ctx context.Context, opts Options, logger *slog.Logger) error { - // 1. Detect provider and parse reference. - provider, diffProvider, err := ParseSourceRef(opts.Ref, logger) - if err != nil { - return err + if len(opts.Refs) == 0 { + return fmt.Errorf("at least one source reference is required") } - token := tokenFromEnv(opts.TokenEnv, provider, logger) - - // 2. Fetch PR/MR diff. - logger.Info("fetching PR/MR data", "ref", opts.Ref, "provider", provider) - prInfo, err := diffProvider.Fetch(ctx, opts.Ref, token, logger) - if err != nil { - return fmt.Errorf("fetching diff: %w", err) + // 1. Parse all references. + snap := SnapshotOptions{Include: opts.Include, Exclude: opts.Exclude, Base: opts.Base} + sources := make([]*Source, 0, len(opts.Refs)) + hasSnapshot := false + allSnapshots := true + for _, ref := range opts.Refs { + src, err := ParseSourceRef(ref, snap, logger) + if err != nil { + return err + } + if src.Kind == KindSnapshot { + hasSnapshot = true + } else { + allSnapshots = false + } + sources = append(sources, src) + } + if !hasSnapshot && (len(opts.Include) > 0 || len(opts.Exclude) > 0 || opts.Base != "") { + return fmt.Errorf("--include/--exclude/--base require a local path source") } - if len(prInfo.Files) == 0 { - return fmt.Errorf("PR/MR has no file changes") + // 2. Fetch each source's changes, in the order given. + sets := make([]*ChangeSet, 0, len(sources)) + for i, src := range sources { + token := "" + if src.Kind == KindPR { + token = tokenFromEnv(opts.TokenEnv, src.Provider, logger) + } + logger.Info("fetching source", "ref", opts.Refs[i], "kind", src.Kind.String()) + cs, err := src.ChangeSource.Fetch(ctx, opts.Refs[i], token, logger) + if err != nil { + return fmt.Errorf("fetching diff: %w", err) + } + if len(cs.Files) == 0 { + return fmt.Errorf("source %q has no file changes", opts.Refs[i]) + } + logger.Info("found file changes", "ref", opts.Refs[i], "count", len(cs.Files)) + sets = append(sets, cs) } - logger.Info("found file changes", "count", len(prInfo.Files)) + // 3. Compose into a single net changeset. + merged, err := Compose(sets, logger) + if err != nil { + return err + } + if len(sets) > 1 { + logger.Info("composed sources", "sources", len(sets), "netChanges", len(merged.Files)) + } - // 3. Derive module name. + // 4. Derive module name. moduleName := opts.ModuleName if moduleName == "" { - moduleName = slugify(prInfo.Title) + moduleName = slugify(merged.Title) } if moduleName == "" { + if allSnapshots { + return fmt.Errorf("--name is required when generating from local files") + } moduleName = "generated-module" } - // 4. Classify files and build module structure. + // 5. Classify files and build module structure. outputDir := opts.OutputDir if outputDir == "" { outputDir = "." } - module := buildModule(prInfo, moduleName, opts.Params, logger) + module := buildModule(merged, moduleName, opts.Params, logger) - // 5. Emit the module. + // 6. Emit the module. return emitModule(outputDir, module, logger) } @@ -225,30 +266,53 @@ func buildModule(pr *ChangeSet, name string, params map[string]string, logger *s }) } - // Add target and gitops operations. - mod.loomFile.Spec.Target = &config.TargetSpec{ - URL: toSSHURL(pr.RepoURL), - Branch: pr.BaseBranch, - FeatureBranch: Parameterize(pr.HeadBranch, params), + // Add target and gitops operations. Sources without PR metadata (commit + // ranges, local snapshots) get synthesized fallbacks; sources without a + // recognizable remote degrade by omitting the target and/or pr operation. + title := pr.Title + if title == "" { + title = "apply " + name + } + featureBranch := pr.HeadBranch + if featureBranch == "" { + featureBranch = "loom/" + name } + + if pr.RepoURL != "" { + mod.loomFile.Spec.Target = &config.TargetSpec{ + URL: toSSHURL(pr.RepoURL), + Branch: pr.BaseBranch, + FeatureBranch: Parameterize(featureBranch, params), + } + } else { + logger.Warn("no repository URL detected; spec.target omitted (fill in manually before running)") + } + mod.loomFile.Spec.Operations = append(mod.loomFile.Spec.Operations, config.Operation{ Name: "commit", CommitPush: &config.CommitPush{ - Message: Parameterize(pr.Title, params), - }, - }, - config.Operation{ - Name: "open-pr", - PR: &config.PR{ - Provider: pr.Provider, - Title: Parameterize(pr.Title, params), - Body: Parameterize(pr.Body, params), - BaseBranch: pr.BaseBranch, + Message: Parameterize(title, params), }, }, ) + if pr.Provider != "" { + mod.loomFile.Spec.Operations = append(mod.loomFile.Spec.Operations, + config.Operation{ + Name: "open-pr", + PR: &config.PR{ + Provider: pr.Provider, + Title: Parameterize(title, params), + Body: Parameterize(pr.Body, params), + BaseBranch: pr.BaseBranch, + }, + }, + ) + } else { + logger.Warn("provider unknown; pr operation omitted (add manually if needed)") + } + return mod } @@ -348,4 +412,3 @@ func toSSHURL(repoURL string) string { path := strings.TrimPrefix(parsed.Path, "/") return fmt.Sprintf("git@%s:%s", host, path) } - diff --git a/pkg/generate/generate_test.go b/pkg/generate/generate_test.go index 0168f3f..62a024a 100644 --- a/pkg/generate/generate_test.go +++ b/pkg/generate/generate_test.go @@ -182,7 +182,6 @@ func TestBuildModule_DefaultGitOps_ParameterizesTarget(t *testing.T) { } } - func TestEmitModule(t *testing.T) { tmpDir := t.TempDir() outputDir := filepath.Join(tmpDir, "test-module") @@ -335,15 +334,18 @@ func TestParseSourceRef_GitHub(t *testing.T) { {"github:owner/repo#1", "github"}, } for _, tt := range tests { - provider, dp, err := ParseSourceRef(tt.ref, testLogger()) + src, err := ParseSourceRef(tt.ref, SnapshotOptions{}, testLogger()) if err != nil { t.Errorf("ParseSourceRef(%q): %v", tt.ref, err) continue } - if provider != tt.provider { - t.Errorf("ParseSourceRef(%q) provider = %q, want %q", tt.ref, provider, tt.provider) + if src.Provider != tt.provider { + t.Errorf("ParseSourceRef(%q) provider = %q, want %q", tt.ref, src.Provider, tt.provider) + } + if src.Kind != KindPR { + t.Errorf("ParseSourceRef(%q) kind = %v, want pr", tt.ref, src.Kind) } - if dp == nil { + if src.ChangeSource == nil { t.Errorf("ParseSourceRef(%q) returned nil ChangeSource", tt.ref) } } @@ -358,7 +360,7 @@ func TestParseSourceRef_RejectsBadURLs(t *testing.T) { "this-has-/pull/42-in-the-middle", } for _, ref := range badRefs { - _, _, err := ParseSourceRef(ref, testLogger()) + _, err := ParseSourceRef(ref, SnapshotOptions{}, testLogger()) if err == nil { t.Errorf("ParseSourceRef(%q) should have been rejected", ref) } @@ -378,15 +380,18 @@ func TestParseSourceRef_GitLab(t *testing.T) { {"gitlab:group/repo!5", "gitlab"}, } for _, tt := range tests { - provider, dp, err := ParseSourceRef(tt.ref, testLogger()) + src, err := ParseSourceRef(tt.ref, SnapshotOptions{}, testLogger()) if err != nil { t.Errorf("ParseSourceRef(%q): %v", tt.ref, err) continue } - if provider != tt.provider { - t.Errorf("ParseSourceRef(%q) provider = %q, want %q", tt.ref, provider, tt.provider) + if src.Provider != tt.provider { + t.Errorf("ParseSourceRef(%q) provider = %q, want %q", tt.ref, src.Provider, tt.provider) + } + if src.Kind != KindPR { + t.Errorf("ParseSourceRef(%q) kind = %v, want pr", tt.ref, src.Kind) } - if dp == nil { + if src.ChangeSource == nil { t.Errorf("ParseSourceRef(%q) returned nil ChangeSource", tt.ref) } } @@ -400,7 +405,7 @@ func TestParseSourceRef_UnknownProvider(t *testing.T) { "", } for _, ref := range refs { - _, _, err := ParseSourceRef(ref, testLogger()) + _, err := ParseSourceRef(ref, SnapshotOptions{}, testLogger()) if err == nil { t.Errorf("ParseSourceRef(%q) expected error for unknown provider", ref) } @@ -464,8 +469,8 @@ func TestParseGitHubPRRef(t *testing.T) { func TestParseGitHubPRRef_Errors(t *testing.T) { badRefs := []string{ "https://github.com/myorg/myrepo/pull/notanumber", - "github:myorg/myrepo", // missing #number - "github:myorg#42", // missing /repo + "github:myorg/myrepo", // missing #number + "github:myorg#42", // missing /repo "not-a-valid-reference", // no /pull/ or github: prefix } for _, ref := range badRefs { @@ -508,8 +513,8 @@ func TestParseGitLabMRRef(t *testing.T) { func TestParseGitLabMRRef_Errors(t *testing.T) { badRefs := []string{ - "gitlab:group/repo", // missing !number - "gitlab:group/repo!abc", // non-numeric + "gitlab:group/repo", // missing !number + "gitlab:group/repo!abc", // non-numeric } for _, ref := range badRefs { _, _, _, err := parseGitLabMRRef(ref) @@ -1398,6 +1403,86 @@ func TestChangeType_String(t *testing.T) { } } +// --- Degraded gitops paths (commit / snapshot sources) --- + +func TestBuildModule_NoRepoURL_OmitsTarget(t *testing.T) { + cs := &ChangeSet{ + Title: "local change", + Provider: "github", + Files: []FileChange{{Type: ChangeAdded, Path: "f.yaml", NewContent: []byte("x: 1")}}, + } + mod := buildModule(cs, "test", nil, testLogger()) + + if mod.loomFile.Spec.Target != nil { + t.Error("expected target to be omitted when repo URL is unknown") + } + // commitPush is still emitted. + hasCommit := false + for _, op := range mod.loomFile.Spec.Operations { + if op.CommitPush != nil { + hasCommit = true + } + } + if !hasCommit { + t.Error("expected commitPush operation") + } +} + +func TestBuildModule_NoProvider_OmitsPROp(t *testing.T) { + cs := &ChangeSet{ + Title: "bitbucket change", + RepoURL: "git@bitbucket.org:o/r.git", + BaseBranch: "main", + Files: []FileChange{{Type: ChangeAdded, Path: "f.yaml", NewContent: []byte("x: 1")}}, + } + mod := buildModule(cs, "test", nil, testLogger()) + + for _, op := range mod.loomFile.Spec.Operations { + if op.PR != nil { + t.Error("expected pr operation to be omitted when provider is unknown") + } + } + if mod.loomFile.Spec.Target == nil { + t.Fatal("expected target (repo URL is known)") + } +} + +func TestBuildModule_SynthesizesFeatureBranchAndTitle(t *testing.T) { + cs := &ChangeSet{ + // No Title, no HeadBranch — e.g. a snapshot source. + RepoURL: "https://github.com/o/r.git", + BaseBranch: "main", + Provider: "github", + Files: []FileChange{{Type: ChangeAdded, Path: "f.yaml", NewContent: []byte("x: 1")}}, + } + mod := buildModule(cs, "my-module", nil, testLogger()) + + if mod.loomFile.Spec.Target.FeatureBranch != "loom/my-module" { + t.Errorf("featureBranch = %q, want loom/my-module", mod.loomFile.Spec.Target.FeatureBranch) + } + for _, op := range mod.loomFile.Spec.Operations { + if op.CommitPush != nil && op.CommitPush.Message != "apply my-module" { + t.Errorf("commit message = %q, want 'apply my-module'", op.CommitPush.Message) + } + if op.PR != nil && op.PR.Title != "apply my-module" { + t.Errorf("pr title = %q, want 'apply my-module'", op.PR.Title) + } + } +} + +// --- End-to-end: multiple sources composed through Run --- + +func TestRun_SnapshotFlagsRequireSnapshotSource(t *testing.T) { + opts := Options{ + Refs: []string{"github:o/r#1"}, + Include: []string{"a/**"}, + } + err := Run(t.Context(), opts, testLogger()) + if err == nil || !strings.Contains(err.Error(), "require a local path source") { + t.Errorf("expected flag-validation error, got %v", err) + } +} + func testLogger() *slog.Logger { return slog.New(slog.NewTextHandler(os.Stderr, &slog.HandlerOptions{Level: slog.LevelWarn})) } diff --git a/pkg/generate/provider.go b/pkg/generate/provider.go index 67f60f4..5298d1f 100644 --- a/pkg/generate/provider.go +++ b/pkg/generate/provider.go @@ -4,17 +4,62 @@ import ( "context" "fmt" "log/slog" + "net/url" "os" "os/exec" "regexp" "strings" ) -// ChangeSource fetches file changes from a PR/MR. +// ChangeSource fetches file changes from a source (PR/MR, commit range, or +// local snapshot). type ChangeSource interface { Fetch(ctx context.Context, ref string, token string, logger *slog.Logger) (*ChangeSet, error) } +// SourceKind classifies what kind of source a reference points at. +type SourceKind int + +const ( + // KindPR is a GitHub pull request or GitLab merge request. + KindPR SourceKind = iota + // KindCommit is a single commit or commit range, fetched via git. + KindCommit + // KindSnapshot is the current state of files in a local checkout. + KindSnapshot +) + +func (k SourceKind) String() string { + switch k { + case KindPR: + return "pr" + case KindCommit: + return "commit" + case KindSnapshot: + return "snapshot" + default: + return "unknown" + } +} + +// Source is a parsed source reference. +type Source struct { + Kind SourceKind + // Provider is "github" or "gitlab" when known, "" otherwise. Only PR + // sources require a provider; commit and snapshot sources are git-native + // and use it solely to populate the generated pr operation. + Provider string + ChangeSource ChangeSource +} + +// SnapshotOptions configures snapshot (local file) sources. The options are +// shared by all snapshot refs of a single generate invocation. +type SnapshotOptions struct { + Include []string + Exclude []string + Base string +} + // Regex patterns for URL-based provider detection. // These validate the full URL structure, not just a substring. var ( @@ -22,45 +67,84 @@ var ( githubPRURLPattern = regexp.MustCompile(`^https?://[^/]+/.+/pull/\d+/?$`) // Matches: https:////-/merge_requests/[/] gitlabMRURLPattern = regexp.MustCompile(`^https?://[^/]+/.+/-/merge_requests/\d+/?$`) + // Matches: https:///[/-]/commit/[/] (GitHub and GitLab). + commitURLPattern = regexp.MustCompile(`^(https?://[^/]+/.+?)(?:/-)?/commit/([0-9a-fA-F]{7,40})/?$`) + // Matches: https:///[/-]/compare/...[/] (GitHub and GitLab). + compareURLPattern = regexp.MustCompile(`^(https?://[^/]+/.+?)(?:/-)?/compare/([^/]+?)\.\.\.([^/]+?)/?$`) + + // A hex commit SHA (abbreviated or full). + shaPattern = regexp.MustCompile(`^[0-9a-fA-F]{7,40}$`) + // A git rev token (SHA or tag name) — no path separators or colons, so + // SSH URL remainders can never match. + revTokenPattern = regexp.MustCompile(`^[A-Za-z0-9][A-Za-z0-9._\-]*$`) ) -// ParseSourceRef parses a PR/MR URL and returns the provider type and a -// ChangeSource suitable for fetching PR/MR data. +// ParseSourceRef parses a source reference and returns the parsed Source. // -// Short-form references (unambiguous, preferred for self-hosted instances): -// - github:owner/repo#123 -// - gitlab:group/repo!123 +// PR/MR references (see also the generate spec): +// - github:owner/repo#123 / gitlab:group/repo!123 +// - https:///owner/repo/pull/123 +// - https:///group/repo/-/merge_requests/123 // -// URL-based detection (works for any host, including self-hosted): -// - https:///owner/repo/pull/123 → GitHub -// - https:///group/repo/-/merge_requests/123 → GitLab +// Commit references (git-native, any host): +// - @ or @..., where is a short-form +// repo (github:o/r), any git URL, or a local checkout path +// - commit / compare URLs (sugar, normalized to @) // -// For self-hosted instances where URL patterns may be ambiguous, use the -// short-form prefix (github: or gitlab:) to explicitly specify the provider. -func ParseSourceRef(ref string, logger *slog.Logger) (provider string, _ ChangeSource, _ error) { - logger.Debug("parsing PR/MR reference", "ref", ref) +// Snapshot references (current state of local files): +// - a local path: ./checkout, /abs/path, or file: +func ParseSourceRef(ref string, snap SnapshotOptions, logger *slog.Logger) (*Source, error) { + logger.Debug("parsing source reference", "ref", ref) // Short-form references — unambiguous, checked first. if strings.HasPrefix(ref, "github:") { logger.Debug("detected GitHub short-form reference") - return "github", &GitHubSource{}, nil + return &Source{Kind: KindPR, Provider: "github", ChangeSource: &GitHubSource{}}, nil } if strings.HasPrefix(ref, "gitlab:") { logger.Debug("detected GitLab short-form reference") - return "gitlab", &GitLabSource{}, nil + return &Source{Kind: KindPR, Provider: "gitlab", ChangeSource: &GitLabSource{}}, nil } // URL-based detection with strict pattern matching. if githubPRURLPattern.MatchString(ref) { logger.Debug("detected GitHub PR URL") - return "github", &GitHubSource{}, nil + return &Source{Kind: KindPR, Provider: "github", ChangeSource: &GitHubSource{}}, nil } if gitlabMRURLPattern.MatchString(ref) { logger.Debug("detected GitLab MR URL") - return "gitlab", &GitLabSource{}, nil + return &Source{Kind: KindPR, Provider: "gitlab", ChangeSource: &GitLabSource{}}, nil + } + + return nil, fmt.Errorf("cannot detect source kind from reference %q; use a PR/MR URL, a @ commit reference, a local path, or prefix with github: or gitlab:", ref) +} + +// inferProviderFromURL guesses the provider from the repository URL host. +// It is only used to populate the generated pr operation; an empty result +// degrades to omitting that operation. +func inferProviderFromURL(repoURL string) string { + host := hostOf(repoURL) + switch { + case strings.Contains(host, "github"): + return "github" + case strings.Contains(host, "gitlab"): + return "gitlab" + default: + return "" } +} - return "", nil, fmt.Errorf("cannot detect provider from reference %q; use a full URL or prefix with github: or gitlab:", ref) +func hostOf(repoURL string) string { + if parsed, err := url.Parse(repoURL); err == nil && parsed.Host != "" { + return strings.ToLower(parsed.Hostname()) + } + // scp-like syntax: user@host:path + if _, rest, found := strings.Cut(repoURL, "@"); found { + if host, _, ok := strings.Cut(rest, ":"); ok { + return strings.ToLower(host) + } + } + return "" } // tokenFromEnv returns the API token from the given env var, or falls back diff --git a/specs/generate-sources.md b/specs/generate-sources.md new file mode 100644 index 0000000..f310a52 --- /dev/null +++ b/specs/generate-sources.md @@ -0,0 +1,276 @@ +# Generate Sources — Design Proposal (Draft) + +Status: **draft / not implemented**. Extends [`specs/generate.md`](generate.md). + +## Problem + +`loom generate` accepts exactly one PR/MR. In practice the desired end state is often +reached through: + +1. **Several PRs** — the first attempt plus follow-up fixes. +2. **Specific commits** — a single commit or a commit range, not tied to any PR. +3. **Current state of files** — the repo already looks right; no clean PR/commit + history exists to point at. + +The module builder should be able to consume any of these, alone or combined. + +## Core insight + +Everything downstream of `FetchDiff` — classification, SMP computation, +parameterization, emission — only consumes `PRInfo` (metadata + `[]FileChange` +with old/new content). The design therefore: + +1. Generalizes the source abstraction: `DiffProvider` → `ChangeSource`, `PRInfo` → `ChangeSet`. +2. Adds two new source kinds (commits, local snapshot). +3. Adds one new step: **composition** of multiple `ChangeSet`s into a single net + `ChangeSet`, which then flows through the existing pipeline unchanged. + +``` +refs ──► [ChangeSource]* ──► [ChangeSet]* ──► Compose ──► ChangeSet ──► buildModule (unchanged) +``` + +## Source abstraction + +```go +// ChangeSet generalizes PRInfo. All fields except Files are optional; +// availability depends on the source kind. +type ChangeSet struct { + Title, Body string + BaseBranch, HeadBranch string + RepoURL string + Provider string // "github" | "gitlab" | "" (local) + Files []FileChange +} + +type ChangeSource interface { + Fetch(ctx context.Context, token string, logger *slog.Logger) (*ChangeSet, error) +} +``` + +`ParsePRRef` becomes `ParseSourceRef(ref) (provider string, ChangeSource, error)` +and recognizes the grammars below. + +## Reference grammars + +| Form | Kind | Example | +|------|------|---------| +| existing PR/MR forms | PR source | `github:owner/repo#123` | +| `@` | single commit | `git@bitbucket.org:o/r.git@abc1234` | +| `@...` | commit range | `github:o/r@abc1234...def5678` | +| commit URL (sugar) | single commit | `https://github.com/o/r/commit/abc1234` | +| compare URL (sugar) | commit range | `https://gitlab.com/g/r/-/compare/abc...def` | +| local path (`./…`, `/…`, or `file:` prefix) | snapshot | `./checkout-of-target-repo` | + +`` in commit refs is any of: +- a short-form repo (`github:o/r`, `gitlab:g/r`) — sugar that expands to the + canonical SSH URL for that host; +- **any git URL** (`https://…`, `git@host:…`, `ssh://…`) — this is what makes + the commit source platform-agnostic; +- a local path to an existing checkout (no clone needed). + +The rev part is split on the **last** `@` in the ref (SSH URLs contain an +earlier `@`) and must match `` or `...` where `` is a hex +SHA (7–40 chars) or a tag name. Commit/compare URLs from known hosts are +accepted as sugar and normalized to `@` — they are parsed, not +fetched via API. + +Detection order: `file:` prefix / path-like without `@rev` → snapshot; +trailing `@` → commit; short-form with `#`/`!` → PR (existing); then +PR/MR URL patterns. Malformed refs are rejected, never misclassified (same +philosophy as PD2). + +### 1. PR source (existing, unchanged) + +Current `GitHubDiffProvider` / `GitLabDiffProvider`, renamed to implement +`ChangeSource`. + +### 2. Commit source (git-native, platform-agnostic) + +The commit source uses **git only** — no provider API clients. This works +against any git host (GitHub, GitLab, Bitbucket, Gitea, bare SSH remotes) and +authenticates with the user's existing git credentials (SSH keys, credential +helpers) instead of API tokens. `--token-env` does not apply to commit sources. + +**Obtaining the objects:** + +1. If `` is a local path, open it directly (`git.Open`); fetch missing + SHAs from `origin` if needed. +2. Otherwise clone `--bare --filter=blob:none` into a temp directory (blob + contents are fetched lazily, so cost scales with changed files, not repo + size), then `git fetch origin `. Direct SHA fetch requires the + server to allow reachable-SHA-in-want (GitHub, GitLab, and Gitea all do); + if the server refuses, fall back to a full fetch with a warning. Servers + without partial-clone support fall back to a plain bare clone. +3. **Single `sha`** is treated as range `sha^...sha` (first parent for merge + commits — the diff is "what this merge brought in"); the parent is covered + by fetching with `--depth=2`. + +Builds on `pkg/git`'s existing go-git-with-CLI-fallback pattern. + +**Computing the changeset:** + +- File statuses: `git diff -M --name-status ` (rename detection + included — better than most provider compare APIs). +- Old/new content: `git show :` per changed file. + +**Metadata (all from git, no API):** + +| Field | Source | +|-------|--------| +| `Title` / `Body` | Subject / remaining body of the head commit (`git log -1`) | +| `BaseBranch` | Default branch via `git ls-remote --symref origin HEAD` | +| `HeadBranch` | Empty (synthesized later, see GO below) | +| `RepoURL` | The `` part of the ref (or the checkout's `origin`) | +| `Provider` | Inferred from the host, only for the generated `pr` op (empty → degraded path below) | + +### 3. Snapshot source (current state of files) + +Points at a **local checkout** of the target repo. Selection and baseline come +from flags (snapshot refs get no inline grammar for these): + +- `--include ` (repeatable, required for snapshot refs): files to capture, + relative to the checkout root. `--exclude ` optional. +- `--base ` (optional): baseline to diff against. + +Two modes: + +- **No `--base`** — every matched file becomes `ChangeAdded` with its current + working-tree content. Use when the module should *stamp out* these files. +- **With `--base`** — run `git diff --name-status -- ` in the + checkout; old content via `git show :`, new content from the + working tree (captures uncommitted state too). Produces real + added/modified/deleted/renamed classification, so modified YAML becomes SMP + patches. Use when the module should *transform* an existing repo. + +Metadata: `RepoURL` from `git remote get-url origin` (empty if none); +`BaseBranch` = `--base` if it names a branch, else the current branch; +`Provider` inferred from the remote host (`github.com` → github, `gitlab` in +host → gitlab, else empty). `Title`/`Body` empty — `--name` is required when +the only sources are snapshots. + +## Composition + +``` +loom generate [...] +``` + +Refs are fetched independently and composed **in the order given** (oldest → +newest; order is authoritative and never inferred). + +### CS1: Per-file squash + +Composition folds changesets left to right into a map keyed by *current path*, +tracking rename chains (a rename `a→b` re-keys the entry; later changes to `b` +land on the same entry; the final entry records original path → final path). + +For each file, the net change keeps the **old content from the first** +changeset that touched it and the **new content from the last**: + +| earlier \ later | added | modified | deleted | renamed | +|---|---|---|---|---| +| **added** | added* | added | *(dropped)* | added @ new path | +| **modified** | modified* | modified | deleted | renamed + content change | +| **deleted** | modified¹ | modified* | deleted* | — | +| **renamed** | renamed* | renamed + content change | deleted @ old path | renamed old→final | + +¹ delete-then-re-add nets to *modified* (old = original content, new = re-added +content) — SMP-able for YAML. +\* same-cell pairs shouldn't occur from well-ordered sources; treated as +last-wins with a warning. + +### CS2: Continuity check + +Before folding changeset *N* onto the accumulated state: if *N*'s old content +for a file differs from the accumulated new content, log a warning +(`"file changed between sources (source and ); composing anyway (manual review recommended)"`). +This surfaces interleaved out-of-band commits without blocking. + +### CS3: Repo consistency + +All sources with a non-empty `RepoURL` must normalize to the same repository +(compare host + path, ignoring scheme/`.git`). Mismatch → error: +`sources reference different repositories: vs `. A snapshot source with +no remote is compatible with anything but contributes no `RepoURL`. + +### CS4: Metadata merge + +| Field | Rule | +|-------|------| +| Title/Body | From the **last** source that has one (it represents the desired state). | +| BaseBranch | From the last source that has one; else repo default branch. | +| HeadBranch | From the last PR source; if none, synthesized as `loom/`. | +| RepoURL / Provider | From any source that has them (all agree per CS3). | + +Module name derivation (G3) then applies to the merged Title as today. + +### CS5: Empty net change + +If composition cancels everything out (e.g. a PR and its revert), error: +`no net file changes after composing sources`. + +## GitOps operations + +Unchanged for PR-backed generation (GO1–GO4). New cases: + +- **HeadBranch empty** (commit/snapshot-only): `target.featureBranch` = + `loom/` (parameterized). +- **Provider empty** (snapshot with no recognizable remote): the `pr` operation + is **omitted** with a warning; `commitPush` is still emitted. If `RepoURL` is + also empty, `target` is omitted entirely and the module is emitted as a + local-run module (user fills in `target` by hand — warning logged). +- **Commit message** (GO3) falls back to `"apply "` when Title is + empty. + +## CLI summary + +```bash +# several PRs, oldest first — net effect of all three +loom generate github:org/gitops#42 github:org/gitops#47 github:org/gitops#51 \ + -p serviceName=payments -o ./my-module + +# a PR plus the follow-up fix commit +loom generate github:org/gitops#42 github:org/gitops@9f3ab12 ... + +# commit range (short-form sugar) +loom generate 'github:org/gitops@a1b2c3d...f6e5d4c' ... + +# commit range on any git host — no API involved +loom generate 'git@bitbucket.org:org/gitops.git@a1b2c3d...f6e5d4c' ... + +# current state of files in a local checkout (stamp-out module) +loom generate ./gitops-checkout --include 'services/payments/**' -n onboard-payments ... + +# current state diffed against a baseline (transform module) +loom generate ./gitops-checkout --base main --include 'argocd/**' -n update-argo ... +``` + +New flags: `--include`, `--exclude`, `--base` (apply to snapshot refs; error if +given without one). Existing flags (`-p`, `-o`, `-n`, `--token-env`) unchanged; +`--token-env` applies to PR sources only — commit and snapshot sources are +git-native and use the user's git credentials. + +## Error conditions (new) + +| Condition | Error | +|-----------|-------| +| Sources resolve to different repos | `sources reference different repositories: vs ` | +| Net changeset empty | `no net file changes after composing sources` | +| Snapshot ref without `--include` | `snapshot source requires at least one --include` | +| `--include`/`--base` without a snapshot ref | `--include/--base require a local path source` | +| Snapshot path not a git repo with `--base` | `--base requires to be a git repository` | +| Snapshot-only sources without `-n` | `--name is required when generating from local files` | +| Bad commit ref | `cannot resolve commit "" in : ...` | +| Clone/fetch failure (auth, unreachable host) | `fetching : ...` (surfaces the underlying git error) | + +## Implementation phases + +1. **Refactor (no behavior change)**: `PRInfo` → `ChangeSet`, `DiffProvider` → + `ChangeSource`, `ParsePRRef` → `ParseSourceRef`. Existing tests must pass. +2. **Composition + multi-PR**: `Compose()` with CS1–CS5, accept multiple refs. + This alone covers the most common "took a few PRs" scenario. +3. **Commit source**: git-native fetch/diff (bare partial clone or local + checkout), single-commit sugar, commit/compare URL normalization. +4. **Snapshot source**: local git integration, `--include/--exclude/--base`, + target-less emission path. + +Each phase is independently shippable and spec-testable. From 4260f8c08f60d44fc2f8f796d67ce2f74d9e00ff Mon Sep 17 00:00:00 2001 From: Rick Liu Date: Thu, 16 Jul 2026 14:03:34 +0100 Subject: [PATCH 3/8] feat(generate): git-native commit source MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Generate modules from a single commit or commit range using git only — no provider API. Works against any git host (GitHub, GitLab, Bitbucket, Gitea, bare SSH remotes) and authenticates with the user's git credentials. Reference grammar: @ or @..., where is a short-form repo (github:o/r), any git URL, or a local checkout path. Commit and compare URLs are accepted as sugar. Remote repos are cloned bare with --filter=blob:none so cost scales with changed files, with a full-clone fallback for servers without partial clone support. Co-Authored-By: Claude Fable 5 --- pkg/generate/generate_test.go | 67 +++++++++ pkg/generate/provider.go | 105 ++++++++++++- pkg/generate/source_commit.go | 237 ++++++++++++++++++++++++++++++ pkg/generate/source_git_test.go | 211 ++++++++++++++++++++++++++ pkg/generate/source_parse_test.go | 113 ++++++++++++++ 5 files changed, 731 insertions(+), 2 deletions(-) create mode 100644 pkg/generate/source_commit.go create mode 100644 pkg/generate/source_git_test.go create mode 100644 pkg/generate/source_parse_test.go diff --git a/pkg/generate/generate_test.go b/pkg/generate/generate_test.go index 62a024a..925fff4 100644 --- a/pkg/generate/generate_test.go +++ b/pkg/generate/generate_test.go @@ -1472,6 +1472,73 @@ func TestBuildModule_SynthesizesFeatureBranchAndTitle(t *testing.T) { // --- End-to-end: multiple sources composed through Run --- +func TestRun_ComposesMultipleCommitSources(t *testing.T) { + r := newFixtureRepo(t) + c1, c2, c3 := seedHistory(r) + outputDir := filepath.Join(t.TempDir(), "module") + + opts := Options{ + Refs: []string{ + r.dir + "@" + c1 + "..." + c2, + r.dir + "@" + c2 + "..." + c3, + }, + OutputDir: outputDir, + ModuleName: "combo", + } + if err := Run(t.Context(), opts, testLogger()); err != nil { + t.Fatal(err) + } + + data, err := os.ReadFile(filepath.Join(outputDir, "loom.yaml")) + if err != nil { + t.Fatal(err) + } + var lf config.LoomFile + if err := yaml.Unmarshal(data, &lf); err != nil { + t.Fatalf("invalid loom.yaml: %v", err) + } + if lf.Metadata.Name != "combo" { + t.Errorf("module name = %q", lf.Metadata.Name) + } + + // Net effect of both ranges: renamed.yaml added (add+rename composed), + // base.yaml patched, app.json deleted. Fixture has no origin remote, so + // target and pr degrade away; commitPush remains. + var hasNewFiles, hasPatch, hasRm, hasCommit, hasPR bool + for _, op := range lf.Spec.Operations { + switch { + case op.NewFiles != nil: + hasNewFiles = true + case op.Patch != nil: + hasPatch = true + if op.Patch.Target != "config/base.yaml" { + t.Errorf("patch target = %q", op.Patch.Target) + } + case op.Shell != nil && strings.Contains(op.Shell.Command, "rm"): + hasRm = true + case op.CommitPush != nil: + hasCommit = true + case op.PR != nil: + hasPR = true + } + } + if !hasNewFiles || !hasPatch || !hasRm || !hasCommit { + t.Errorf("missing operations (newFiles=%v patch=%v rm=%v commit=%v): %+v", + hasNewFiles, hasPatch, hasRm, hasCommit, lf.Spec.Operations) + } + if hasPR { + t.Error("expected pr operation to be omitted (no remote)") + } + if lf.Spec.Target != nil { + t.Error("expected target to be omitted (no remote)") + } + + // The added-then-renamed file lands as a template at its final path. + if _, err := os.Stat(filepath.Join(outputDir, "config", "renamed.yaml")); err != nil { + t.Errorf("expected template config/renamed.yaml: %v", err) + } +} + func TestRun_SnapshotFlagsRequireSnapshotSource(t *testing.T) { opts := Options{ Refs: []string{"github:o/r#1"}, diff --git a/pkg/generate/provider.go b/pkg/generate/provider.go index 5298d1f..d064ec4 100644 --- a/pkg/generate/provider.go +++ b/pkg/generate/provider.go @@ -96,12 +96,34 @@ var ( func ParseSourceRef(ref string, snap SnapshotOptions, logger *slog.Logger) (*Source, error) { logger.Debug("parsing source reference", "ref", ref) + // Local paths — snapshot, or commit source when an @ suffix is + // present. Only hex SHAs are accepted as rev suffixes on paths so that + // directory names containing '@' are not misclassified. + if path, ok := strings.CutPrefix(ref, "file:"); ok { + return parseLocalRef(path, snap, logger) + } + if isPathLike(ref) { + return parseLocalRef(ref, snap, logger) + } + // Short-form references — unambiguous, checked first. - if strings.HasPrefix(ref, "github:") { + if body, ok := strings.CutPrefix(ref, "github:"); ok { + if repo, base, head, ok := splitRevSuffix(body, true); ok { + logger.Debug("detected GitHub short-form commit reference") + return &Source{Kind: KindCommit, Provider: "github", ChangeSource: &CommitSource{ + RepoURL: fmt.Sprintf("git@github.com:%s.git", repo), BaseRev: base, HeadRev: head, Provider: "github", + }}, nil + } logger.Debug("detected GitHub short-form reference") return &Source{Kind: KindPR, Provider: "github", ChangeSource: &GitHubSource{}}, nil } - if strings.HasPrefix(ref, "gitlab:") { + if body, ok := strings.CutPrefix(ref, "gitlab:"); ok { + if repo, base, head, ok := splitRevSuffix(body, true); ok { + logger.Debug("detected GitLab short-form commit reference") + return &Source{Kind: KindCommit, Provider: "gitlab", ChangeSource: &CommitSource{ + RepoURL: fmt.Sprintf("git@gitlab.com:%s.git", repo), BaseRev: base, HeadRev: head, Provider: "gitlab", + }}, nil + } logger.Debug("detected GitLab short-form reference") return &Source{Kind: KindPR, Provider: "gitlab", ChangeSource: &GitLabSource{}}, nil } @@ -116,9 +138,88 @@ func ParseSourceRef(ref string, snap SnapshotOptions, logger *slog.Logger) (*Sou return &Source{Kind: KindPR, Provider: "gitlab", ChangeSource: &GitLabSource{}}, nil } + // Commit / compare URLs — sugar for @, handled via git. + if m := commitURLPattern.FindStringSubmatch(ref); m != nil { + logger.Debug("detected commit URL") + repoURL := m[1] + ".git" + provider := inferProviderFromURL(repoURL) + return &Source{Kind: KindCommit, Provider: provider, ChangeSource: &CommitSource{ + RepoURL: repoURL, HeadRev: m[2], Provider: provider, + }}, nil + } + if m := compareURLPattern.FindStringSubmatch(ref); m != nil { + logger.Debug("detected compare URL") + repoURL := m[1] + ".git" + provider := inferProviderFromURL(repoURL) + return &Source{Kind: KindCommit, Provider: provider, ChangeSource: &CommitSource{ + RepoURL: repoURL, BaseRev: m[2], HeadRev: m[3], Provider: provider, + }}, nil + } + + // Any other git URL with an @ suffix — platform-agnostic commit ref. + if repo, base, head, ok := splitRevSuffix(ref, true); ok && looksLikeGitURL(repo) { + logger.Debug("detected git URL commit reference") + provider := inferProviderFromURL(repo) + return &Source{Kind: KindCommit, Provider: provider, ChangeSource: &CommitSource{ + RepoURL: repo, BaseRev: base, HeadRev: head, Provider: provider, + }}, nil + } + return nil, fmt.Errorf("cannot detect source kind from reference %q; use a PR/MR URL, a @ commit reference, a local path, or prefix with github: or gitlab:", ref) } +func parseLocalRef(path string, snap SnapshotOptions, logger *slog.Logger) (*Source, error) { + if repo, base, head, ok := splitRevSuffix(path, false); ok { + logger.Debug("detected local commit reference", "path", repo) + return &Source{Kind: KindCommit, ChangeSource: &CommitSource{ + LocalPath: repo, BaseRev: base, HeadRev: head, + }}, nil + } + return nil, fmt.Errorf("local path %q is not a commit reference; snapshot sources are not supported yet", path) +} + +func isPathLike(ref string) bool { + return ref == "." || ref == ".." || + strings.HasPrefix(ref, "./") || strings.HasPrefix(ref, "../") || strings.HasPrefix(ref, "/") +} + +// splitRevSuffix splits "@" or "@..." on the +// last '@'. When allowTags is false only hex SHAs are accepted (used for +// local paths, where '@' may legitimately appear in directory names). +func splitRevSuffix(s string, allowTags bool) (repo, base, head string, ok bool) { + i := strings.LastIndex(s, "@") + if i <= 0 || i == len(s)-1 { + return "", "", "", false + } + repo, rev := s[:i], s[i+1:] + validToken := func(t string) bool { + if shaPattern.MatchString(t) { + return true + } + return allowTags && revTokenPattern.MatchString(t) + } + if from, to, found := strings.Cut(rev, "..."); found { + if validToken(from) && validToken(to) { + return repo, from, to, true + } + return "", "", "", false + } + if validToken(rev) { + return repo, "", rev, true + } + return "", "", "", false +} + +// looksLikeGitURL reports whether s is plausibly a remote git repository URL. +func looksLikeGitURL(s string) bool { + if strings.Contains(s, "://") { + return true + } + // scp-like syntax: user@host:path + head, rest, found := strings.Cut(s, "@") + return found && head != "" && strings.Contains(rest, ":") +} + // inferProviderFromURL guesses the provider from the repository URL host. // It is only used to populate the generated pr operation; an empty result // degrades to omitting that operation. diff --git a/pkg/generate/source_commit.go b/pkg/generate/source_commit.go new file mode 100644 index 0000000..e891fd8 --- /dev/null +++ b/pkg/generate/source_commit.go @@ -0,0 +1,237 @@ +package generate + +import ( + "bytes" + "context" + "fmt" + "log/slog" + "os" + "os/exec" + "strings" +) + +// CommitSource fetches the diff of a commit or commit range using git only — +// no provider API. It works against any git host and authenticates with the +// user's existing git credentials. +type CommitSource struct { + // RepoURL is the remote repository URL. Empty when LocalPath is set. + RepoURL string + // LocalPath points at an existing local checkout to read from instead of + // cloning. + LocalPath string + // BaseRev is the base of the range. Empty means HeadRev's first parent + // (single-commit form). + BaseRev string + // HeadRev is the commit whose state the module should reproduce. + HeadRev string + // Provider is a hint inferred from the repo host ("github", "gitlab", or ""). + Provider string +} + +// Fetch implements ChangeSource. +func (s *CommitSource) Fetch(ctx context.Context, _ string, _ string, logger *slog.Logger) (*ChangeSet, error) { + if !hasBinary("git", logger) { + return nil, fmt.Errorf("git CLI is required for commit sources") + } + + dir := s.LocalPath + repoURL := s.RepoURL + if dir == "" { + tmp, err := os.MkdirTemp("", "loom-commit-*") + if err != nil { + return nil, err + } + defer os.RemoveAll(tmp) + if err := cloneBare(ctx, s.RepoURL, tmp, logger); err != nil { + return nil, fmt.Errorf("fetching %s: %w", s.RepoURL, err) + } + dir = tmp + } else { + if _, err := gitOut(ctx, dir, "rev-parse", "--git-dir"); err != nil { + return nil, fmt.Errorf("%s is not a git repository: %w", dir, err) + } + if repoURL == "" { + if out, err := gitOut(ctx, dir, "remote", "get-url", "origin"); err == nil { + repoURL = strings.TrimSpace(string(out)) + } + } + } + + head := s.HeadRev + base := s.BaseRev + if base == "" { + // Single commit: diff against the first parent. + base = head + "^" + } + for _, rev := range []string{s.HeadRev, s.BaseRev} { + if rev == "" { + continue + } + if err := ensureRev(ctx, dir, rev, logger); err != nil { + return nil, err + } + } + // The synthesized parent rev only resolves once the head commit exists. + if s.BaseRev == "" { + if _, err := gitOut(ctx, dir, "rev-parse", "--verify", "--quiet", base+"^{commit}"); err != nil { + return nil, fmt.Errorf("cannot resolve parent of commit %q (shallow history?): %w", head, err) + } + } + + files, err := gitDiffFiles(ctx, dir, base, head, logger) + if err != nil { + return nil, err + } + + title, body := commitMessage(ctx, dir, head) + provider := s.Provider + if provider == "" { + provider = inferProviderFromURL(repoURL) + } + + return &ChangeSet{ + Title: title, + Body: body, + BaseBranch: defaultBranch(ctx, dir, logger), + RepoURL: repoURL, + Provider: provider, + Files: files, + }, nil +} + +// cloneBare clones url into dir as a bare partial clone (blobs fetched +// lazily). Servers that reject partial clone fall back to a full bare clone. +func cloneBare(ctx context.Context, url, dir string, logger *slog.Logger) error { + logger.Info("cloning repository", "url", url) + _, err := gitOut(ctx, ".", "clone", "--bare", "--filter=blob:none", url, dir) + if err == nil { + return nil + } + logger.Debug("partial clone failed; retrying full bare clone", "error", err) + if rmErr := os.RemoveAll(dir); rmErr != nil { + return rmErr + } + _, err = gitOut(ctx, ".", "clone", "--bare", url, dir) + return err +} + +// ensureRev makes rev resolvable in dir, fetching it from origin if needed +// (e.g. a SHA not reachable from any branch, or missing from a local clone). +func ensureRev(ctx context.Context, dir, rev string, logger *slog.Logger) error { + if _, err := gitOut(ctx, dir, "rev-parse", "--verify", "--quiet", rev+"^{commit}"); err == nil { + return nil + } + logger.Debug("rev not present locally; fetching from origin", "rev", rev) + if _, err := gitOut(ctx, dir, "fetch", "origin", rev); err != nil { + return fmt.Errorf("cannot resolve commit %q: %w", rev, err) + } + if _, err := gitOut(ctx, dir, "rev-parse", "--verify", "--quiet", rev+"^{commit}"); err != nil { + return fmt.Errorf("cannot resolve commit %q: %w", rev, err) + } + return nil +} + +// gitDiffFiles diffs base against head (or against the working tree when +// head is empty) and materializes old/new contents for each changed file. +func gitDiffFiles(ctx context.Context, dir, base, head string, logger *slog.Logger) ([]FileChange, error) { + args := []string{"diff", "-M", "--name-status", "-z", base} + if head != "" { + args = append(args, head) + } + out, err := gitOut(ctx, dir, args...) + if err != nil { + return nil, err + } + + readOld := func(path string) []byte { + content, err := gitOut(ctx, dir, "show", base+":"+path) + if err != nil { + logger.Warn("cannot read old content", "file", path, "error", err) + return nil + } + return content + } + readNew := func(path string) []byte { + var content []byte + var err error + if head != "" { + content, err = gitOut(ctx, dir, "show", head+":"+path) + } else { + content, err = os.ReadFile(dir + "/" + path) + } + if err != nil { + logger.Warn("cannot read new content", "file", path, "error", err) + return nil + } + return content + } + + var files []FileChange + fields := strings.Split(strings.TrimSuffix(string(out), "\x00"), "\x00") + for i := 0; i < len(fields); i++ { + status := fields[i] + if status == "" { + continue + } + switch status[0] { + case 'A': + i++ + files = append(files, FileChange{Type: ChangeAdded, Path: fields[i], NewContent: readNew(fields[i])}) + case 'M', 'T': + i++ + files = append(files, FileChange{Type: ChangeModified, Path: fields[i], OldContent: readOld(fields[i]), NewContent: readNew(fields[i])}) + case 'D': + i++ + files = append(files, FileChange{Type: ChangeDeleted, Path: fields[i], OldContent: readOld(fields[i])}) + case 'R': + oldPath, newPath := fields[i+1], fields[i+2] + i += 2 + files = append(files, FileChange{Type: ChangeRenamed, Path: newPath, OldPath: oldPath, OldContent: readOld(oldPath), NewContent: readNew(newPath)}) + case 'C': + // Copies: the new path is a new file. + newPath := fields[i+2] + i += 2 + files = append(files, FileChange{Type: ChangeAdded, Path: newPath, NewContent: readNew(newPath)}) + default: + logger.Warn("unhandled diff status; file skipped", "status", status, "file", fields[i+1]) + i++ + } + } + return files, nil +} + +func commitMessage(ctx context.Context, dir, rev string) (title, body string) { + if out, err := gitOut(ctx, dir, "log", "-1", "--format=%s", rev); err == nil { + title = strings.TrimSpace(string(out)) + } + if out, err := gitOut(ctx, dir, "log", "-1", "--format=%b", rev); err == nil { + body = strings.TrimSpace(string(out)) + } + return title, body +} + +// defaultBranch returns the repository's default branch: for bare clones the +// clone's HEAD mirrors the remote default; for checkouts, origin's HEAD, then +// the current branch. +func defaultBranch(ctx context.Context, dir string, logger *slog.Logger) string { + if out, err := gitOut(ctx, dir, "symbolic-ref", "--short", "refs/remotes/origin/HEAD"); err == nil { + return strings.TrimPrefix(strings.TrimSpace(string(out)), "origin/") + } + if out, err := gitOut(ctx, dir, "symbolic-ref", "--short", "HEAD"); err == nil { + return strings.TrimSpace(string(out)) + } + logger.Debug("cannot determine default branch") + return "" +} + +// gitOut runs git with -C dir and returns stdout. +func gitOut(ctx context.Context, dir string, args ...string) ([]byte, error) { + cmd := exec.CommandContext(ctx, "git", append([]string{"-C", dir}, args...)...) + var stdout, stderr bytes.Buffer + cmd.Stdout = &stdout + cmd.Stderr = &stderr + if err := cmd.Run(); err != nil { + return nil, fmt.Errorf("git %s: %w: %s", strings.Join(args, " "), err, strings.TrimSpace(stderr.String())) + } + return stdout.Bytes(), nil +} diff --git a/pkg/generate/source_git_test.go b/pkg/generate/source_git_test.go new file mode 100644 index 0000000..0e0df29 --- /dev/null +++ b/pkg/generate/source_git_test.go @@ -0,0 +1,211 @@ +package generate + +import ( + "context" + "os" + "os/exec" + "path/filepath" + "strings" + "testing" +) + +// fixtureRepo drives a throwaway git repository for source tests. +type fixtureRepo struct { + t *testing.T + dir string +} + +func newFixtureRepo(t *testing.T) *fixtureRepo { + t.Helper() + if _, err := exec.LookPath("git"); err != nil { + t.Skip("git CLI not available") + } + r := &fixtureRepo{t: t, dir: t.TempDir()} + r.git("init", "-b", "main") + r.git("config", "user.email", "test@example.com") + r.git("config", "user.name", "Test") + r.git("config", "commit.gpgsign", "false") + return r +} + +func (r *fixtureRepo) git(args ...string) string { + r.t.Helper() + out, err := gitOut(context.Background(), r.dir, args...) + if err != nil { + r.t.Fatalf("fixture: %v", err) + } + return strings.TrimSpace(string(out)) +} + +func (r *fixtureRepo) write(rel, content string) { + r.t.Helper() + path := filepath.Join(r.dir, rel) + if err := os.MkdirAll(filepath.Dir(path), 0o755); err != nil { + r.t.Fatal(err) + } + if err := os.WriteFile(path, []byte(content), 0o644); err != nil { + r.t.Fatal(err) + } +} + +func (r *fixtureRepo) commit(message string) string { + r.t.Helper() + r.git("add", "-A") + r.git("commit", "-m", message) + return r.git("rev-parse", "HEAD") +} + +// seedHistory builds three commits and returns their SHAs: +// +// c1: base.yaml (replicas: 1), app.json +// c2: base.yaml → replicas: 3, +new.yaml, -app.json (has a body) +// c3: new.yaml renamed to renamed.yaml +func seedHistory(r *fixtureRepo) (c1, c2, c3 string) { + r.write("config/base.yaml", "name: app\nreplicas: 1\n") + r.write("app.json", `{"a":1}`) + c1 = r.commit("initial commit") + + r.write("config/base.yaml", "name: app\nreplicas: 3\n") + r.write("config/new.yaml", "fresh: true\n") + r.git("rm", "-q", "app.json") + c2 = r.commit("scale app to three replicas\n\nthe body text") + + r.git("mv", "config/new.yaml", "config/renamed.yaml") + c3 = r.commit("rename new to renamed") + return c1, c2, c3 +} + +func fileByPath(t *testing.T, files []FileChange, path string) FileChange { + t.Helper() + for _, f := range files { + if f.Path == path { + return f + } + } + t.Fatalf("file %q not found in %+v", path, files) + return FileChange{} +} + +// --- CommitSource --- + +func TestCommitSource_SingleCommit(t *testing.T) { + r := newFixtureRepo(t) + _, c2, _ := seedHistory(r) + + src := &CommitSource{LocalPath: r.dir, HeadRev: c2} + cs, err := src.Fetch(context.Background(), "", "", testLogger()) + if err != nil { + t.Fatal(err) + } + + if cs.Title != "scale app to three replicas" { + t.Errorf("title = %q", cs.Title) + } + if cs.Body != "the body text" { + t.Errorf("body = %q", cs.Body) + } + if cs.BaseBranch != "main" { + t.Errorf("baseBranch = %q, want main", cs.BaseBranch) + } + if len(cs.Files) != 3 { + t.Fatalf("expected 3 file changes, got %+v", cs.Files) + } + + mod := fileByPath(t, cs.Files, "config/base.yaml") + if mod.Type != ChangeModified { + t.Errorf("base.yaml type = %v, want modified", mod.Type) + } + if !strings.Contains(string(mod.OldContent), "replicas: 1") || !strings.Contains(string(mod.NewContent), "replicas: 3") { + t.Errorf("base.yaml contents wrong: %q → %q", mod.OldContent, mod.NewContent) + } + + added := fileByPath(t, cs.Files, "config/new.yaml") + if added.Type != ChangeAdded || string(added.NewContent) != "fresh: true\n" { + t.Errorf("new.yaml: %v %q", added.Type, added.NewContent) + } + + deleted := fileByPath(t, cs.Files, "app.json") + if deleted.Type != ChangeDeleted { + t.Errorf("app.json type = %v, want deleted", deleted.Type) + } +} + +func TestCommitSource_Range(t *testing.T) { + r := newFixtureRepo(t) + c1, _, c3 := seedHistory(r) + + src := &CommitSource{LocalPath: r.dir, BaseRev: c1, HeadRev: c3} + cs, err := src.Fetch(context.Background(), "", "", testLogger()) + if err != nil { + t.Fatal(err) + } + + // Across the whole range: base.yaml modified, renamed.yaml added + // (new.yaml never existed at c1), app.json deleted. + if len(cs.Files) != 3 { + t.Fatalf("expected 3 file changes, got %+v", cs.Files) + } + if f := fileByPath(t, cs.Files, "config/renamed.yaml"); f.Type != ChangeAdded { + t.Errorf("renamed.yaml type = %v, want added", f.Type) + } + // Range metadata comes from the head commit. + if cs.Title != "rename new to renamed" { + t.Errorf("title = %q", cs.Title) + } +} + +func TestCommitSource_Rename(t *testing.T) { + r := newFixtureRepo(t) + _, c2, c3 := seedHistory(r) + + src := &CommitSource{LocalPath: r.dir, BaseRev: c2, HeadRev: c3} + cs, err := src.Fetch(context.Background(), "", "", testLogger()) + if err != nil { + t.Fatal(err) + } + + if len(cs.Files) != 1 { + t.Fatalf("expected 1 file change, got %+v", cs.Files) + } + f := cs.Files[0] + if f.Type != ChangeRenamed || f.OldPath != "config/new.yaml" || f.Path != "config/renamed.yaml" { + t.Errorf("unexpected rename: %+v", f) + } +} + +func TestCommitSource_RemoteClone(t *testing.T) { + r := newFixtureRepo(t) + _, c2, _ := seedHistory(r) + + src := &CommitSource{RepoURL: "file://" + r.dir, HeadRev: c2} + cs, err := src.Fetch(context.Background(), "", "", testLogger()) + if err != nil { + t.Fatal(err) + } + if len(cs.Files) != 3 { + t.Errorf("expected 3 file changes, got %+v", cs.Files) + } + if cs.RepoURL != "file://"+r.dir { + t.Errorf("repoURL = %q", cs.RepoURL) + } + if cs.BaseBranch != "main" { + t.Errorf("baseBranch = %q, want main", cs.BaseBranch) + } +} + +func TestCommitSource_BadRev(t *testing.T) { + r := newFixtureRepo(t) + seedHistory(r) + + src := &CommitSource{LocalPath: r.dir, HeadRev: "deadbeef1234567"} + if _, err := src.Fetch(context.Background(), "", "", testLogger()); err == nil { + t.Error("expected error for unresolvable rev") + } +} + +func TestCommitSource_NotARepo(t *testing.T) { + src := &CommitSource{LocalPath: t.TempDir(), HeadRev: "abc1234"} + if _, err := src.Fetch(context.Background(), "", "", testLogger()); err == nil { + t.Error("expected error for non-repo path") + } +} diff --git a/pkg/generate/source_parse_test.go b/pkg/generate/source_parse_test.go new file mode 100644 index 0000000..7e780c3 --- /dev/null +++ b/pkg/generate/source_parse_test.go @@ -0,0 +1,113 @@ +package generate + +import ( + "testing" +) + +func parseCommitRef(t *testing.T, ref string) (*Source, *CommitSource) { + t.Helper() + src, err := ParseSourceRef(ref, SnapshotOptions{}, testLogger()) + if err != nil { + t.Fatalf("ParseSourceRef(%q): %v", ref, err) + } + if src.Kind != KindCommit { + t.Fatalf("ParseSourceRef(%q) kind = %v, want commit", ref, src.Kind) + } + cs, ok := src.ChangeSource.(*CommitSource) + if !ok { + t.Fatalf("ParseSourceRef(%q) source type = %T, want *CommitSource", ref, src.ChangeSource) + } + return src, cs +} + +func TestParseSourceRef_CommitRefs(t *testing.T) { + tests := []struct { + ref string + repoURL string + base string + head string + provider string + }{ + // Short-form sugar. + {"github:o/r@abc1234", "git@github.com:o/r.git", "", "abc1234", "github"}, + {"github:o/r@abc1234...def5678", "git@github.com:o/r.git", "abc1234", "def5678", "github"}, + {"gitlab:g/r@abc1234", "git@gitlab.com:g/r.git", "", "abc1234", "gitlab"}, + // Tags allowed on unambiguous repos. + {"github:o/r@v1.0.0...v1.1.0", "git@github.com:o/r.git", "v1.0.0", "v1.1.0", "github"}, + // Commit / compare URL sugar. + {"https://github.com/o/r/commit/abc1234", "https://github.com/o/r.git", "", "abc1234", "github"}, + {"https://gitlab.com/g/r/-/commit/abc1234", "https://gitlab.com/g/r.git", "", "abc1234", "gitlab"}, + {"https://github.com/o/r/compare/abc1234...def5678", "https://github.com/o/r.git", "abc1234", "def5678", "github"}, + {"https://gitlab.com/g/r/-/compare/abc1234...def5678", "https://gitlab.com/g/r.git", "abc1234", "def5678", "gitlab"}, + // Arbitrary git URLs — platform-agnostic. + {"git@bitbucket.org:o/r.git@abc1234...def5678", "git@bitbucket.org:o/r.git", "abc1234", "def5678", ""}, + {"https://gitea.example.com/o/r.git@abc1234", "https://gitea.example.com/o/r.git", "", "abc1234", ""}, + {"git@github.enterprise.io:o/r.git@abc1234", "git@github.enterprise.io:o/r.git", "", "abc1234", "github"}, + } + + for _, tt := range tests { + src, cs := parseCommitRef(t, tt.ref) + if cs.RepoURL != tt.repoURL { + t.Errorf("%q: repoURL = %q, want %q", tt.ref, cs.RepoURL, tt.repoURL) + } + if cs.BaseRev != tt.base || cs.HeadRev != tt.head { + t.Errorf("%q: range = %q...%q, want %q...%q", tt.ref, cs.BaseRev, cs.HeadRev, tt.base, tt.head) + } + if src.Provider != tt.provider { + t.Errorf("%q: provider = %q, want %q", tt.ref, src.Provider, tt.provider) + } + } +} + +func TestParseSourceRef_LocalCommitRef(t *testing.T) { + _, cs := parseCommitRef(t, "./checkout@a1b2c3d...f6e5d4c") + if cs.LocalPath != "./checkout" { + t.Errorf("localPath = %q, want ./checkout", cs.LocalPath) + } + if cs.BaseRev != "a1b2c3d" || cs.HeadRev != "f6e5d4c" { + t.Errorf("range = %q...%q", cs.BaseRev, cs.HeadRev) + } +} + +func TestParseSourceRef_PRShortFormStillWins(t *testing.T) { + // A '#' short-form must remain a PR source even though it contains no rev. + src, err := ParseSourceRef("github:owner/repo#123", SnapshotOptions{}, testLogger()) + if err != nil { + t.Fatal(err) + } + if src.Kind != KindPR { + t.Errorf("kind = %v, want pr", src.Kind) + } +} + +func TestParseSourceRef_BareGitURLWithoutRevRejected(t *testing.T) { + // A git URL without @rev is not a valid source (nothing to diff). + refs := []string{ + "git@github.com:o/r.git", + "https://github.com/o/r.git", + } + for _, ref := range refs { + if _, err := ParseSourceRef(ref, SnapshotOptions{}, testLogger()); err == nil { + t.Errorf("ParseSourceRef(%q) expected error", ref) + } + } +} + +func TestInferProviderFromURL(t *testing.T) { + tests := []struct { + url string + want string + }{ + {"git@github.com:o/r.git", "github"}, + {"https://github.enterprise.io/o/r.git", "github"}, + {"git@gitlab.com:g/r.git", "gitlab"}, + {"https://gitlab.example.com/g/r.git", "gitlab"}, + {"git@bitbucket.org:o/r.git", ""}, + {"", ""}, + } + for _, tt := range tests { + if got := inferProviderFromURL(tt.url); got != tt.want { + t.Errorf("inferProviderFromURL(%q) = %q, want %q", tt.url, got, tt.want) + } + } +} From 3b7cf88c624de5228b40d49d7f42c18b5b2b7767 Mon Sep 17 00:00:00 2001 From: Rick Liu Date: Thu, 16 Jul 2026 14:09:31 +0100 Subject: [PATCH 4/8] feat(generate): snapshot source for local files MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Generate modules from the current state of files in a local checkout. A local path ref with --include globs captures matched files: without --base every file becomes a template (stamp-out module); with --base the working tree — including uncommitted and untracked files — is diffed against that git ref so modified YAML still produces SMP patches (transform module). Repo URL, base branch, and provider are read from the checkout's git remote when available; modules without a remote degrade per the generate-sources spec. --name is required when generating from local files only. Co-Authored-By: Claude Fable 5 --- pkg/generate/generate_test.go | 15 +++ pkg/generate/provider.go | 5 +- pkg/generate/source_git_test.go | 108 ++++++++++++++++ pkg/generate/source_parse_test.go | 58 +++++++++ pkg/generate/source_snapshot.go | 207 ++++++++++++++++++++++++++++++ 5 files changed, 392 insertions(+), 1 deletion(-) create mode 100644 pkg/generate/source_snapshot.go diff --git a/pkg/generate/generate_test.go b/pkg/generate/generate_test.go index 925fff4..0cb6b57 100644 --- a/pkg/generate/generate_test.go +++ b/pkg/generate/generate_test.go @@ -1539,6 +1539,21 @@ func TestRun_ComposesMultipleCommitSources(t *testing.T) { } } +func TestRun_SnapshotOnlyRequiresName(t *testing.T) { + dir := t.TempDir() + mustWrite(t, dir, "a/f.yaml", "x: 1\n") + + opts := Options{ + Refs: []string{dir}, + Include: []string{"a/**"}, + OutputDir: t.TempDir(), + } + err := Run(t.Context(), opts, testLogger()) + if err == nil || !strings.Contains(err.Error(), "--name is required") { + t.Errorf("expected name-required error, got %v", err) + } +} + func TestRun_SnapshotFlagsRequireSnapshotSource(t *testing.T) { opts := Options{ Refs: []string{"github:o/r#1"}, diff --git a/pkg/generate/provider.go b/pkg/generate/provider.go index d064ec4..724e857 100644 --- a/pkg/generate/provider.go +++ b/pkg/generate/provider.go @@ -175,7 +175,10 @@ func parseLocalRef(path string, snap SnapshotOptions, logger *slog.Logger) (*Sou LocalPath: repo, BaseRev: base, HeadRev: head, }}, nil } - return nil, fmt.Errorf("local path %q is not a commit reference; snapshot sources are not supported yet", path) + logger.Debug("detected snapshot reference", "path", path) + return &Source{Kind: KindSnapshot, ChangeSource: &SnapshotSource{ + Path: path, Include: snap.Include, Exclude: snap.Exclude, Base: snap.Base, + }}, nil } func isPathLike(ref string) bool { diff --git a/pkg/generate/source_git_test.go b/pkg/generate/source_git_test.go index 0e0df29..f06102d 100644 --- a/pkg/generate/source_git_test.go +++ b/pkg/generate/source_git_test.go @@ -209,3 +209,111 @@ func TestCommitSource_NotARepo(t *testing.T) { t.Error("expected error for non-repo path") } } + +// --- SnapshotSource --- + +func TestSnapshotSource_NoBase_CapturesMatchedFiles(t *testing.T) { + dir := t.TempDir() + mustWrite(t, dir, "services/payments/deploy.yaml", "kind: Deployment\n") + mustWrite(t, dir, "services/payments/svc.yaml", "kind: Service\n") + mustWrite(t, dir, "services/billing/deploy.yaml", "kind: Deployment\n") + mustWrite(t, dir, "README.md", "docs\n") + + src := &SnapshotSource{Path: dir, Include: []string{"services/payments/**"}} + cs, err := src.Fetch(context.Background(), "", "", testLogger()) + if err != nil { + t.Fatal(err) + } + + if len(cs.Files) != 2 { + t.Fatalf("expected 2 files, got %+v", cs.Files) + } + for _, f := range cs.Files { + if f.Type != ChangeAdded { + t.Errorf("%s: type = %v, want added", f.Path, f.Type) + } + } + // Not a git repo: no repo metadata, degraded module expected downstream. + if cs.RepoURL != "" || cs.Provider != "" { + t.Errorf("expected empty repo metadata, got %q / %q", cs.RepoURL, cs.Provider) + } +} + +func TestSnapshotSource_Exclude(t *testing.T) { + dir := t.TempDir() + mustWrite(t, dir, "services/payments/deploy.yaml", "kind: Deployment\n") + mustWrite(t, dir, "services/payments/secret.yaml", "kind: Secret\n") + + src := &SnapshotSource{Path: dir, Include: []string{"services/**"}, Exclude: []string{"**/secret.yaml"}} + cs, err := src.Fetch(context.Background(), "", "", testLogger()) + if err != nil { + t.Fatal(err) + } + if len(cs.Files) != 1 || cs.Files[0].Path != "services/payments/deploy.yaml" { + t.Errorf("expected only deploy.yaml, got %+v", cs.Files) + } +} + +func TestSnapshotSource_WithBase_DiffsWorkingTree(t *testing.T) { + r := newFixtureRepo(t) + r.write("config/app.yaml", "replicas: 1\n") + r.write("config/keep.yaml", "keep: true\n") + r.commit("initial") + + // Uncommitted current state: modify, delete, and add an untracked file. + r.write("config/app.yaml", "replicas: 5\n") + if err := os.Remove(filepath.Join(r.dir, "config/keep.yaml")); err != nil { + t.Fatal(err) + } + r.write("config/brand-new.yaml", "new: true\n") + + src := &SnapshotSource{Path: r.dir, Include: []string{"config/**"}, Base: "main"} + cs, err := src.Fetch(context.Background(), "", "", testLogger()) + if err != nil { + t.Fatal(err) + } + + if len(cs.Files) != 3 { + t.Fatalf("expected 3 changes, got %+v", cs.Files) + } + mod := fileByPath(t, cs.Files, "config/app.yaml") + if mod.Type != ChangeModified || !strings.Contains(string(mod.OldContent), "replicas: 1") || !strings.Contains(string(mod.NewContent), "replicas: 5") { + t.Errorf("app.yaml: %v %q → %q", mod.Type, mod.OldContent, mod.NewContent) + } + if f := fileByPath(t, cs.Files, "config/keep.yaml"); f.Type != ChangeDeleted { + t.Errorf("keep.yaml type = %v, want deleted", f.Type) + } + if f := fileByPath(t, cs.Files, "config/brand-new.yaml"); f.Type != ChangeAdded { + t.Errorf("brand-new.yaml type = %v, want added (untracked)", f.Type) + } + if cs.BaseBranch != "main" { + t.Errorf("baseBranch = %q, want main", cs.BaseBranch) + } +} + +func TestSnapshotSource_RequiresInclude(t *testing.T) { + src := &SnapshotSource{Path: t.TempDir()} + _, err := src.Fetch(context.Background(), "", "", testLogger()) + if err == nil || !strings.Contains(err.Error(), "--include") { + t.Errorf("expected include-required error, got %v", err) + } +} + +func TestSnapshotSource_BaseRequiresGitRepo(t *testing.T) { + src := &SnapshotSource{Path: t.TempDir(), Include: []string{"**"}, Base: "main"} + _, err := src.Fetch(context.Background(), "", "", testLogger()) + if err == nil || !strings.Contains(err.Error(), "git repository") { + t.Errorf("expected git-repo-required error, got %v", err) + } +} + +func mustWrite(t *testing.T, dir, rel, content string) { + t.Helper() + path := filepath.Join(dir, rel) + if err := os.MkdirAll(filepath.Dir(path), 0o755); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(path, []byte(content), 0o644); err != nil { + t.Fatal(err) + } +} diff --git a/pkg/generate/source_parse_test.go b/pkg/generate/source_parse_test.go index 7e780c3..205dcee 100644 --- a/pkg/generate/source_parse_test.go +++ b/pkg/generate/source_parse_test.go @@ -69,6 +69,38 @@ func TestParseSourceRef_LocalCommitRef(t *testing.T) { } } +func TestParseSourceRef_SnapshotRefs(t *testing.T) { + snap := SnapshotOptions{Include: []string{"a/**"}, Exclude: []string{"b/**"}, Base: "main"} + tests := []struct { + ref string + path string + }{ + {"./checkout", "./checkout"}, + {"/abs/path", "/abs/path"}, + {"file:relative/dir", "relative/dir"}, + // Non-SHA suffix after @ on a path stays a snapshot path. + {"./release@v2", "./release@v2"}, + } + for _, tt := range tests { + src, err := ParseSourceRef(tt.ref, snap, testLogger()) + if err != nil { + t.Errorf("ParseSourceRef(%q): %v", tt.ref, err) + continue + } + if src.Kind != KindSnapshot { + t.Errorf("ParseSourceRef(%q) kind = %v, want snapshot", tt.ref, src.Kind) + continue + } + ss := src.ChangeSource.(*SnapshotSource) + if ss.Path != tt.path { + t.Errorf("ParseSourceRef(%q) path = %q, want %q", tt.ref, ss.Path, tt.path) + } + if len(ss.Include) != 1 || len(ss.Exclude) != 1 || ss.Base != "main" { + t.Errorf("ParseSourceRef(%q) snapshot options not threaded: %+v", tt.ref, ss) + } + } +} + func TestParseSourceRef_PRShortFormStillWins(t *testing.T) { // A '#' short-form must remain a PR source even though it contains no rev. src, err := ParseSourceRef("github:owner/repo#123", SnapshotOptions{}, testLogger()) @@ -111,3 +143,29 @@ func TestInferProviderFromURL(t *testing.T) { } } } + +func TestMatchGlob(t *testing.T) { + tests := []struct { + pattern string + path string + want bool + }{ + {"services/payments/**", "services/payments/deploy.yaml", true}, + {"services/payments/**", "services/payments/sub/dir/file.yaml", true}, + {"services/payments/**", "services/billing/deploy.yaml", false}, + {"**/*.yaml", "a/b/c.yaml", true}, + {"**/*.yaml", "top.yaml", true}, + {"**/*.yaml", "a/b/c.json", false}, + {"*.yaml", "top.yaml", true}, + {"*.yaml", "a/top.yaml", false}, + // No wildcards → directory prefix convenience. + {"services/payments", "services/payments/deploy.yaml", true}, + {"services/payments", "services/payments", true}, + {"services/payments", "services/payments-v2/deploy.yaml", false}, + } + for _, tt := range tests { + if got := matchGlob(tt.pattern, tt.path); got != tt.want { + t.Errorf("matchGlob(%q, %q) = %v, want %v", tt.pattern, tt.path, got, tt.want) + } + } +} diff --git a/pkg/generate/source_snapshot.go b/pkg/generate/source_snapshot.go new file mode 100644 index 0000000..dd1cf1a --- /dev/null +++ b/pkg/generate/source_snapshot.go @@ -0,0 +1,207 @@ +package generate + +import ( + "context" + "fmt" + "io/fs" + "log/slog" + "os" + "path" + "path/filepath" + "strings" +) + +// SnapshotSource captures the current state of files in a local checkout. +// Without Base every matched file becomes an added file (a stamp-out module); +// with Base the working tree is diffed against that git ref (a transform +// module), so modified YAML still produces SMP patches. +type SnapshotSource struct { + Path string + Include []string + Exclude []string + Base string +} + +// Fetch implements ChangeSource. +func (s *SnapshotSource) Fetch(ctx context.Context, _ string, _ string, logger *slog.Logger) (*ChangeSet, error) { + if len(s.Include) == 0 { + return nil, fmt.Errorf("snapshot source %q requires at least one --include", s.Path) + } + info, err := os.Stat(s.Path) + if err != nil || !info.IsDir() { + return nil, fmt.Errorf("snapshot source %q is not a directory", s.Path) + } + + isGit := false + if hasBinary("git", logger) { + if _, err := gitOut(ctx, s.Path, "rev-parse", "--git-dir"); err == nil { + isGit = true + } + } + + var files []FileChange + if s.Base == "" { + files, err = s.walkFiles(logger) + } else { + if !isGit { + return nil, fmt.Errorf("--base requires %s to be a git repository", s.Path) + } + files, err = s.diffAgainstBase(ctx, logger) + } + if err != nil { + return nil, err + } + + cs := &ChangeSet{Files: files} + if isGit { + if out, err := gitOut(ctx, s.Path, "remote", "get-url", "origin"); err == nil { + cs.RepoURL = strings.TrimSpace(string(out)) + cs.Provider = inferProviderFromURL(cs.RepoURL) + } + cs.BaseBranch = s.baseBranch(ctx) + } + return cs, nil +} + +// walkFiles snapshots every matched file in the working tree as added. +func (s *SnapshotSource) walkFiles(logger *slog.Logger) ([]FileChange, error) { + var files []FileChange + err := filepath.WalkDir(s.Path, func(p string, d fs.DirEntry, err error) error { + if err != nil { + return err + } + rel, relErr := filepath.Rel(s.Path, p) + if relErr != nil { + return relErr + } + rel = filepath.ToSlash(rel) + if d.IsDir() { + if d.Name() == ".git" { + return filepath.SkipDir + } + return nil + } + if !s.matches(rel) { + return nil + } + content, readErr := os.ReadFile(p) + if readErr != nil { + logger.Warn("cannot read file; skipped", "file", rel, "error", readErr) + return nil + } + files = append(files, FileChange{Type: ChangeAdded, Path: rel, NewContent: content}) + return nil + }) + return files, err +} + +// diffAgainstBase diffs the working tree (including uncommitted and untracked +// files) against the base ref and keeps the matched entries. +func (s *SnapshotSource) diffAgainstBase(ctx context.Context, logger *slog.Logger) ([]FileChange, error) { + // git reports diff paths relative to the repository root, so the + // snapshot path must be the root for includes and reads to line up. + if top, err := gitOut(ctx, s.Path, "rev-parse", "--show-toplevel"); err == nil { + topPath, _ := filepath.EvalSymlinks(strings.TrimSpace(string(top))) + absPath, _ := filepath.Abs(s.Path) + absPath, _ = filepath.EvalSymlinks(absPath) + if topPath != absPath { + return nil, fmt.Errorf("snapshot path %s must be the repository root (%s) when using --base", s.Path, topPath) + } + } + + if _, err := gitOut(ctx, s.Path, "rev-parse", "--verify", "--quiet", s.Base+"^{commit}"); err != nil { + return nil, fmt.Errorf("cannot resolve base ref %q in %s: %w", s.Base, s.Path, err) + } + + all, err := gitDiffFiles(ctx, s.Path, s.Base, "", logger) + if err != nil { + return nil, err + } + var files []FileChange + for _, f := range all { + if s.matches(f.Path) { + files = append(files, f) + } + } + + // git diff does not report untracked files — capture them too; + // they are part of the current state. + out, err := gitOut(ctx, s.Path, "ls-files", "--others", "--exclude-standard", "-z") + if err != nil { + return nil, err + } + for _, rel := range strings.Split(strings.TrimSuffix(string(out), "\x00"), "\x00") { + if rel == "" || !s.matches(rel) { + continue + } + content, readErr := os.ReadFile(filepath.Join(s.Path, rel)) + if readErr != nil { + logger.Warn("cannot read untracked file; skipped", "file", rel, "error", readErr) + continue + } + files = append(files, FileChange{Type: ChangeAdded, Path: rel, NewContent: content}) + } + return files, nil +} + +func (s *SnapshotSource) baseBranch(ctx context.Context) string { + if s.Base != "" { + if _, err := gitOut(ctx, s.Path, "show-ref", "--verify", "--quiet", "refs/heads/"+s.Base); err == nil { + return s.Base + } + } + if out, err := gitOut(ctx, s.Path, "rev-parse", "--abbrev-ref", "HEAD"); err == nil { + if branch := strings.TrimSpace(string(out)); branch != "HEAD" { + return branch + } + } + return "" +} + +func (s *SnapshotSource) matches(rel string) bool { + if !matchAnyGlob(s.Include, rel) { + return false + } + return !matchAnyGlob(s.Exclude, rel) +} + +func matchAnyGlob(patterns []string, rel string) bool { + for _, p := range patterns { + if matchGlob(p, rel) { + return true + } + } + return false +} + +// matchGlob matches a slash-separated glob against a relative path, with ** +// matching any number of path segments. A pattern without wildcards also +// matches everything under it when it names a directory prefix. +func matchGlob(pattern, rel string) bool { + pattern = strings.Trim(path.Clean(pattern), "/") + if !strings.ContainsAny(pattern, "*?[") { + return rel == pattern || strings.HasPrefix(rel, pattern+"/") + } + return matchSegments(strings.Split(pattern, "/"), strings.Split(rel, "/")) +} + +func matchSegments(pat, segs []string) bool { + if len(pat) == 0 { + return len(segs) == 0 + } + if pat[0] == "**" { + for i := 0; i <= len(segs); i++ { + if matchSegments(pat[1:], segs[i:]) { + return true + } + } + return false + } + if len(segs) == 0 { + return false + } + if ok, _ := path.Match(pat[0], segs[0]); !ok { + return false + } + return matchSegments(pat[1:], segs[1:]) +} From 2d12ffb297ca5481e1ed7e20d7a6a3a6dc645dc2 Mon Sep 17 00:00:00 2001 From: Rick Liu Date: Thu, 16 Jul 2026 14:11:56 +0100 Subject: [PATCH 5/8] docs(spec): mark generate-sources implemented, align details Single-commit parent resolution relies on full history in the bare partial clone rather than a depth-2 fetch, and the error table gains the no-changes and repo-root-with-base conditions. Co-Authored-By: Claude Fable 5 --- specs/generate-sources.md | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/specs/generate-sources.md b/specs/generate-sources.md index f310a52..c06da57 100644 --- a/specs/generate-sources.md +++ b/specs/generate-sources.md @@ -1,6 +1,6 @@ # Generate Sources — Design Proposal (Draft) -Status: **draft / not implemented**. Extends [`specs/generate.md`](generate.md). +Status: **implemented**. Extends [`specs/generate.md`](generate.md). ## Problem @@ -102,8 +102,8 @@ helpers) instead of API tokens. `--token-env` does not apply to commit sources. if the server refuses, fall back to a full fetch with a warning. Servers without partial-clone support fall back to a plain bare clone. 3. **Single `sha`** is treated as range `sha^...sha` (first parent for merge - commits — the diff is "what this merge brought in"); the parent is covered - by fetching with `--depth=2`. + commits — the diff is "what this merge brought in"). The parent is always + available: the bare partial clone filters blobs, not history. Builds on `pkg/git`'s existing go-git-with-CLI-fallback pattern. @@ -259,8 +259,10 @@ git-native and use the user's git credentials. | `--include`/`--base` without a snapshot ref | `--include/--base require a local path source` | | Snapshot path not a git repo with `--base` | `--base requires to be a git repository` | | Snapshot-only sources without `-n` | `--name is required when generating from local files` | -| Bad commit ref | `cannot resolve commit "" in : ...` | +| Bad commit ref | `cannot resolve commit "": ...` | | Clone/fetch failure (auth, unreachable host) | `fetching : ...` (surfaces the underlying git error) | +| A source has no file changes | `source "" has no file changes` | +| Snapshot path is not the repo root with `--base` | `snapshot path must be the repository root () when using --base` | ## Implementation phases From 673fdea8d313ca683be60122fb6a7531d1963ba2 Mon Sep 17 00:00:00 2001 From: Rick Liu Date: Tue, 21 Jul 2026 23:08:29 +0100 Subject: [PATCH 6/8] feat(generate): allow at most one snapshot source per invocation --include/--exclude/--base are invocation-wide, so multiple snapshot refs would silently share them; same-repo composition (CS3) makes multiple snapshots redundant anyway. Error out instead of guessing. Co-Authored-By: Claude Fable 5 --- pkg/generate/generate.go | 14 ++++++++++---- pkg/generate/generate_test.go | 18 +++++++++++++++++- specs/generate-sources.md | 3 ++- 3 files changed, 29 insertions(+), 6 deletions(-) diff --git a/pkg/generate/generate.go b/pkg/generate/generate.go index 71c8efd..c1d4fd7 100644 --- a/pkg/generate/generate.go +++ b/pkg/generate/generate.go @@ -45,7 +45,7 @@ func Run(ctx context.Context, opts Options, logger *slog.Logger) error { // 1. Parse all references. snap := SnapshotOptions{Include: opts.Include, Exclude: opts.Exclude, Base: opts.Base} sources := make([]*Source, 0, len(opts.Refs)) - hasSnapshot := false + snapshotCount := 0 allSnapshots := true for _, ref := range opts.Refs { src, err := ParseSourceRef(ref, snap, logger) @@ -53,14 +53,20 @@ func Run(ctx context.Context, opts Options, logger *slog.Logger) error { return err } if src.Kind == KindSnapshot { - hasSnapshot = true + snapshotCount++ } else { allSnapshots = false } sources = append(sources, src) } - if !hasSnapshot && (len(opts.Include) > 0 || len(opts.Exclude) > 0 || opts.Base != "") { - return fmt.Errorf("--include/--exclude/--base require a local path source") + if snapshotCount == 0 && (len(opts.Include) > 0 || len(opts.Exclude) > 0 || opts.Base != "") { + return fmt.Errorf("--include/--exclude/--base require a snapshot source") + } + // --include/--exclude/--base are invocation-wide, so several snapshot + // sources would silently share them — and same-repo composition makes + // multiple snapshots redundant anyway (CS3). + if snapshotCount > 1 { + return fmt.Errorf("at most one snapshot source per invocation; combine --include globs instead") } // 2. Fetch each source's changes, in the order given. diff --git a/pkg/generate/generate_test.go b/pkg/generate/generate_test.go index 0cb6b57..85df0f0 100644 --- a/pkg/generate/generate_test.go +++ b/pkg/generate/generate_test.go @@ -1560,11 +1560,27 @@ func TestRun_SnapshotFlagsRequireSnapshotSource(t *testing.T) { Include: []string{"a/**"}, } err := Run(t.Context(), opts, testLogger()) - if err == nil || !strings.Contains(err.Error(), "require a local path source") { + if err == nil || !strings.Contains(err.Error(), "require a snapshot source") { t.Errorf("expected flag-validation error, got %v", err) } } +func TestRun_AtMostOneSnapshotSource(t *testing.T) { + dirA, dirB := t.TempDir(), t.TempDir() + mustWrite(t, dirA, "a/f.yaml", "x: 1\n") + mustWrite(t, dirB, "a/f.yaml", "x: 2\n") + + opts := Options{ + Refs: []string{dirA, dirB}, + Include: []string{"a/**"}, + ModuleName: "two-snaps", + } + err := Run(t.Context(), opts, testLogger()) + if err == nil || !strings.Contains(err.Error(), "at most one snapshot source") { + t.Errorf("expected single-snapshot guard error, got %v", err) + } +} + func testLogger() *slog.Logger { return slog.New(slog.NewTextHandler(os.Stderr, &slog.HandlerOptions{Level: slog.LevelWarn})) } diff --git a/specs/generate-sources.md b/specs/generate-sources.md index c06da57..269d18f 100644 --- a/specs/generate-sources.md +++ b/specs/generate-sources.md @@ -256,7 +256,8 @@ git-native and use the user's git credentials. | Sources resolve to different repos | `sources reference different repositories: vs ` | | Net changeset empty | `no net file changes after composing sources` | | Snapshot ref without `--include` | `snapshot source requires at least one --include` | -| `--include`/`--base` without a snapshot ref | `--include/--base require a local path source` | +| `--include`/`--base` without a snapshot ref | `--include/--exclude/--base require a snapshot source` | +| More than one snapshot ref | `at most one snapshot source per invocation; combine --include globs instead` | | Snapshot path not a git repo with `--base` | `--base requires to be a git repository` | | Snapshot-only sources without `-n` | `--name is required when generating from local files` | | Bad commit ref | `cannot resolve commit "": ...` | From 4cb700c40be17e575fdeb2650dbc69e284f18fca Mon Sep 17 00:00:00 2001 From: Rick Liu Date: Tue, 21 Jul 2026 23:21:18 +0100 Subject: [PATCH 7/8] feat(generate): remote snapshot source MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Capture the committed tree of a repository without a local checkout via the canonical snapshot:[@] form — git-native like the commit source (bare partial clone, user's git credentials, any host). A bare local path stays sugar for the working-tree snapshot; @ pins the committed tree at a tag/branch/SHA for both local and remote forms, listed via git ls-tree with symlinks and submodules skipped. Ranges are rejected: snapshots capture one state. Co-Authored-By: Claude Fable 5 --- cmd/generate.go | 13 ++- pkg/generate/provider.go | 34 ++++++ pkg/generate/source_git_test.go | 91 +++++++++++++++ pkg/generate/source_parse_test.go | 50 ++++++++ pkg/generate/source_snapshot.go | 188 +++++++++++++++++++++++++++--- specs/generate-sources.md | 79 +++++++++---- 6 files changed, 410 insertions(+), 45 deletions(-) diff --git a/cmd/generate.go b/cmd/generate.go index e1b3410..c4f4d84 100644 --- a/cmd/generate.go +++ b/cmd/generate.go @@ -39,8 +39,11 @@ Supported references: git@bitbucket.org:owner/repo.git@abc1234...def5678 https://github.com/owner/repo/commit/abc1234 ./checkout@abc1234...def5678 - Current state of local files (with --include, optionally --base): - ./checkout /abs/path file:relative/path + Snapshot of files (with --include, optionally --base): + ./checkout /abs/path file:relative/path (local working tree) + snapshot:github:owner/repo@v1.2.3 (remote, committed tree at ref) + snapshot:git@host:owner/repo.git (remote, default branch) + snapshot:./checkout@release-2024 (local, committed tree at ref) By default, files are generated into the current directory. Use -o to specify a different output directory.`, @@ -53,9 +56,9 @@ func init() { generateCmd.Flags().StringVarP(&genOutput, "output", "o", "", "Output directory (default: current directory)") generateCmd.Flags().StringVarP(&genName, "name", "n", "", "Module name (default: derived from PR title or commit subject)") generateCmd.Flags().StringVar(&genTokenEnv, "token-env", "", "Env var holding the API token for PR/MR sources (default: GITHUB_TOKEN or GITLAB_TOKEN)") - generateCmd.Flags().StringArrayVar(&genInclude, "include", nil, "Glob of files to capture from a local path source (can be repeated; ** matches directories)") - generateCmd.Flags().StringArrayVar(&genExclude, "exclude", nil, "Glob of files to skip from a local path source (can be repeated)") - generateCmd.Flags().StringVar(&genBase, "base", "", "Git ref to diff a local path source against (default: capture files as-is)") + generateCmd.Flags().StringArrayVar(&genInclude, "include", nil, "Glob of files to capture from a snapshot source (can be repeated; ** matches directories)") + generateCmd.Flags().StringArrayVar(&genExclude, "exclude", nil, "Glob of files to skip from a snapshot source (can be repeated)") + generateCmd.Flags().StringVar(&genBase, "base", "", "Git ref to diff a snapshot source against (default: capture files as-is)") rootCmd.AddCommand(generateCmd) } diff --git a/pkg/generate/provider.go b/pkg/generate/provider.go index 724e857..9b950b0 100644 --- a/pkg/generate/provider.go +++ b/pkg/generate/provider.go @@ -96,6 +96,12 @@ var ( func ParseSourceRef(ref string, snap SnapshotOptions, logger *slog.Logger) (*Source, error) { logger.Debug("parsing source reference", "ref", ref) + // Explicit snapshot references — canonical form for both remote repos + // and local paths; a trailing @ pins the committed tree at that ref. + if body, ok := strings.CutPrefix(ref, "snapshot:"); ok { + return parseSnapshotRef(body, snap, logger) + } + // Local paths — snapshot, or commit source when an @ suffix is // present. Only hex SHAs are accepted as rev suffixes on paths so that // directory names containing '@' are not misclassified. @@ -168,6 +174,34 @@ func ParseSourceRef(ref string, snap SnapshotOptions, logger *slog.Logger) (*Sou return nil, fmt.Errorf("cannot detect source kind from reference %q; use a PR/MR URL, a @ commit reference, a local path, or prefix with github: or gitlab:", ref) } +// parseSnapshotRef parses the body of a snapshot:[@] +// reference. Because the snapshot: prefix is explicit, tag and branch names +// are accepted as refs (unlike bare local paths, which require hex SHAs). +func parseSnapshotRef(body string, snap SnapshotOptions, logger *slog.Logger) (*Source, error) { + repo, refName := body, "" + if r, base, head, ok := splitRevSuffix(body, true); ok { + if base != "" { + return nil, fmt.Errorf("snapshot references take a single ref, not a range: %q", body) + } + repo, refName = r, head + } + if r, ok := strings.CutPrefix(repo, "github:"); ok { + repo = fmt.Sprintf("git@github.com:%s.git", r) + } else if r, ok := strings.CutPrefix(repo, "gitlab:"); ok { + repo = fmt.Sprintf("git@gitlab.com:%s.git", r) + } + + src := &SnapshotSource{Ref: refName, Include: snap.Include, Exclude: snap.Exclude, Base: snap.Base} + if looksLikeGitURL(repo) { + logger.Debug("detected remote snapshot reference", "repo", repo, "ref", refName) + src.RepoURL = repo + return &Source{Kind: KindSnapshot, Provider: inferProviderFromURL(repo), ChangeSource: src}, nil + } + logger.Debug("detected local snapshot reference", "path", repo, "ref", refName) + src.Path = repo + return &Source{Kind: KindSnapshot, ChangeSource: src}, nil +} + func parseLocalRef(path string, snap SnapshotOptions, logger *slog.Logger) (*Source, error) { if repo, base, head, ok := splitRevSuffix(path, false); ok { logger.Debug("detected local commit reference", "path", repo) diff --git a/pkg/generate/source_git_test.go b/pkg/generate/source_git_test.go index f06102d..c432176 100644 --- a/pkg/generate/source_git_test.go +++ b/pkg/generate/source_git_test.go @@ -291,6 +291,97 @@ func TestSnapshotSource_WithBase_DiffsWorkingTree(t *testing.T) { } } +// Remote snapshots see only committed state — worktree edits in the source +// repo must not leak into the captured files. +func TestSnapshotSource_Remote_CapturesCommittedTree(t *testing.T) { + r := newFixtureRepo(t) + r.write("config/app.yaml", "replicas: 1\n") + r.write("docs/readme.md", "hi\n") + r.commit("initial") + // Uncommitted change that must NOT appear in the snapshot. + r.write("config/app.yaml", "replicas: 99\n") + + src := &SnapshotSource{RepoURL: "file://" + r.dir, Include: []string{"config/**"}} + cs, err := src.Fetch(context.Background(), "", "", testLogger()) + if err != nil { + t.Fatal(err) + } + + if len(cs.Files) != 1 { + t.Fatalf("expected 1 file, got %+v", cs.Files) + } + f := cs.Files[0] + if f.Type != ChangeAdded || f.Path != "config/app.yaml" { + t.Errorf("unexpected file: %+v", f) + } + if string(f.NewContent) != "replicas: 1\n" { + t.Errorf("expected committed content, got %q", f.NewContent) + } + if cs.RepoURL != "file://"+r.dir || cs.BaseBranch != "main" { + t.Errorf("metadata: repo=%q branch=%q", cs.RepoURL, cs.BaseBranch) + } +} + +func TestSnapshotSource_Remote_AtRef(t *testing.T) { + r := newFixtureRepo(t) + c1, _, _ := seedHistory(r) + + src := &SnapshotSource{RepoURL: "file://" + r.dir, Ref: c1, Include: []string{"config/**"}} + cs, err := src.Fetch(context.Background(), "", "", testLogger()) + if err != nil { + t.Fatal(err) + } + + // At c1 only config/base.yaml exists, with the original content. + if len(cs.Files) != 1 { + t.Fatalf("expected 1 file at c1, got %+v", cs.Files) + } + if !strings.Contains(string(cs.Files[0].NewContent), "replicas: 1") { + t.Errorf("expected content at c1, got %q", cs.Files[0].NewContent) + } +} + +func TestSnapshotSource_Remote_WithBase(t *testing.T) { + r := newFixtureRepo(t) + c1, c2, _ := seedHistory(r) + + src := &SnapshotSource{RepoURL: "file://" + r.dir, Ref: c2, Base: c1, Include: []string{"config/**"}} + cs, err := src.Fetch(context.Background(), "", "", testLogger()) + if err != nil { + t.Fatal(err) + } + + // c1→c2 touched config/base.yaml (modified) and config/new.yaml (added); + // app.json is outside the include globs. + if len(cs.Files) != 2 { + t.Fatalf("expected 2 files, got %+v", cs.Files) + } + if f := fileByPath(t, cs.Files, "config/base.yaml"); f.Type != ChangeModified { + t.Errorf("base.yaml type = %v, want modified", f.Type) + } + if f := fileByPath(t, cs.Files, "config/new.yaml"); f.Type != ChangeAdded { + t.Errorf("new.yaml type = %v, want added", f.Type) + } +} + +// A local snapshot pinned to a ref captures the committed tree, not the +// working tree — same semantics as the remote form. +func TestSnapshotSource_LocalAtRef(t *testing.T) { + r := newFixtureRepo(t) + r.write("config/app.yaml", "replicas: 1\n") + c1 := r.commit("initial") + r.write("config/app.yaml", "replicas: 99\n") // uncommitted + + src := &SnapshotSource{Path: r.dir, Ref: c1, Include: []string{"config/**"}} + cs, err := src.Fetch(context.Background(), "", "", testLogger()) + if err != nil { + t.Fatal(err) + } + if len(cs.Files) != 1 || string(cs.Files[0].NewContent) != "replicas: 1\n" { + t.Errorf("expected committed content at ref, got %+v", cs.Files) + } +} + func TestSnapshotSource_RequiresInclude(t *testing.T) { src := &SnapshotSource{Path: t.TempDir()} _, err := src.Fetch(context.Background(), "", "", testLogger()) diff --git a/pkg/generate/source_parse_test.go b/pkg/generate/source_parse_test.go index 205dcee..0501ac9 100644 --- a/pkg/generate/source_parse_test.go +++ b/pkg/generate/source_parse_test.go @@ -1,6 +1,7 @@ package generate import ( + "strings" "testing" ) @@ -101,6 +102,55 @@ func TestParseSourceRef_SnapshotRefs(t *testing.T) { } } +func TestParseSourceRef_SnapshotPrefix(t *testing.T) { + tests := []struct { + ref string + repoURL string + path string + refName string + provider string + }{ + // Remote: short-form sugar, git URLs, with and without a ref. + {"snapshot:github:o/r@v1.2.3", "git@github.com:o/r.git", "", "v1.2.3", "github"}, + {"snapshot:gitlab:g/r", "git@gitlab.com:g/r.git", "", "", "gitlab"}, + {"snapshot:git@bitbucket.org:o/r.git", "git@bitbucket.org:o/r.git", "", "", ""}, + {"snapshot:https://gitea.example.com/o/r.git@abc1234", "https://gitea.example.com/o/r.git", "", "abc1234", ""}, + // Local: committed tree at a ref; tags/branches allowed under the + // explicit prefix. + {"snapshot:./checkout@release-2024", "", "./checkout", "release-2024", ""}, + {"snapshot:./checkout", "", "./checkout", "", ""}, + } + for _, tt := range tests { + src, err := ParseSourceRef(tt.ref, SnapshotOptions{Include: []string{"**"}}, testLogger()) + if err != nil { + t.Errorf("ParseSourceRef(%q): %v", tt.ref, err) + continue + } + if src.Kind != KindSnapshot { + t.Errorf("%q: kind = %v, want snapshot", tt.ref, src.Kind) + continue + } + ss := src.ChangeSource.(*SnapshotSource) + if ss.RepoURL != tt.repoURL || ss.Path != tt.path || ss.Ref != tt.refName { + t.Errorf("%q: repo=%q path=%q ref=%q, want repo=%q path=%q ref=%q", + tt.ref, ss.RepoURL, ss.Path, ss.Ref, tt.repoURL, tt.path, tt.refName) + } + if src.Provider != tt.provider { + t.Errorf("%q: provider = %q, want %q", tt.ref, src.Provider, tt.provider) + } + if len(ss.Include) != 1 { + t.Errorf("%q: snapshot options not threaded", tt.ref) + } + } +} + +func TestParseSourceRef_SnapshotPrefixRejectsRange(t *testing.T) { + _, err := ParseSourceRef("snapshot:github:o/r@abc1234...def5678", SnapshotOptions{}, testLogger()) + if err == nil || !strings.Contains(err.Error(), "single ref, not a range") { + t.Errorf("expected range-rejection error, got %v", err) + } +} + func TestParseSourceRef_PRShortFormStillWins(t *testing.T) { // A '#' short-form must remain a PR source even though it contains no rev. src, err := ParseSourceRef("github:owner/repo#123", SnapshotOptions{}, testLogger()) diff --git a/pkg/generate/source_snapshot.go b/pkg/generate/source_snapshot.go index dd1cf1a..e8a1f5e 100644 --- a/pkg/generate/source_snapshot.go +++ b/pkg/generate/source_snapshot.go @@ -11,12 +11,21 @@ import ( "strings" ) -// SnapshotSource captures the current state of files in a local checkout. +// SnapshotSource captures the state of files in a repository. The source is +// either a local checkout (Path) or a remote repository (RepoURL, cloned +// bare with lazy blobs). +// +// Without Ref, a local snapshot captures the working tree — including +// uncommitted and untracked files. With Ref (always for remotes, where no +// working tree exists), it captures the committed tree at that ref. +// // Without Base every matched file becomes an added file (a stamp-out module); -// with Base the working tree is diffed against that git ref (a transform +// with Base the captured state is diffed against that git ref (a transform // module), so modified YAML still produces SMP patches. type SnapshotSource struct { - Path string + Path string // local checkout; mutually exclusive with RepoURL + RepoURL string // remote repository URL + Ref string // committed ref to capture; empty = worktree (local) or default branch (remote) Include []string Exclude []string Base string @@ -25,7 +34,14 @@ type SnapshotSource struct { // Fetch implements ChangeSource. func (s *SnapshotSource) Fetch(ctx context.Context, _ string, _ string, logger *slog.Logger) (*ChangeSet, error) { if len(s.Include) == 0 { - return nil, fmt.Errorf("snapshot source %q requires at least one --include", s.Path) + name := s.Path + if name == "" { + name = s.RepoURL + } + return nil, fmt.Errorf("snapshot source %q requires at least one --include", name) + } + if s.RepoURL != "" { + return s.fetchRemote(ctx, logger) } info, err := os.Stat(s.Path) if err != nil || !info.IsDir() { @@ -39,6 +55,23 @@ func (s *SnapshotSource) Fetch(ctx context.Context, _ string, _ string, logger * } } + if s.Ref != "" { + // Committed tree at a ref — same semantics as a remote snapshot, + // minus the clone. + if !isGit { + return nil, fmt.Errorf("snapshot ref %q requires %s to be a git repository", s.Ref, s.Path) + } + if err := s.requireRepoRoot(ctx); err != nil { + return nil, err + } + cs, err := s.snapshotAtRef(ctx, s.Path, logger) + if err != nil { + return nil, err + } + s.addOriginMetadata(ctx, cs) + return cs, nil + } + var files []FileChange if s.Base == "" { files, err = s.walkFiles(logger) @@ -54,15 +87,127 @@ func (s *SnapshotSource) Fetch(ctx context.Context, _ string, _ string, logger * cs := &ChangeSet{Files: files} if isGit { - if out, err := gitOut(ctx, s.Path, "remote", "get-url", "origin"); err == nil { - cs.RepoURL = strings.TrimSpace(string(out)) - cs.Provider = inferProviderFromURL(cs.RepoURL) - } + s.addOriginMetadata(ctx, cs) cs.BaseBranch = s.baseBranch(ctx) } return cs, nil } +// fetchRemote clones the repository bare (blobs fetched lazily) and captures +// the committed tree at Ref, or its diff against Base. +func (s *SnapshotSource) fetchRemote(ctx context.Context, logger *slog.Logger) (*ChangeSet, error) { + if !hasBinary("git", logger) { + return nil, fmt.Errorf("git CLI is required for remote snapshot sources") + } + tmp, err := os.MkdirTemp("", "loom-snapshot-*") + if err != nil { + return nil, err + } + defer os.RemoveAll(tmp) + if err := cloneBare(ctx, s.RepoURL, tmp, logger); err != nil { + return nil, fmt.Errorf("fetching %s: %w", s.RepoURL, err) + } + cs, err := s.snapshotAtRef(ctx, tmp, logger) + if err != nil { + return nil, err + } + cs.RepoURL = s.RepoURL + cs.Provider = inferProviderFromURL(s.RepoURL) + return cs, nil +} + +// snapshotAtRef captures the committed state at Ref (HEAD when empty) in the +// given git dir: the full matched tree, or its diff against Base. +func (s *SnapshotSource) snapshotAtRef(ctx context.Context, dir string, logger *slog.Logger) (*ChangeSet, error) { + ref := s.Ref + if ref == "" { + ref = "HEAD" + } else if err := ensureRev(ctx, dir, ref, logger); err != nil { + return nil, err + } + + var files []FileChange + if s.Base != "" { + if err := ensureRev(ctx, dir, s.Base, logger); err != nil { + return nil, err + } + all, err := gitDiffFiles(ctx, dir, s.Base, ref, logger) + if err != nil { + return nil, err + } + for _, f := range all { + if s.matches(f.Path) { + files = append(files, f) + } + } + } else { + var err error + files, err = s.lsTreeFiles(ctx, dir, ref, logger) + if err != nil { + return nil, err + } + } + + cs := &ChangeSet{Files: files} + for _, cand := range []string{s.Base, s.Ref} { + if cand == "" { + continue + } + if _, err := gitOut(ctx, dir, "show-ref", "--verify", "--quiet", "refs/heads/"+cand); err == nil { + cs.BaseBranch = cand + break + } + } + if cs.BaseBranch == "" { + cs.BaseBranch = defaultBranch(ctx, dir, logger) + } + return cs, nil +} + +// lsTreeFiles captures every matched regular file in the tree at ref. +func (s *SnapshotSource) lsTreeFiles(ctx context.Context, dir, ref string, logger *slog.Logger) ([]FileChange, error) { + out, err := gitOut(ctx, dir, "ls-tree", "-r", "-z", ref) + if err != nil { + return nil, err + } + var files []FileChange + for _, entry := range strings.Split(strings.TrimSuffix(string(out), "\x00"), "\x00") { + meta, rel, ok := strings.Cut(entry, "\t") + if !ok { + continue + } + fields := strings.Fields(meta) // + if len(fields) < 2 { + continue + } + if !s.matches(rel) { + continue + } + // Only regular files: skip symlinks (mode 120000) and submodules + // (type commit) — neither can be reproduced as a template. + if fields[1] != "blob" || fields[0] == "120000" { + logger.Warn("skipping non-regular file in snapshot", "file", rel, "mode", fields[0], "type", fields[1]) + continue + } + content, err := gitOut(ctx, dir, "show", ref+":"+rel) + if err != nil { + logger.Warn("cannot read file at ref; skipped", "file", rel, "error", err) + continue + } + files = append(files, FileChange{Type: ChangeAdded, Path: rel, NewContent: content}) + } + return files, nil +} + +// addOriginMetadata fills repo URL and provider from the local checkout's +// origin remote, when it has one. +func (s *SnapshotSource) addOriginMetadata(ctx context.Context, cs *ChangeSet) { + if out, err := gitOut(ctx, s.Path, "remote", "get-url", "origin"); err == nil { + cs.RepoURL = strings.TrimSpace(string(out)) + cs.Provider = inferProviderFromURL(cs.RepoURL) + } +} + // walkFiles snapshots every matched file in the working tree as added. func (s *SnapshotSource) walkFiles(logger *slog.Logger) ([]FileChange, error) { var files []FileChange @@ -95,18 +240,27 @@ func (s *SnapshotSource) walkFiles(logger *slog.Logger) ([]FileChange, error) { return files, err } +// requireRepoRoot rejects snapshot paths below the repository root: git +// reports paths relative to the root, so includes and reads would not line up. +func (s *SnapshotSource) requireRepoRoot(ctx context.Context) error { + top, err := gitOut(ctx, s.Path, "rev-parse", "--show-toplevel") + if err != nil { + return nil + } + topPath, _ := filepath.EvalSymlinks(strings.TrimSpace(string(top))) + absPath, _ := filepath.Abs(s.Path) + absPath, _ = filepath.EvalSymlinks(absPath) + if topPath != absPath { + return fmt.Errorf("snapshot path %s must be the repository root (%s) when using --base or a ref", s.Path, topPath) + } + return nil +} + // diffAgainstBase diffs the working tree (including uncommitted and untracked // files) against the base ref and keeps the matched entries. func (s *SnapshotSource) diffAgainstBase(ctx context.Context, logger *slog.Logger) ([]FileChange, error) { - // git reports diff paths relative to the repository root, so the - // snapshot path must be the root for includes and reads to line up. - if top, err := gitOut(ctx, s.Path, "rev-parse", "--show-toplevel"); err == nil { - topPath, _ := filepath.EvalSymlinks(strings.TrimSpace(string(top))) - absPath, _ := filepath.Abs(s.Path) - absPath, _ = filepath.EvalSymlinks(absPath) - if topPath != absPath { - return nil, fmt.Errorf("snapshot path %s must be the repository root (%s) when using --base", s.Path, topPath) - } + if err := s.requireRepoRoot(ctx); err != nil { + return nil, err } if _, err := gitOut(ctx, s.Path, "rev-parse", "--verify", "--quiet", s.Base+"^{commit}"); err != nil { diff --git a/specs/generate-sources.md b/specs/generate-sources.md index 269d18f..9dfa50b 100644 --- a/specs/generate-sources.md +++ b/specs/generate-sources.md @@ -21,7 +21,7 @@ parameterization, emission — only consumes `PRInfo` (metadata + `[]FileChange` with old/new content). The design therefore: 1. Generalizes the source abstraction: `DiffProvider` → `ChangeSource`, `PRInfo` → `ChangeSet`. -2. Adds two new source kinds (commits, local snapshot). +2. Adds two new source kinds (commits, snapshots — local or remote). 3. Adds one new step: **composition** of multiple `ChangeSet`s into a single net `ChangeSet`, which then flows through the existing pipeline unchanged. @@ -59,7 +59,8 @@ and recognizes the grammars below. | `@...` | commit range | `github:o/r@abc1234...def5678` | | commit URL (sugar) | single commit | `https://github.com/o/r/commit/abc1234` | | compare URL (sugar) | commit range | `https://gitlab.com/g/r/-/compare/abc...def` | -| local path (`./…`, `/…`, or `file:` prefix) | snapshot | `./checkout-of-target-repo` | +| local path (`./…`, `/…`, or `file:` prefix) | snapshot (working tree) | `./checkout-of-target-repo` | +| `snapshot:[@]` | snapshot (committed tree; working tree for a local path without `@`) | `snapshot:github:o/r@v1.2.3` | `` in commit refs is any of: - a short-form repo (`github:o/r`, `gitlab:g/r`) — sugar that expands to the @@ -74,10 +75,16 @@ SHA (7–40 chars) or a tag name. Commit/compare URLs from known hosts are accepted as sugar and normalized to `@` — they are parsed, not fetched via API. -Detection order: `file:` prefix / path-like without `@rev` → snapshot; -trailing `@` → commit; short-form with `#`/`!` → PR (existing); then -PR/MR URL patterns. Malformed refs are rejected, never misclassified (same -philosophy as PD2). +Detection order: `snapshot:` prefix → snapshot; `file:` prefix / path-like +without `@rev` → snapshot; trailing `@` → commit; short-form with +`#`/`!` → PR (existing); then PR/MR URL patterns. Malformed refs are +rejected, never misclassified (same philosophy as PD2). + +`snapshot:` is the canonical snapshot form; a bare local path is sugar for +`snapshot:`. Under the explicit prefix, the `@` suffix accepts tag +and branch names (bare paths require hex SHAs, so directory names containing +`@` are not misclassified), and a range is rejected: snapshots capture one +state. ### 1. PR source (existing, unchanged) @@ -125,28 +132,44 @@ Builds on `pkg/git`'s existing go-git-with-CLI-fallback pattern. ### 3. Snapshot source (current state of files) -Points at a **local checkout** of the target repo. Selection and baseline come -from flags (snapshot refs get no inline grammar for these): +Captures the state of files in the target repo — from a **local checkout** +(bare path, or `snapshot:`) or a **remote repository** +(`snapshot:[@]`, git-native like the commit source: bare partial +clone, user's git credentials, any host). Selection and baseline come from +flags (snapshot refs carry only the optional `@` inline): - `--include ` (repeatable, required for snapshot refs): files to capture, - relative to the checkout root. `--exclude ` optional. + relative to the repository root. `--exclude ` optional. - `--base ` (optional): baseline to diff against. -Two modes: +At most **one snapshot ref per invocation**: the flags are invocation-wide, +and same-repo composition (CS3) makes multiple snapshots redundant. + +**What is captured:** + +- **Local path without `@`** — the **working tree**, including + uncommitted and untracked files. This is the only form that can see + uncommitted state. +- **`@` (local or remote), or a remote without `@`** — the + **committed tree** at that ref (default branch when omitted), listed via + `git ls-tree -r`. Symlinks and submodules are skipped with a warning. -- **No `--base`** — every matched file becomes `ChangeAdded` with its current - working-tree content. Use when the module should *stamp out* these files. -- **With `--base`** — run `git diff --name-status -- ` in the - checkout; old content via `git show :`, new content from the - working tree (captures uncommitted state too). Produces real - added/modified/deleted/renamed classification, so modified YAML becomes SMP - patches. Use when the module should *transform* an existing repo. +**Two modes** (orthogonal to the above): -Metadata: `RepoURL` from `git remote get-url origin` (empty if none); -`BaseBranch` = `--base` if it names a branch, else the current branch; -`Provider` inferred from the remote host (`github.com` → github, `gitlab` in -host → gitlab, else empty). `Title`/`Body` empty — `--name` is required when -the only sources are snapshots. +- **No `--base`** — every matched file becomes `ChangeAdded` with its + captured content. Use when the module should *stamp out* these files. +- **With `--base`** — the captured state is diffed against the base ref + (`git diff` against the working tree or between the two revs). Produces + real added/modified/deleted/renamed classification, so modified YAML + becomes SMP patches. Use when the module should *transform* an existing + repo. + +Metadata: `RepoURL` from the ref itself (remote) or +`git remote get-url origin` (local, empty if none); `BaseBranch` = `--base` +or the `@` if either names a branch, else the current/default branch; +`Provider` inferred from the repo host (`github` in host → github, `gitlab` +in host → gitlab, else empty). `Title`/`Body` empty — `--name` is required +when the only sources are snapshots. ## Composition @@ -242,6 +265,12 @@ loom generate ./gitops-checkout --include 'services/payments/**' -n onboard-paym # current state diffed against a baseline (transform module) loom generate ./gitops-checkout --base main --include 'argocd/**' -n update-argo ... + +# committed tree of a remote repo at a tag — no local checkout needed +loom generate 'snapshot:github:org/gitops@v1.2.3' --include 'services/payments/**' -n onboard-payments ... + +# remote snapshot diffed against a baseline +loom generate 'snapshot:git@bitbucket.org:org/gitops.git' --base v1.0.0 --include 'argocd/**' -n update-argo ... ``` New flags: `--include`, `--exclude`, `--base` (apply to snapshot refs; error if @@ -263,7 +292,9 @@ git-native and use the user's git credentials. | Bad commit ref | `cannot resolve commit "": ...` | | Clone/fetch failure (auth, unreachable host) | `fetching : ...` (surfaces the underlying git error) | | A source has no file changes | `source "" has no file changes` | -| Snapshot path is not the repo root with `--base` | `snapshot path must be the repository root () when using --base` | +| Snapshot path is not the repo root with `--base` or `@` | `snapshot path must be the repository root () when using --base or a ref` | +| Snapshot ref with a range | `snapshot references take a single ref, not a range: ""` | +| Local snapshot `@` outside a git repo | `snapshot ref "" requires to be a git repository` | ## Implementation phases @@ -275,5 +306,7 @@ git-native and use the user's git credentials. checkout), single-commit sugar, commit/compare URL normalization. 4. **Snapshot source**: local git integration, `--include/--exclude/--base`, target-less emission path. +5. **Remote snapshots**: `snapshot:` prefix, committed-tree capture via + `git ls-tree` (local `@` and remote repos), single-snapshot guard. Each phase is independently shippable and spec-testable. From 85fed09d493073f463929ab748232d3721281b6b Mon Sep 17 00:00:00 2001 From: Rick Liu Date: Tue, 21 Jul 2026 23:35:59 +0100 Subject: [PATCH 8/8] docs(generate): cover commit, snapshot, and multi-source generation Rewrite the generate guide and CLI reference for the new source kinds: git-native commit/range refs, local and remote snapshots with --include/--exclude/--base, source composition, and the degraded git-ops behavior for sources without PR metadata. Drop the documented but never-implemented --exclude-git-ops flag. Co-Authored-By: Claude Fable 5 --- docs/guide/generate.md | 103 +++++++++++++++++++++++++++------ docs/reference/cli-generate.md | 73 +++++++++++++++++++---- 2 files changed, 149 insertions(+), 27 deletions(-) diff --git a/docs/guide/generate.md b/docs/guide/generate.md index e3b6267..1ad924c 100644 --- a/docs/guide/generate.md +++ b/docs/guide/generate.md @@ -1,6 +1,6 @@ # Generate -`loom generate` reverse-engineers a reusable module from an existing GitHub PR or GitLab MR. This is the fastest way to turn a manual change you've already made into repeatable automation. +`loom generate` reverse-engineers a reusable module from changes that already exist — a GitHub PR or GitLab MR, specific commits, or the current state of files in a repository. This is the fastest way to turn a manual change you've already made into repeatable automation. ## How It Works @@ -12,7 +12,7 @@ loom generate https://github.com/myorg/gitops-repo/pull/42 \ -p namespace=fintech ``` -Loom fetches the PR diff and produces a ready-to-use module: +Loom fetches the changes and produces a ready-to-use module: - **Added files** become templates with {{ .paramName }} replacing the concrete values, including in file and folder names. - **Modified YAML files** become strategic merge patches under `__functions/patches/`. @@ -21,6 +21,76 @@ Loom fetches the PR diff and produces a ready-to-use module: The generated `loom.yaml` declares all parameters as required and wires up the operations in order. +## Sources + +Real changes don't always live in a single tidy PR. Generate accepts three kinds of sources, and they can be combined: + +### Pull Requests / Merge Requests + +```bash +loom generate https://github.com/myorg/gitops-repo/pull/42 ... +loom generate github:myorg/gitops-repo#42 ... +loom generate gitlab:mygroup/gitops-repo!42 ... +``` + +Fetched via the provider API (see [Authentication](#authentication)). PR title, body, and branches carry over into the generated `commitPush` and `pr` operations. + +### Commits and Commit Ranges + +Sometimes the change is a commit or a range of commits, not a PR. Commit sources are **git-native** — no provider API, no token. They work against any git host (GitHub, GitLab, Bitbucket, Gitea, bare SSH remotes) using your existing git credentials: + +```bash +# one commit +loom generate github:myorg/gitops-repo@9f3ab12 ... + +# a range: everything from base (exclusive) to head (inclusive) +loom generate 'github:myorg/gitops-repo@a1b2c3d...f6e5d4c' ... + +# any git host — the repo part is just a git URL +loom generate 'git@bitbucket.org:myorg/gitops-repo.git@a1b2c3d...f6e5d4c' ... + +# a checkout you already have (no clone) +loom generate './gitops-checkout@a1b2c3d...f6e5d4c' ... +``` + +Commit and compare URLs from GitHub/GitLab also work as shorthand (`https://github.com/o/r/commit/abc1234`). Remote repos are cloned bare with lazy blob fetching, so cost scales with the changed files, not the repository size. + +### Snapshots (current state of files) + +Sometimes there's no clean history at all — the files just *are* the desired state. A snapshot source captures files matched by `--include` globs: + +```bash +# working tree of a local checkout, including uncommitted files +loom generate ./gitops-checkout --include 'services/payments/**' -n onboard-payments ... + +# committed tree of a remote repo at a tag — no local checkout needed +loom generate 'snapshot:github:myorg/gitops-repo@v1.2.3' --include 'services/payments/**' -n onboard-payments ... + +# diff the current state against a baseline instead of capturing everything +loom generate ./gitops-checkout --base main --include 'argocd/**' -n update-argo ... +``` + +Two orthogonal choices: + +- **What is captured.** A bare local path captures the **working tree** — including uncommitted and untracked files (the only form that can). `snapshot:[@]` captures the **committed tree** at a ref (default branch if omitted), locally or remotely. +- **How it becomes a module.** Without `--base`, every matched file becomes a template — a *stamp-out* module. With `--base `, the captured state is diffed against that baseline, so modified YAML becomes strategic merge patches — a *transform* module. + +Snapshots have no title to derive a name from, so `-n` is required. One snapshot source per invocation; use repeated `--include` flags to capture multiple areas. + +## Combining Sources + +If the desired state took a few attempts to reach, pass all of them **oldest first** — Loom composes them into one net changeset: + +```bash +# the original PR plus two follow-up fixes +loom generate github:myorg/gitops#42 github:myorg/gitops#47 github:myorg/gitops#51 ... + +# a PR plus the fix commit that landed after it +loom generate github:myorg/gitops#42 github:myorg/gitops@9f3ab12 ... +``` + +Composition works per file: a file added in one PR and modified in the next becomes a single template with the final content; add-then-delete disappears; a PR and its revert cancel out (and error, since nothing is left). All sources must reference the same repository. Metadata (title, branches) comes from the last source that has it — the one closest to the desired state. + ## Example Given a PR that added these files for the "payments" service in the "fintech" namespace: @@ -60,20 +130,19 @@ loom run ./onboard-service -p serviceName=billing -p namespace=platform Same structure, different parameters. What took a PR now takes a command. -## Supported Providers +## Git Operations in the Generated Module -Both GitHub and GitLab are supported: +PR sources carry full metadata, so the module gets a `target`, a `commitPush`, and a `pr` operation. Sources with less metadata degrade gracefully: -``` -https://github.com/owner/repo/pull/123 -https://gitlab.com/group/repo/-/merge_requests/123 -github:owner/repo#123 -gitlab:group/repo!123 -``` +- No head branch (commits, snapshots) → the feature branch is synthesized as `loom/`. +- No recognizable provider (e.g. Bitbucket) → the `pr` operation is omitted with a warning; `commitPush` remains. +- No repository URL at all (a directory with no git remote) → `target` is omitted; fill it in before running the module. ## Authentication -Uses `GITHUB_TOKEN` / `GITLAB_TOKEN` environment variables by default. If the token is not set but the `gh` or `glab` CLI is installed and authenticated, Loom falls back to it automatically. +Only PR/MR sources talk to a provider API. They use `GITHUB_TOKEN` / `GITLAB_TOKEN` environment variables by default; if the token is not set but the `gh` or `glab` CLI is installed and authenticated, Loom falls back to it automatically. + +Commit and snapshot sources go through git directly and use whatever credentials git already has (SSH keys, credential helpers). ## Options @@ -81,14 +150,14 @@ Uses `GITHUB_TOKEN` / `GITLAB_TOKEN` environment variables by default. If the to |------|-------------| | `-p, --param key=value` | Concrete value to parameterize (repeatable) | | `-o, --output dir` | Output directory (default: current directory) | -| `-n, --name name` | Module name (default: derived from PR title) | -| `--token-env VAR` | Env var holding the API token | -| `--exclude-git-ops` | Skip generating `target`, `commitPush`, and `pr` operations | +| `-n, --name name` | Module name (default: derived from PR title or commit subject) | +| `--token-env VAR` | Env var holding the API token (PR/MR sources only) | +| `--include glob` | Files to capture from a snapshot source (repeatable; `**` matches directories) | +| `--exclude glob` | Files to skip from a snapshot source (repeatable) | +| `--base git-ref` | Baseline to diff a snapshot source against | By default, the generated module is written to the current directory. Use `-o` to write to a different directory: ```bash -loom generate -o ./my-module +loom generate -o ./my-module ``` - -Use `--exclude-git-ops` when you only want the template/patch operations and will handle git operations separately. diff --git a/docs/reference/cli-generate.md b/docs/reference/cli-generate.md index fcefacb..617a97c 100644 --- a/docs/reference/cli-generate.md +++ b/docs/reference/cli-generate.md @@ -1,9 +1,9 @@ # loom generate -Generate a reusable loom module from an existing GitHub PR or GitLab MR. This is the fastest way to turn a manual change you've already made into repeatable automation. +Generate a reusable loom module from existing changes: GitHub PRs / GitLab MRs, commits or commit ranges, or a snapshot of files in a repository. Multiple references compose into one net changeset. ``` -loom generate [flags] +loom generate [...] [flags] ``` ## Flags @@ -12,12 +12,16 @@ loom generate [flags] |------|-------------| | `-p, --param key=value` | Concrete value to parameterize (repeatable). Every occurrence of the literal value is replaced with a template expression. | | `-o, --output dir` | Output directory for the generated module (default: current directory) | -| `-n, --name name` | Module name (default: derived from PR title) | -| `--token-env VAR` | Env var holding the API token (default: `GITHUB_TOKEN` or `GITLAB_TOKEN`) | -| `--exclude-git-ops` | Skip generating `target`, `commitPush`, and `pr` operations | +| `-n, --name name` | Module name (default: derived from PR title or commit subject; required for snapshot-only sources) | +| `--token-env VAR` | Env var holding the API token for PR/MR sources (default: `GITHUB_TOKEN` or `GITLAB_TOKEN`) | +| `--include glob` | Files to capture from a snapshot source (repeatable; `**` matches any number of directories). Required with a snapshot ref. | +| `--exclude glob` | Files to skip from a snapshot source (repeatable) | +| `--base git-ref` | Baseline to diff a snapshot source against (default: capture matched files as-is) | ## Supported References +### PR / MR (provider API) + ``` https://github.com/owner/repo/pull/123 https://gitlab.com/group/repo/-/merge_requests/123 @@ -25,17 +29,66 @@ github:owner/repo#123 gitlab:group/repo!123 ``` +Self-hosted instances work with full URLs; use the `github:` / `gitlab:` prefix when the URL pattern is ambiguous. + +### Commit or commit range (git-native, any host) + +``` +github:owner/repo@abc1234 # one commit (diff vs. its first parent) +github:owner/repo@abc1234...def5678 # range: base (exclusive) to head (inclusive) +git@bitbucket.org:owner/repo.git@abc1234...def5678 # any git URL as the repo part +./checkout@abc1234...def5678 # local checkout, no clone +https://github.com/owner/repo/commit/abc1234 # commit URL shorthand +https://gitlab.com/group/repo/-/compare/a...b # compare URL shorthand +``` + +The rev suffix is split on the **last** `@` and must be a hex SHA (7–40 chars) or tag name, or a `...` range of them. No API or token involved: remote repos are cloned bare with lazy blob fetching and your git credentials. + +### Snapshot (state of files) + +``` +./checkout # working tree, incl. uncommitted/untracked files +/abs/path file:relative/path # same, alternative spellings +snapshot:github:owner/repo@v1.2.3 # committed tree of a remote repo at a ref +snapshot:git@host:owner/repo.git # committed tree at the default branch +snapshot:./checkout@release-2024 # committed tree of a local repo at a ref +``` + +Requires at least one `--include`. At most one snapshot ref per invocation. Without `--base` every matched file becomes a template; with `--base` the captured state is diffed against that ref, producing patches for modified YAML. Local paths with `@` (and `--base`) must point at the repository root. Symlinks and submodules are skipped with a warning. + +## Multiple Sources + +Pass references **oldest first**; they are composed in the order given into a single net changeset: + +```bash +loom generate github:org/gitops#42 github:org/gitops#47 github:org/gitops@9f3ab12 +``` + +- Per file: old content from the first source touching it, new content from the last. Add-then-delete drops out; delete-then-re-add nets to a modification; rename chains collapse. +- All sources must reference the same repository (HTTPS/SSH spellings are equivalent). +- Metadata (title, body, branches) comes from the last source that has it. +- If everything cancels out (e.g. a PR and its revert), generation fails with `no net file changes after composing sources`. + +## Generated Git Operations + +| Source metadata | Result | +|-----------------|--------| +| Full PR metadata | `target` + `commitPush` + `pr` operations, as before | +| No head branch (commit/snapshot) | Feature branch synthesized as `loom/` | +| No recognizable provider | `pr` operation omitted (warning); `commitPush` kept | +| No repository URL | `target` omitted (warning); fill in manually before running | + ## Output By default, the generated module is written to the current directory. Use `-o` to write to a different directory: ```bash -loom generate -o ./my-module +loom generate -o ./my-module ``` ## How It Works -Loom fetches the PR diff and produces a ready-to-use module: +Loom fetches the changes from each source and produces a ready-to-use module: - **Added files** become templates with {{ .paramName }} replacing the concrete values, including in file and folder names. - **Modified YAML files** become strategic merge patches under `__functions/patches/`. @@ -46,10 +99,10 @@ The generated `loom.yaml` declares all parameters as required and wires up the o ## Example -You onboarded a service called "payments" via a PR. Now you want to make that repeatable: +You onboarded a service called "payments" via a PR, then fixed it in a follow-up commit. Make the combined result repeatable: ```bash -loom generate https://github.com/myorg/gitops-repo/pull/42 \ +loom generate https://github.com/myorg/gitops-repo/pull/42 github:myorg/gitops-repo@9f3ab12 \ -p serviceName=payments \ -p namespace=fintech ``` @@ -62,4 +115,4 @@ loom run ./onboard-service -p serviceName=billing -p namespace=platform ## Authentication -Uses `GITHUB_TOKEN` / `GITLAB_TOKEN` environment variables by default. If the token is not set but the `gh` or `glab` CLI is installed and authenticated, Loom falls back to it automatically. +Only PR/MR sources use a provider API: `GITHUB_TOKEN` / `GITLAB_TOKEN` by default, with automatic fallback to an authenticated `gh` / `glab` CLI. Commit and snapshot sources are git-native and use your existing git credentials (SSH keys, credential helpers).