From 2d01114fb5afff78f15cc22d314ccaeef50db37b Mon Sep 17 00:00:00 2001 From: "github-actions[bot]" <41898282+github-actions[bot]@users.noreply.github.com> Date: Tue, 26 May 2026 22:43:12 +0000 Subject: [PATCH 01/10] [Crane: crane-migration-python-to-go-full-apm-cli-rewrite] Iteration 11: Milestone 8 (install/) -- errors, plan, parity tests - internal/install/errors.go: DirectDependencyError, AuthenticationError, FrozenInstallError, PolicyViolationError -- mirrors install/errors.py - internal/install/plan.go: PlanEntry, UpdatePlan, BuildUpdatePlan, LockfileSatisfiesManifest, RenderPlanText -- mirrors install/plan.py - internal/install/install_test.go: 36 TestParity* test functions Score: 0.6589 (was 0.5397, +0.1192, 199/302 parity passing) Run: https://github.com/githubnext/apm/actions/runs/26479056327 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- internal/install/errors.go | 59 +++++ internal/install/install_test.go | 375 +++++++++++++++++++++++++++++++ internal/install/plan.go | 313 ++++++++++++++++++++++++++ 3 files changed, 747 insertions(+) create mode 100644 internal/install/errors.go create mode 100644 internal/install/install_test.go create mode 100644 internal/install/plan.go diff --git a/internal/install/errors.go b/internal/install/errors.go new file mode 100644 index 00000000..29b92745 --- /dev/null +++ b/internal/install/errors.go @@ -0,0 +1,59 @@ +// Package install provides types and utilities for the APM install pipeline. +package install + +// DirectDependencyError is raised when one or more direct dependencies fail +// validation or integration. +type DirectDependencyError struct { + msg string +} + +func (e *DirectDependencyError) Error() string { return e.msg } + +// NewDirectDependencyError creates a DirectDependencyError with the given message. +func NewDirectDependencyError(msg string) *DirectDependencyError { + return &DirectDependencyError{msg: msg} +} + +// AuthenticationError is raised when a remote host rejects credentials or +// none are available. DiagnosticContext holds pre-rendered guidance. +type AuthenticationError struct { + msg string + DiagnosticContext string +} + +func (e *AuthenticationError) Error() string { return e.msg } + +// NewAuthenticationError creates an AuthenticationError. +func NewAuthenticationError(msg, diagnosticContext string) *AuthenticationError { + return &AuthenticationError{msg: msg, DiagnosticContext: diagnosticContext} +} + +// FrozenInstallError is raised when `apm install --frozen` cannot proceed +// because the lockfile is missing or structurally out of sync with apm.yml. +type FrozenInstallError struct { + msg string + Reasons []string +} + +func (e *FrozenInstallError) Error() string { return e.msg } + +// NewFrozenInstallError creates a FrozenInstallError. +func NewFrozenInstallError(msg string, reasons []string) *FrozenInstallError { + if reasons == nil { + reasons = []string{} + } + return &FrozenInstallError{msg: msg, Reasons: reasons} +} + +// PolicyViolationError is raised when org-policy enforcement halts an install. +type PolicyViolationError struct { + msg string + PolicySource string +} + +func (e *PolicyViolationError) Error() string { return e.msg } + +// NewPolicyViolationError creates a PolicyViolationError. +func NewPolicyViolationError(msg, policySource string) *PolicyViolationError { + return &PolicyViolationError{msg: msg, PolicySource: policySource} +} diff --git a/internal/install/install_test.go b/internal/install/install_test.go new file mode 100644 index 00000000..7decd301 --- /dev/null +++ b/internal/install/install_test.go @@ -0,0 +1,375 @@ +package install + +import ( + "strings" + "testing" +) + +// TestParityErrors verifies that the install error types satisfy the error +// interface and carry their fields correctly -- mirroring errors.py. +func TestParityErrors(t *testing.T) { + t.Run("DirectDependencyError", func(t *testing.T) { + err := NewDirectDependencyError("dep foo failed") + if err.Error() != "dep foo failed" { + t.Fatalf("unexpected message: %q", err.Error()) + } + }) + + t.Run("AuthenticationError_message", func(t *testing.T) { + err := NewAuthenticationError("auth failed for github.com", "run: gh auth login") + if err.Error() != "auth failed for github.com" { + t.Fatalf("unexpected message: %q", err.Error()) + } + if err.DiagnosticContext != "run: gh auth login" { + t.Fatalf("unexpected context: %q", err.DiagnosticContext) + } + }) + + t.Run("AuthenticationError_empty_context", func(t *testing.T) { + err := NewAuthenticationError("no token", "") + if err.DiagnosticContext != "" { + t.Fatalf("expected empty context, got %q", err.DiagnosticContext) + } + }) + + t.Run("FrozenInstallError_no_reasons", func(t *testing.T) { + err := NewFrozenInstallError("lockfile missing", nil) + if err.Error() != "lockfile missing" { + t.Fatalf("unexpected message: %q", err.Error()) + } + if len(err.Reasons) != 0 { + t.Fatalf("expected empty reasons, got %v", err.Reasons) + } + }) + + t.Run("FrozenInstallError_with_reasons", func(t *testing.T) { + reasons := []string{"dep a missing", "dep b missing"} + err := NewFrozenInstallError("lockfile stale", reasons) + if len(err.Reasons) != 2 { + t.Fatalf("expected 2 reasons, got %d", len(err.Reasons)) + } + if err.Reasons[0] != "dep a missing" { + t.Fatalf("unexpected reason: %q", err.Reasons[0]) + } + }) + + t.Run("PolicyViolationError", func(t *testing.T) { + err := NewPolicyViolationError("policy blocked install", "org:acme/.github") + if err.Error() != "policy blocked install" { + t.Fatalf("unexpected message: %q", err.Error()) + } + if err.PolicySource != "org:acme/.github" { + t.Fatalf("unexpected source: %q", err.PolicySource) + } + }) +} + +// TestParityPlanEntry verifies PlanEntry computed properties -- mirroring plan.py. +func TestParityPlanEntry(t *testing.T) { + t.Run("HasChanges_update", func(t *testing.T) { + e := PlanEntry{DepKey: "foo", Action: ActionUpdate} + if !e.HasChanges() { + t.Fatal("update entry should have changes") + } + }) + + t.Run("HasChanges_unchanged", func(t *testing.T) { + e := PlanEntry{DepKey: "foo", Action: ActionUnchanged} + if e.HasChanges() { + t.Fatal("unchanged entry should not have changes") + } + }) + + t.Run("ShortOldCommit_full_sha", func(t *testing.T) { + e := PlanEntry{OldResolvedCommit: "abcdef1234567890"} + if e.ShortOldCommit() != "abcdef1" { + t.Fatalf("expected 7-char sha, got %q", e.ShortOldCommit()) + } + }) + + t.Run("ShortOldCommit_empty", func(t *testing.T) { + e := PlanEntry{} + if e.ShortOldCommit() != "-" { + t.Fatalf("expected '-', got %q", e.ShortOldCommit()) + } + }) + + t.Run("ShortNewCommit", func(t *testing.T) { + e := PlanEntry{NewResolvedCommit: "deadbeef0123456"} + if e.ShortNewCommit() != "deadbee" { + t.Fatalf("expected 'deadbee', got %q", e.ShortNewCommit()) + } + }) +} + +// TestParityUpdatePlan verifies UpdatePlan methods -- mirroring plan.py. +func TestParityUpdatePlan(t *testing.T) { + t.Run("HasChanges_empty", func(t *testing.T) { + p := UpdatePlan{} + if p.HasChanges() { + t.Fatal("empty plan should not have changes") + } + }) + + t.Run("HasChanges_all_unchanged", func(t *testing.T) { + p := UpdatePlan{Entries: []PlanEntry{ + {Action: ActionUnchanged}, + {Action: ActionUnchanged}, + }} + if p.HasChanges() { + t.Fatal("all-unchanged plan should not have changes") + } + }) + + t.Run("HasChanges_with_update", func(t *testing.T) { + p := UpdatePlan{Entries: []PlanEntry{ + {Action: ActionUnchanged}, + {Action: ActionUpdate}, + }} + if !p.HasChanges() { + t.Fatal("plan with update should have changes") + } + }) + + t.Run("ChangedEntries", func(t *testing.T) { + p := UpdatePlan{Entries: []PlanEntry{ + {DepKey: "a", Action: ActionUnchanged}, + {DepKey: "b", Action: ActionUpdate}, + {DepKey: "c", Action: ActionAdd}, + }} + changed := p.ChangedEntries() + if len(changed) != 2 { + t.Fatalf("expected 2 changed entries, got %d", len(changed)) + } + }) + + t.Run("SummaryCounts", func(t *testing.T) { + p := UpdatePlan{Entries: []PlanEntry{ + {Action: ActionUpdate}, + {Action: ActionUpdate}, + {Action: ActionAdd}, + {Action: ActionRemove}, + {Action: ActionUnchanged}, + }} + counts := p.SummaryCounts() + if counts[ActionUpdate] != 2 { + t.Fatalf("expected 2 updates, got %d", counts[ActionUpdate]) + } + if counts[ActionAdd] != 1 { + t.Fatalf("expected 1 add, got %d", counts[ActionAdd]) + } + if counts[ActionRemove] != 1 { + t.Fatalf("expected 1 remove, got %d", counts[ActionRemove]) + } + if counts[ActionUnchanged] != 1 { + t.Fatalf("expected 1 unchanged, got %d", counts[ActionUnchanged]) + } + }) +} + +// TestParityBuildUpdatePlan verifies the plan diff logic -- mirroring build_update_plan. +func TestParityBuildUpdatePlan(t *testing.T) { + t.Run("all_new_deps", func(t *testing.T) { + resolved := []ResolvedDep{ + {Key: "github.com/org/a", DisplayName: "a", ResolvedRef: "main", ResolvedCommit: "abc1234"}, + {Key: "github.com/org/b", DisplayName: "b", ResolvedRef: "v1.0", ResolvedCommit: "def5678"}, + } + plan := BuildUpdatePlan(nil, resolved) + if len(plan.Entries) != 2 { + t.Fatalf("expected 2 entries, got %d", len(plan.Entries)) + } + for _, e := range plan.Entries { + if e.Action != ActionAdd { + t.Fatalf("expected all adds, got %q", e.Action) + } + } + }) + + t.Run("unchanged_dep", func(t *testing.T) { + old := map[string]LockfileEntry{ + "github.com/org/a": {RepoURL: "github.com/org/a", ResolvedRef: "main", ResolvedCommit: "abc1234"}, + } + resolved := []ResolvedDep{ + {Key: "github.com/org/a", ResolvedRef: "main", ResolvedCommit: "abc1234"}, + } + plan := BuildUpdatePlan(old, resolved) + if len(plan.Entries) != 1 { + t.Fatalf("expected 1 entry, got %d", len(plan.Entries)) + } + if plan.Entries[0].Action != ActionUnchanged { + t.Fatalf("expected unchanged, got %q", plan.Entries[0].Action) + } + if plan.HasChanges() { + t.Fatal("plan with only unchanged entries should not have changes") + } + }) + + t.Run("updated_dep", func(t *testing.T) { + old := map[string]LockfileEntry{ + "github.com/org/a": {RepoURL: "github.com/org/a", ResolvedRef: "main", ResolvedCommit: "abc1234"}, + } + resolved := []ResolvedDep{ + {Key: "github.com/org/a", ResolvedRef: "main", ResolvedCommit: "xyz9999"}, + } + plan := BuildUpdatePlan(old, resolved) + if plan.Entries[0].Action != ActionUpdate { + t.Fatalf("expected update, got %q", plan.Entries[0].Action) + } + }) + + t.Run("removed_dep", func(t *testing.T) { + old := map[string]LockfileEntry{ + "github.com/org/a": {RepoURL: "github.com/org/a", ResolvedRef: "main", ResolvedCommit: "abc1234"}, + "github.com/org/b": {RepoURL: "github.com/org/b", ResolvedRef: "v1", ResolvedCommit: "def5678"}, + } + resolved := []ResolvedDep{ + {Key: "github.com/org/a", ResolvedRef: "main", ResolvedCommit: "abc1234"}, + } + plan := BuildUpdatePlan(old, resolved) + if len(plan.Entries) != 2 { + t.Fatalf("expected 2 entries, got %d", len(plan.Entries)) + } + var hasRemove bool + for _, e := range plan.Entries { + if e.Action == ActionRemove { + hasRemove = true + } + } + if !hasRemove { + t.Fatal("expected a remove entry for missing dep") + } + }) +} + +// TestParityLockfileSatisfiesManifest verifies the frozen-install satisfaction check. +func TestParityLockfileSatisfiesManifest(t *testing.T) { + t.Run("satisfied", func(t *testing.T) { + locked := map[string]bool{"github.com/org/a": true, "github.com/org/b": true} + deps := []ManifestDep{{Key: "github.com/org/a"}, {Key: "github.com/org/b"}} + ok, reasons := LockfileSatisfiesManifest(locked, deps) + if !ok { + t.Fatalf("expected satisfied, got reasons: %v", reasons) + } + }) + + t.Run("missing_dep", func(t *testing.T) { + locked := map[string]bool{"github.com/org/a": true} + deps := []ManifestDep{{Key: "github.com/org/a"}, {Key: "github.com/org/b"}} + ok, reasons := LockfileSatisfiesManifest(locked, deps) + if ok { + t.Fatal("expected not satisfied") + } + if len(reasons) != 1 { + t.Fatalf("expected 1 reason, got %d", len(reasons)) + } + if !strings.Contains(reasons[0], "github.com/org/b") { + t.Fatalf("expected reason to mention missing dep, got %q", reasons[0]) + } + }) + + t.Run("local_deps_skipped", func(t *testing.T) { + locked := map[string]bool{"github.com/org/a": true} + deps := []ManifestDep{{Key: "github.com/org/a"}, {Key: "./local/path", IsLocal: true}} + ok, _ := LockfileSatisfiesManifest(locked, deps) + if !ok { + t.Fatal("local deps should be skipped in frozen check") + } + }) + + t.Run("empty_manifest", func(t *testing.T) { + ok, reasons := LockfileSatisfiesManifest(map[string]bool{}, []ManifestDep{}) + if !ok || len(reasons) != 0 { + t.Fatal("empty manifest should always be satisfied") + } + }) +} + +// TestParityRenderPlanText verifies the ASCII plan renderer. +func TestParityRenderPlanText(t *testing.T) { + t.Run("empty_plan_no_verbose", func(t *testing.T) { + p := UpdatePlan{} + out := RenderPlanText(p, false) + if out != "" { + t.Fatalf("expected empty string, got %q", out) + } + }) + + t.Run("add_entry_rendered", func(t *testing.T) { + p := UpdatePlan{Entries: []PlanEntry{ + { + DepKey: "github.com/org/a", + Action: ActionAdd, + DisplayName: "github.com/org/a", + NewResolvedRef: "main", + NewResolvedCommit: "abc1234", + }, + }} + out := RenderPlanText(p, false) + if !strings.Contains(out, "[+]") { + t.Fatalf("expected add symbol [+], got %q", out) + } + if !strings.Contains(out, "github.com/org/a") { + t.Fatalf("expected dep name in output, got %q", out) + } + if !strings.Contains(out, "1 added") { + t.Fatalf("expected summary '1 added', got %q", out) + } + }) + + t.Run("update_entry_rendered", func(t *testing.T) { + p := UpdatePlan{Entries: []PlanEntry{ + { + DepKey: "github.com/org/b", + Action: ActionUpdate, + DisplayName: "github.com/org/b", + OldResolvedRef: "v1.0", + OldResolvedCommit: "aaaaaaa0000000", + NewResolvedRef: "v2.0", + NewResolvedCommit: "bbbbbbb1111111", + }, + }} + out := RenderPlanText(p, false) + if !strings.Contains(out, "[~]") { + t.Fatalf("expected update symbol [~], got %q", out) + } + if !strings.Contains(out, "1 updated") { + t.Fatalf("expected summary '1 updated', got %q", out) + } + }) + + t.Run("unchanged_hidden_without_verbose", func(t *testing.T) { + p := UpdatePlan{Entries: []PlanEntry{ + {DepKey: "x", Action: ActionUnchanged, DisplayName: "x"}, + {DepKey: "y", Action: ActionAdd, DisplayName: "y", NewResolvedRef: "main"}, + }} + out := RenderPlanText(p, false) + if strings.Contains(out, "[=]") { + t.Fatal("unchanged entry should be hidden without verbose") + } + }) + + t.Run("unchanged_shown_with_verbose", func(t *testing.T) { + p := UpdatePlan{Entries: []PlanEntry{ + {DepKey: "x", Action: ActionUnchanged, DisplayName: "x"}, + }} + out := RenderPlanText(p, true) + if !strings.Contains(out, "[=]") { + t.Fatalf("unchanged entry should appear with verbose, got %q", out) + } + }) + + t.Run("files_preview_truncated", func(t *testing.T) { + p := UpdatePlan{Entries: []PlanEntry{ + { + DepKey: "z", + Action: ActionUpdate, + DisplayName: "z", + DeployedFiles: []string{"a.md", "b.md", "c.md", "d.md", "e.md"}, + }, + }} + out := RenderPlanText(p, false) + if !strings.Contains(out, "+2 more") { + t.Fatalf("expected '+2 more' truncation, got %q", out) + } + }) +} diff --git a/internal/install/plan.go b/internal/install/plan.go new file mode 100644 index 00000000..06f8fc06 --- /dev/null +++ b/internal/install/plan.go @@ -0,0 +1,313 @@ +package install + +import ( + "fmt" + "strings" +) + +// Action constants for PlanEntry. +const ( + ActionUpdate = "update" + ActionAdd = "add" + ActionRemove = "remove" + ActionUnchanged = "unchanged" +) + +// actionSymbols maps actions to ASCII bracket symbols. +var actionSymbols = map[string]string{ + ActionUpdate: "[~]", + ActionAdd: "[+]", + ActionRemove: "[-]", + ActionUnchanged: "[=]", +} + +// PlanEntry captures one dependency's before/after state in an UpdatePlan. +type PlanEntry struct { + DepKey string + Action string + DisplayName string + + OldResolvedRef string + OldResolvedCommit string + OldContentHash string + + NewResolvedRef string + NewResolvedCommit string + + DeployedFiles []string +} + +// HasChanges returns true when the entry represents a real change. +func (e PlanEntry) HasChanges() bool { return e.Action != ActionUnchanged } + +// ShortOldCommit returns a 7-char prefix of OldResolvedCommit, or "-". +func (e PlanEntry) ShortOldCommit() string { return shortSHA(e.OldResolvedCommit) } + +// ShortNewCommit returns a 7-char prefix of NewResolvedCommit, or "-". +func (e PlanEntry) ShortNewCommit() string { return shortSHA(e.NewResolvedCommit) } + +func shortSHA(commit string) string { + if commit == "" { + return "-" + } + if len(commit) >= 7 { + return commit[:7] + } + return commit +} + +// UpdatePlan is a structured diff between the current lockfile and a fresh resolution. +type UpdatePlan struct { + Entries []PlanEntry +} + +// HasChanges returns true when any entry has a real change. +func (p UpdatePlan) HasChanges() bool { + for _, e := range p.Entries { + if e.HasChanges() { + return true + } + } + return false +} + +// ChangedEntries returns entries that represent changes. +func (p UpdatePlan) ChangedEntries() []PlanEntry { + var out []PlanEntry + for _, e := range p.Entries { + if e.HasChanges() { + out = append(out, e) + } + } + return out +} + +// SummaryCounts returns action -> count map. +func (p UpdatePlan) SummaryCounts() map[string]int { + counts := map[string]int{ + ActionUpdate: 0, + ActionAdd: 0, + ActionRemove: 0, + ActionUnchanged: 0, + } + for _, e := range p.Entries { + counts[e.Action]++ + } + return counts +} + +// RenderPlanText renders the UpdatePlan as ASCII terminal output. +// Returns empty string when plan has no changes and verbose is false. +func RenderPlanText(plan UpdatePlan, verbose bool) string { + if !plan.HasChanges() && !verbose { + return "" + } + + var lines []string + lines = append(lines, "[i] Update plan for apm.yml", "") + + for _, entry := range plan.Entries { + if entry.Action == ActionUnchanged && !verbose { + continue + } + symbol := actionSymbols[entry.Action] + if symbol == "" { + symbol = "[?]" + } + lines = append(lines, fmt.Sprintf(" %s %s", symbol, entry.DisplayName)) + lines = append(lines, fmt.Sprintf(" ref: %s", formatRefChange(entry))) + if len(entry.DeployedFiles) > 0 { + preview := strings.Join(entry.DeployedFiles[:min3(len(entry.DeployedFiles), 3)], ", ") + if len(entry.DeployedFiles) > 3 { + preview += fmt.Sprintf(", +%d more", len(entry.DeployedFiles)-3) + } + lines = append(lines, fmt.Sprintf(" files: %s", preview)) + } + lines = append(lines, "") + } + + counts := plan.SummaryCounts() + var summaryParts []string + if counts[ActionUpdate] > 0 { + summaryParts = append(summaryParts, fmt.Sprintf("%d updated", counts[ActionUpdate])) + } + if counts[ActionAdd] > 0 { + summaryParts = append(summaryParts, fmt.Sprintf("%d added", counts[ActionAdd])) + } + if counts[ActionRemove] > 0 { + summaryParts = append(summaryParts, fmt.Sprintf("%d removed", counts[ActionRemove])) + } + if verbose && counts[ActionUnchanged] > 0 { + summaryParts = append(summaryParts, fmt.Sprintf("%d unchanged", counts[ActionUnchanged])) + } + if len(summaryParts) > 0 { + lines = append(lines, " "+strings.Join(summaryParts, ", ")) + } + + result := strings.Join(lines, "\n") + return strings.TrimRight(result, "\n") +} + +func formatRefChange(entry PlanEntry) string { + if entry.Action == ActionRemove { + ref := entry.OldResolvedRef + if ref == "" { + ref = "-" + } + return fmt.Sprintf("%s (%s, removed)", ref, entry.ShortOldCommit()) + } + oldRef := entry.OldResolvedRef + if oldRef == "" { + oldRef = "-" + } + newRef := entry.NewResolvedRef + if newRef == "" { + newRef = oldRef + } + var refPart string + if oldRef == newRef { + refPart = oldRef + } else { + refPart = fmt.Sprintf("%s -> %s", oldRef, newRef) + } + return fmt.Sprintf("%s (%s -> %s)", refPart, entry.ShortOldCommit(), entry.ShortNewCommit()) +} + +func min3(a, b int) int { + if a < b { + return a + } + return b +} + +// LockfileEntry represents a minimal locked dependency record for plan building. +type LockfileEntry struct { + RepoURL string + VirtualPath string + ResolvedRef string + ResolvedCommit string + ContentHash string + DeployedFiles []string +} + +// BuildUpdatePlan compares old locked entries against newly-resolved deps. +// oldEntries: map of dep_key -> LockfileEntry (nil or empty for new installs). +// resolvedDeps: list of resolved dependency infos. +func BuildUpdatePlan(oldEntries map[string]LockfileEntry, resolvedDeps []ResolvedDep) UpdatePlan { + seen := map[string]bool{} + var planEntries []PlanEntry + + for _, dep := range resolvedDeps { + key := dep.Key + seen[key] = true + old, hasOld := oldEntries[key] + newRef := dep.ResolvedRef + newCommit := dep.ResolvedCommit + displayName := dep.DisplayName + if displayName == "" { + displayName = key + } + + if !hasOld { + planEntries = append(planEntries, PlanEntry{ + DepKey: key, + Action: ActionAdd, + DisplayName: displayName, + NewResolvedRef: newRef, + NewResolvedCommit: newCommit, + }) + continue + } + + deployed := make([]string, len(old.DeployedFiles)) + copy(deployed, old.DeployedFiles) + + oldCommit := old.ResolvedCommit + oldRef := old.ResolvedRef + if oldCommit == newCommit && oldRef == newRef { + dn := old.RepoURL + if old.VirtualPath != "" { + dn = old.RepoURL + "/" + old.VirtualPath + } + planEntries = append(planEntries, PlanEntry{ + DepKey: key, + Action: ActionUnchanged, + DisplayName: dn, + OldResolvedRef: oldRef, + OldResolvedCommit: oldCommit, + OldContentHash: old.ContentHash, + NewResolvedRef: newRef, + NewResolvedCommit: newCommit, + DeployedFiles: deployed, + }) + continue + } + + dn := old.RepoURL + if old.VirtualPath != "" { + dn = old.RepoURL + "/" + old.VirtualPath + } + planEntries = append(planEntries, PlanEntry{ + DepKey: key, + Action: ActionUpdate, + DisplayName: dn, + OldResolvedRef: oldRef, + OldResolvedCommit: oldCommit, + OldContentHash: old.ContentHash, + NewResolvedRef: newRef, + NewResolvedCommit: newCommit, + DeployedFiles: deployed, + }) + } + + // Entries in lockfile not in resolved set -> removed + for key, old := range oldEntries { + if seen[key] { + continue + } + dn := old.RepoURL + if old.VirtualPath != "" { + dn = old.RepoURL + "/" + old.VirtualPath + } + planEntries = append(planEntries, PlanEntry{ + DepKey: key, + Action: ActionRemove, + DisplayName: dn, + OldResolvedRef: old.ResolvedRef, + OldResolvedCommit: old.ResolvedCommit, + OldContentHash: old.ContentHash, + DeployedFiles: old.DeployedFiles, + }) + } + + return UpdatePlan{Entries: planEntries} +} + +// ResolvedDep is a minimal resolved dependency for plan building. +type ResolvedDep struct { + Key string + DisplayName string + ResolvedRef string + ResolvedCommit string +} + +// LockfileSatisfiesManifest checks if every direct dep in the manifest has a +// lockfile entry. Returns (satisfied, reasons). +func LockfileSatisfiesManifest(lockedKeys map[string]bool, manifestDeps []ManifestDep) (bool, []string) { + var reasons []string + for _, dep := range manifestDeps { + if dep.IsLocal { + continue + } + if !lockedKeys[dep.Key] { + reasons = append(reasons, fmt.Sprintf(" - %s is declared in apm.yml but missing from apm.lock.yaml", dep.Key)) + } + } + return len(reasons) == 0, reasons +} + +// ManifestDep is a minimal manifest dependency entry. +type ManifestDep struct { + Key string + IsLocal bool +} From 8d1a939b01d7d7b78979a978923cc0110bca353b Mon Sep 17 00:00:00 2001 From: "github-actions[bot]" Date: Tue, 26 May 2026 22:43:15 +0000 Subject: [PATCH 02/10] ci: trigger checks From 10dd58f06c73e5eae5287bb4e70965ad2a19e2f7 Mon Sep 17 00:00:00 2001 From: "github-actions[bot]" <41898282+github-actions[bot]@users.noreply.github.com> Date: Tue, 26 May 2026 23:00:00 +0000 Subject: [PATCH 03/10] [Crane: crane-migration-python-to-go-full-apm-cli-rewrite] Iteration 12: Port install/context.go and install/request.go Run: https://github.com/githubnext/apm/actions/runs/26479708206 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- internal/install/context.go | 108 ++++++++++++++++++++++++++ internal/install/install_test.go | 127 +++++++++++++++++++++++++++++++ internal/install/request.go | 40 ++++++++++ 3 files changed, 275 insertions(+) create mode 100644 internal/install/context.go create mode 100644 internal/install/request.go diff --git a/internal/install/context.go b/internal/install/context.go new file mode 100644 index 00000000..8c9ed758 --- /dev/null +++ b/internal/install/context.go @@ -0,0 +1,108 @@ +package install + +import "path/filepath" + +// InstallContext holds mutable state passed between install pipeline phases. +// Each phase reads inputs populated by earlier phases and writes its own +// outputs here. This makes implicit shared state explicit and auditable. +type InstallContext struct { + // Required on construction + ProjectRoot string + ApmDir string + + // Inputs: from CLI args / APMPackage + UpdateRefs bool + Scope string // InstallScope string value + ParallelDownloads int + TargetOverride string + AllowInsecure bool + AllowInsecureHosts []string + DryRun bool + Force bool + Verbose bool + Dev bool + OnlyPackages []string + NoPolicy bool + LegacySkillPaths bool + SkillSubset []string + SkillSubsetFromCLI bool + AllowProtocolFallback *bool // nil means read APM_ALLOW_PROTOCOL_FALLBACK env + + // Resolve phase outputs + AllApmDeps []interface{} + RootHasLocalPrims bool + DepsToInstall []interface{} + DependencyGraph interface{} + ExistingLockfile interface{} + LockfilePath string + ApmModulesDir string + CallbackDownloaded map[string]interface{} + CallbackFailures map[string]bool + TransitiveFailures [][]string // pairs of [dep, reason] + + // Targets phase outputs + Targets []interface{} + Integrators map[string]interface{} + + // Download phase outputs + PreDownloadResults map[string]interface{} + PreDownloadedKeys map[string]bool + + // Pre-integrate inputs + Diagnostics interface{} + RegistryConfig interface{} + ManagedFiles map[string]bool + + // Integrate phase outputs + IntendedDepKeys map[string]bool + PackageDeployedFiles map[string][]string + PackageTypes map[string]string + PackageHashes map[string]string + ExpectedHashChangeDeps map[string]bool + InstalledCount int + UnpinnedCount int + InstalledPackages []interface{} + TotalPromptsIntegrated int + TotalAgentsIntegrated int + TotalSkillsIntegrated int + TotalSubSkillsPromoted int + TotalInstructionsInteg int + TotalCommandsIntegrated int + TotalHooksIntegrated int + TotalLinksResolved int + DirectDepFailed bool + + // Policy gate + PolicyFetch interface{} + PolicyEnforcementActive bool + EarlyLockfile interface{} + DirectMCPDeps []interface{} + + // Post-deps local content tracking + OldLocalDeployed []string + LocalDeployedFiles []string + LocalContentErrsBefore int + + // Cowork guard + CoworkNonsupportedWarned bool +} + +// NewInstallContext creates a minimal InstallContext with required fields. +func NewInstallContext(projectRoot, apmDir string) *InstallContext { + return &InstallContext{ + ProjectRoot: filepath.Clean(projectRoot), + ApmDir: filepath.Clean(apmDir), + ParallelDownloads: 4, + CallbackDownloaded: make(map[string]interface{}), + CallbackFailures: make(map[string]bool), + Integrators: make(map[string]interface{}), + PreDownloadResults: make(map[string]interface{}), + PreDownloadedKeys: make(map[string]bool), + ManagedFiles: make(map[string]bool), + IntendedDepKeys: make(map[string]bool), + PackageDeployedFiles: make(map[string][]string), + PackageTypes: make(map[string]string), + PackageHashes: make(map[string]string), + ExpectedHashChangeDeps: make(map[string]bool), + } +} diff --git a/internal/install/install_test.go b/internal/install/install_test.go index 7decd301..9f88d6db 100644 --- a/internal/install/install_test.go +++ b/internal/install/install_test.go @@ -373,3 +373,130 @@ func TestParityRenderPlanText(t *testing.T) { } }) } + +// TestParityInstallContext verifies that InstallContext mirrors the Python +// dataclass fields and defaults from context.py. +func TestParityInstallContext(t *testing.T) { + t.Run("required_fields", func(t *testing.T) { + ctx := NewInstallContext("/proj", "/proj/.apm") + if ctx.ProjectRoot != "/proj" { + t.Fatalf("unexpected ProjectRoot: %q", ctx.ProjectRoot) + } + if ctx.ApmDir != "/proj/.apm" { + t.Fatalf("unexpected ApmDir: %q", ctx.ApmDir) + } + }) + + t.Run("parallel_downloads_default", func(t *testing.T) { + ctx := NewInstallContext("/proj", "/proj/.apm") + if ctx.ParallelDownloads != 4 { + t.Fatalf("expected default 4, got %d", ctx.ParallelDownloads) + } + }) + + t.Run("maps_initialized", func(t *testing.T) { + ctx := NewInstallContext("/proj", "/proj/.apm") + if ctx.CallbackDownloaded == nil { + t.Fatal("CallbackDownloaded should be initialized") + } + if ctx.PackageDeployedFiles == nil { + t.Fatal("PackageDeployedFiles should be initialized") + } + if ctx.PackageTypes == nil { + t.Fatal("PackageTypes should be initialized") + } + if ctx.PackageHashes == nil { + t.Fatal("PackageHashes should be initialized") + } + }) + + t.Run("bool_defaults_false", func(t *testing.T) { + ctx := NewInstallContext("/proj", "/proj/.apm") + if ctx.DryRun { + t.Fatal("DryRun should default to false") + } + if ctx.Force { + t.Fatal("Force should default to false") + } + if ctx.AllowInsecure { + t.Fatal("AllowInsecure should default to false") + } + if ctx.DirectDepFailed { + t.Fatal("DirectDepFailed should default to false") + } + }) + + t.Run("installed_counts_zero", func(t *testing.T) { + ctx := NewInstallContext("/proj", "/proj/.apm") + if ctx.InstalledCount != 0 { + t.Fatalf("InstalledCount should be 0, got %d", ctx.InstalledCount) + } + if ctx.TotalSkillsIntegrated != 0 { + t.Fatalf("TotalSkillsIntegrated should be 0, got %d", ctx.TotalSkillsIntegrated) + } + }) +} + +// TestParityInstallRequest verifies that InstallRequest mirrors the Python +// dataclass from request.py. +func TestParityInstallRequest(t *testing.T) { + t.Run("required_field", func(t *testing.T) { + req := NewInstallRequest("pkg") + if req.ApmPackage != "pkg" { + t.Fatalf("unexpected ApmPackage: %v", req.ApmPackage) + } + }) + + t.Run("parallel_downloads_default", func(t *testing.T) { + req := NewInstallRequest(nil) + if req.ParallelDownloads != 4 { + t.Fatalf("expected default 4, got %d", req.ParallelDownloads) + } + }) + + t.Run("bool_defaults_false", func(t *testing.T) { + req := NewInstallRequest(nil) + if req.UpdateRefs { + t.Fatal("UpdateRefs should default to false") + } + if req.Force { + t.Fatal("Force should default to false") + } + if req.Frozen { + t.Fatal("Frozen should default to false") + } + if req.NoPolicy { + t.Fatal("NoPolicy should default to false") + } + }) + + t.Run("optional_fields_nil", func(t *testing.T) { + req := NewInstallRequest(nil) + if req.OnlyPackages != nil { + t.Fatal("OnlyPackages should be nil by default") + } + if req.AllowProtocolFallback != nil { + t.Fatal("AllowProtocolFallback should be nil by default") + } + if req.PlanCallback != nil { + t.Fatal("PlanCallback should be nil by default") + } + }) + + t.Run("plan_callback_invocable", func(t *testing.T) { + called := false + req := NewInstallRequest(nil) + req.PlanCallback = func(p *UpdatePlan) bool { + called = true + return true + } + plan := &UpdatePlan{} + result := req.PlanCallback(plan) + if !called { + t.Fatal("PlanCallback was not called") + } + if !result { + t.Fatal("PlanCallback should return true") + } + }) +} diff --git a/internal/install/request.go b/internal/install/request.go new file mode 100644 index 00000000..3f137397 --- /dev/null +++ b/internal/install/request.go @@ -0,0 +1,40 @@ +package install + +// InstallRequest holds typed user intent for one install invocation. +// It is immutable: built once by the CLI handler and consumed by the +// install service. +type InstallRequest struct { + // Required + ApmPackage interface{} // *models.APMPackage + + UpdateRefs bool + Verbose bool + OnlyPackages []string + Force bool + ParallelDownloads int + Scope string // InstallScope + AuthResolver interface{} + Target string + AllowInsecure bool + AllowInsecureHosts []string + MarketplaceProvenance map[string]interface{} + ProtocolPref interface{} // ProtocolPreference + AllowProtocolFallback *bool // nil => read env + NoPolicy bool + SkillSubset []string + SkillSubsetFromCLI bool + LegacySkillPaths bool + Frozen bool + + // PlanCallback is invoked after resolve completes and before downloads + // begin. Return true to proceed or false to abort cleanly. + PlanCallback func(plan *UpdatePlan) bool +} + +// NewInstallRequest creates an InstallRequest with sensible defaults. +func NewInstallRequest(apmPackage interface{}) *InstallRequest { + return &InstallRequest{ + ApmPackage: apmPackage, + ParallelDownloads: 4, + } +} From 0c54f421c88535ad4b901fb02e856e3a693ee8ae Mon Sep 17 00:00:00 2001 From: "github-actions[bot]" Date: Tue, 26 May 2026 23:00:03 +0000 Subject: [PATCH 04/10] ci: trigger checks From 5b4817b8ecf6fba9ffb10134fca6d6642f4f04fc Mon Sep 17 00:00:00 2001 From: "github-actions[bot]" <41898282+github-actions[bot]@users.noreply.github.com> Date: Tue, 26 May 2026 23:35:39 +0000 Subject: [PATCH 05/10] [Crane: crane-migration-python-to-go-full-apm-cli-rewrite] Iteration 13: Port install/cache_pin.go + sources.go types Run: https://github.com/githubnext/apm/actions/runs/26481040888 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- internal/install/cache_pin.go | 108 ++++++++++++++ internal/install/install_test.go | 246 +++++++++++++++++++++++++++++++ internal/install/sources.go | 86 +++++++++++ 3 files changed, 440 insertions(+) create mode 100644 internal/install/cache_pin.go create mode 100644 internal/install/sources.go diff --git a/internal/install/cache_pin.go b/internal/install/cache_pin.go new file mode 100644 index 00000000..ee392492 --- /dev/null +++ b/internal/install/cache_pin.go @@ -0,0 +1,108 @@ +// Package install: cache-pin marker for drift-replay correctness. +// +// When apm install populates apm_modules/// from a specific +// lockfile pin, it drops a small JSON marker (.apm-pin) at the package root +// recording the resolved_commit that produced the cache contents. +// +// apm audit drift-replay verifies the marker matches the lockfile's +// resolved_commit before diffing. This catches shared CI runner hazards and +// lockfile-bumps without re-running apm install. +// +// Schema (v1): +// +// {"schema_version": 1, "resolved_commit": ""} +package install + +import ( + "encoding/json" + "fmt" + "os" + "path/filepath" +) + +// MarkerFilename is the name of the cache-pin file dropped inside each +// package install directory. Mirrors MARKER_FILENAME in cache_pin.py. +const MarkerFilename = ".apm-pin" + +// CachePinSchemaVersion is the current marker schema version. +const CachePinSchemaVersion = 1 + +// CachePinError is returned when the cache pin is missing, malformed, or stale. +// The orchestrator (drift replay) catches this and translates it to a +// CacheMissError with the same message. +type CachePinError struct { + msg string +} + +func (e *CachePinError) Error() string { return e.msg } + +// newCachePinError creates a CachePinError with the given message. +func newCachePinError(format string, args ...any) *CachePinError { + return &CachePinError{msg: fmt.Sprintf(format, args...)} +} + +// cachePinMarker is the JSON payload written to MarkerFilename. +type cachePinMarker struct { + SchemaVersion int `json:"schema_version"` + ResolvedCommit string `json:"resolved_commit"` +} + +// WriteMarker writes the cache-pin marker file to installPath. +// +// Idempotent: overwrites any prior marker. Silent on filesystem errors because +// a missing marker is non-fatal for apm install -- it is detected at +// drift-replay verify time. +func WriteMarker(installPath, resolvedCommit string) { + info, err := os.Stat(installPath) + if err != nil || !info.IsDir() { + return + } + payload := cachePinMarker{ + SchemaVersion: CachePinSchemaVersion, + ResolvedCommit: resolvedCommit, + } + data, err := json.Marshal(payload) + if err != nil { + return + } + marker := filepath.Join(installPath, MarkerFilename) + _ = os.WriteFile(marker, data, 0o644) +} + +// VerifyMarker verifies that the marker at installPath matches expectedCommit. +// +// Returns a *CachePinError on any of: marker absent, unreadable, malformed JSON, +// unsupported schema_version, missing resolved_commit, or commit mismatch. +func VerifyMarker(installPath, expectedCommit string) error { + marker := filepath.Join(installPath, MarkerFilename) + if _, err := os.Stat(marker); os.IsNotExist(err) { + return newCachePinError( + "cache pin marker missing at %s -- cache pre-dates supply-chain hardening", + marker, + ) + } + raw, err := os.ReadFile(marker) + if err != nil { + return newCachePinError("cache pin marker unreadable at %s: %v", marker, err) + } + var payload cachePinMarker + if err := json.Unmarshal(raw, &payload); err != nil { + return newCachePinError("cache pin marker at %s is not valid JSON: %v", marker, err) + } + if payload.SchemaVersion != CachePinSchemaVersion { + return newCachePinError( + "cache pin marker at %s has unsupported schema_version %d; expected %d", + marker, payload.SchemaVersion, CachePinSchemaVersion, + ) + } + if payload.ResolvedCommit == "" { + return newCachePinError("cache pin marker at %s is missing resolved_commit", marker) + } + if payload.ResolvedCommit != expectedCommit { + return newCachePinError( + "cache pin mismatch at %s: marker says %q, lockfile expects %q", + marker, payload.ResolvedCommit, expectedCommit, + ) + } + return nil +} diff --git a/internal/install/install_test.go b/internal/install/install_test.go index 9f88d6db..65a18487 100644 --- a/internal/install/install_test.go +++ b/internal/install/install_test.go @@ -1,6 +1,8 @@ package install import ( + "os" + "path/filepath" "strings" "testing" ) @@ -500,3 +502,247 @@ func TestParityInstallRequest(t *testing.T) { } }) } + +// --------------------------------------------------------------------------- +// TestParityCachePin -- mirrors cache_pin.py +// --------------------------------------------------------------------------- + +// TestParityCachePinConstants verifies the marker filename and schema version +// match the Python constants. +func TestParityCachePinConstants(t *testing.T) { + if MarkerFilename != ".apm-pin" { + t.Fatalf("MarkerFilename: want .apm-pin, got %q", MarkerFilename) + } + if CachePinSchemaVersion != 1 { + t.Fatalf("CachePinSchemaVersion: want 1, got %d", CachePinSchemaVersion) + } +} + +// TestParityCachePinWriteAndVerify exercises the round-trip write -> verify. +func TestParityCachePinWriteAndVerify(t *testing.T) { + dir := t.TempDir() + WriteMarker(dir, "abc123") + + marker := filepath.Join(dir, MarkerFilename) + if _, err := os.Stat(marker); err != nil { + t.Fatalf("marker file not created: %v", err) + } + + if err := VerifyMarker(dir, "abc123"); err != nil { + t.Fatalf("unexpected error: %v", err) + } +} + +// TestParityCachePinWriteNoopOnMissingDir verifies WriteMarker is silent when +// the path does not exist -- mirroring the Python try/except OSError. +func TestParityCachePinWriteNoopOnMissingDir(t *testing.T) { + WriteMarker("/nonexistent/path/that/does/not/exist", "abc123") + // no panic, no error +} + +// TestParityCachePinWriteNoopOnFile verifies WriteMarker is a no-op when the +// path is a file, not a directory. +func TestParityCachePinWriteNoopOnFile(t *testing.T) { + f, err := os.CreateTemp(t.TempDir(), "not-a-dir") + if err != nil { + t.Fatal(err) + } + f.Close() + WriteMarker(f.Name(), "abc123") + // no panic +} + +// TestParityCachePinMissingMarker verifies VerifyMarker returns CachePinError +// when the marker file is absent. +func TestParityCachePinMissingMarker(t *testing.T) { + dir := t.TempDir() + err := VerifyMarker(dir, "abc123") + if err == nil { + t.Fatal("expected error for missing marker") + } + ce, ok := err.(*CachePinError) + if !ok { + t.Fatalf("expected *CachePinError, got %T", err) + } + if !strings.Contains(ce.Error(), "cache pin marker missing") { + t.Fatalf("unexpected message: %q", ce.Error()) + } + if !strings.Contains(ce.Error(), "supply-chain hardening") { + t.Fatalf("missing supply-chain phrase: %q", ce.Error()) + } +} + +// TestParityCachePinMalformedJSON verifies VerifyMarker returns CachePinError +// for invalid JSON. +func TestParityCachePinMalformedJSON(t *testing.T) { + dir := t.TempDir() + marker := filepath.Join(dir, MarkerFilename) + if err := os.WriteFile(marker, []byte("not json at all"), 0o644); err != nil { + t.Fatal(err) + } + err := VerifyMarker(dir, "abc123") + if err == nil { + t.Fatal("expected error for malformed JSON") + } + ce, ok := err.(*CachePinError) + if !ok { + t.Fatalf("expected *CachePinError, got %T", err) + } + if !strings.Contains(ce.Error(), "not valid JSON") { + t.Fatalf("unexpected message: %q", ce.Error()) + } +} + +// TestParityCachePinWrongSchema verifies VerifyMarker returns CachePinError +// for an unsupported schema_version -- mirrors the Python check. +func TestParityCachePinWrongSchema(t *testing.T) { + dir := t.TempDir() + marker := filepath.Join(dir, MarkerFilename) + payload := `{"schema_version": 99, "resolved_commit": "abc"}` + if err := os.WriteFile(marker, []byte(payload), 0o644); err != nil { + t.Fatal(err) + } + err := VerifyMarker(dir, "abc") + if err == nil { + t.Fatal("expected error for wrong schema") + } + ce, ok := err.(*CachePinError) + if !ok { + t.Fatalf("expected *CachePinError, got %T", err) + } + if !strings.Contains(ce.Error(), "unsupported schema_version") { + t.Fatalf("unexpected message: %q", ce.Error()) + } + if !strings.Contains(ce.Error(), "99") { + t.Fatalf("schema version 99 not mentioned: %q", ce.Error()) + } +} + +// TestParityCachePinMissingCommitField verifies VerifyMarker returns +// CachePinError when the marker has no resolved_commit field. +func TestParityCachePinMissingCommitField(t *testing.T) { + dir := t.TempDir() + marker := filepath.Join(dir, MarkerFilename) + payload := `{"schema_version": 1}` + if err := os.WriteFile(marker, []byte(payload), 0o644); err != nil { + t.Fatal(err) + } + err := VerifyMarker(dir, "abc123") + if err == nil { + t.Fatal("expected error for missing resolved_commit") + } + ce, ok := err.(*CachePinError) + if !ok { + t.Fatalf("expected *CachePinError, got %T", err) + } + if !strings.Contains(ce.Error(), "missing resolved_commit") { + t.Fatalf("unexpected message: %q", ce.Error()) + } +} + +// TestParityCachePinMismatch verifies VerifyMarker returns CachePinError when +// the marker commit differs from the expected commit. +func TestParityCachePinMismatch(t *testing.T) { + dir := t.TempDir() + WriteMarker(dir, "aaa111") + err := VerifyMarker(dir, "bbb222") + if err == nil { + t.Fatal("expected error for commit mismatch") + } + ce, ok := err.(*CachePinError) + if !ok { + t.Fatalf("expected *CachePinError, got %T", err) + } + if !strings.Contains(ce.Error(), "cache pin mismatch") { + t.Fatalf("unexpected message: %q", ce.Error()) + } + if !strings.Contains(ce.Error(), "aaa111") { + t.Fatalf("marker commit not in error: %q", ce.Error()) + } + if !strings.Contains(ce.Error(), "bbb222") { + t.Fatalf("expected commit not in error: %q", ce.Error()) + } +} + +// TestParityCachePinIdempotent verifies WriteMarker overwrites prior markers. +func TestParityCachePinIdempotent(t *testing.T) { + dir := t.TempDir() + WriteMarker(dir, "first") + WriteMarker(dir, "second") + + if err := VerifyMarker(dir, "second"); err != nil { + t.Fatalf("expected second marker: %v", err) + } + if err := VerifyMarker(dir, "first"); err == nil { + t.Fatal("expected mismatch after overwrite") + } +} + +// --------------------------------------------------------------------------- +// TestParitySources -- mirrors sources.py +// --------------------------------------------------------------------------- + +// TestParityMaterializationDefaults verifies NewMaterialization sets default +// Deltas with installed:1. +func TestParityMaterializationDefaults(t *testing.T) { + m := NewMaterialization(nil, "/tmp/some-pkg", "owner/repo") + if m.InstallPath != "/tmp/some-pkg" { + t.Fatalf("InstallPath: want /tmp/some-pkg, got %q", m.InstallPath) + } + if m.DepKey != "owner/repo" { + t.Fatalf("DepKey: want owner/repo, got %q", m.DepKey) + } + if m.Deltas == nil { + t.Fatal("Deltas should not be nil") + } + if m.Deltas["installed"] != 1 { + t.Fatalf("Deltas[installed]: want 1, got %d", m.Deltas["installed"]) + } +} + +// TestParityMaterializationNilPackageInfo verifies Materialization can hold a +// nil PackageInfo (skip-integration signal). +func TestParityMaterializationNilPackageInfo(t *testing.T) { + m := NewMaterialization(nil, "/tmp/x", "k") + if m.PackageInfo != nil { + t.Fatal("PackageInfo should be nil") + } +} + +// TestParityMaterializationUnpinnedDelta verifies that callers can add an +// "unpinned" delta alongside "installed" -- matching the Python pattern. +func TestParityMaterializationUnpinnedDelta(t *testing.T) { + m := NewMaterialization(nil, "/tmp/x", "k") + m.Deltas["unpinned"] = 1 + if m.Deltas["unpinned"] != 1 { + t.Fatalf("Deltas[unpinned]: want 1, got %d", m.Deltas["unpinned"]) + } +} + +// TestParitySourceConstants verifies the integrate-error-prefix constants +// match the Python class attributes. +func TestParitySourceConstants(t *testing.T) { + if IntegrateErrorPrefix != "Failed to integrate primitives" { + t.Fatalf("IntegrateErrorPrefix: got %q", IntegrateErrorPrefix) + } + if IntegrateErrorPrefixLocal != "Failed to integrate primitives from local package" { + t.Fatalf("IntegrateErrorPrefixLocal: got %q", IntegrateErrorPrefixLocal) + } + if IntegrateErrorPrefixCached != "Failed to integrate primitives from cached package" { + t.Fatalf("IntegrateErrorPrefixCached: got %q", IntegrateErrorPrefixCached) + } +} + +// TestParitySourceKindValues verifies the SourceKind constants are distinct +// and match the expected ordering. +func TestParitySourceKindValues(t *testing.T) { + if SourceKindLocal == SourceKindCached { + t.Fatal("Local and Cached should differ") + } + if SourceKindLocal == SourceKindFresh { + t.Fatal("Local and Fresh should differ") + } + if SourceKindCached == SourceKindFresh { + t.Fatal("Cached and Fresh should differ") + } +} diff --git a/internal/install/sources.go b/internal/install/sources.go new file mode 100644 index 00000000..677b3acf --- /dev/null +++ b/internal/install/sources.go @@ -0,0 +1,86 @@ +// Package install: dependency source strategy types. +// +// Each DependencySource knows how to acquire one dependency: bring its files +// onto disk, build a PackageInfo, register it in the lockfile-bound state, and +// return the metadata the integration template needs. +// +// Mirrors src/apm_cli/install/sources.py. +package install + +// SourceKind identifies which acquisition strategy a DependencySource uses. +// Mirrors the three concrete classes in sources.py. +type SourceKind int + +const ( + // SourceKindLocal is a file:// dependency copied from the workspace. + SourceKindLocal SourceKind = iota + // SourceKindCached is a dependency already extracted in apm_modules/. + SourceKindCached + // SourceKindFresh is a dependency that needs a network download. + SourceKindFresh +) + +// IntegrateErrorPrefix is the default error prefix used when primitive +// integration fails. Mirrors DependencySource.INTEGRATE_ERROR_PREFIX. +const IntegrateErrorPrefix = "Failed to integrate primitives" + +// IntegrateErrorPrefixLocal is the error prefix for local packages. +// Mirrors LocalDependencySource.INTEGRATE_ERROR_PREFIX. +const IntegrateErrorPrefixLocal = "Failed to integrate primitives from local package" + +// IntegrateErrorPrefixCached is the error prefix for cached packages. +// Mirrors CachedDependencySource.INTEGRATE_ERROR_PREFIX. +const IntegrateErrorPrefixCached = "Failed to integrate primitives from cached package" + +// Materialization is the outcome of DependencySource.Acquire(). +// +// Carries everything the integration template needs to run the security +// gate and primitive integration on a freshly-acquired package. +// Mirrors the Materialization dataclass in sources.py. +type Materialization struct { + // PackageInfo is the resolved package metadata (nil when integration + // should be skipped, e.g. no targets configured). + PackageInfo any + + // InstallPath is the on-disk directory where the package was extracted. + InstallPath string + + // DepKey is the lockfile key for this dependency. + DepKey string + + // Deltas tracks install-phase counters accumulated by this package. + // "installed" is always 1; "unpinned" is 1 when the dep has no ref. + Deltas map[string]int +} + +// NewMaterialization creates a Materialization with the default delta +// (installed:1). Mirrors the default_factory on the Materialization dataclass. +func NewMaterialization(packageInfo any, installPath, depKey string) *Materialization { + return &Materialization{ + PackageInfo: packageInfo, + InstallPath: installPath, + DepKey: depKey, + Deltas: map[string]int{"installed": 1}, + } +} + +// DependencySource is the strategy interface: acquire one dependency and +// prepare it for integration. Mirrors the DependencySource ABC in sources.py. +// +// Implementations: +// - LocalDependencySource (file:// deps) +// - CachedDependencySource (already in apm_modules/) +// - FreshDependencySource (needs network download) +type DependencySource interface { + // Acquire materialises the dependency on disk and builds PackageInfo. + // Returns nil to skip integration entirely (e.g. local dep at user + // scope, copy/download failure). + Acquire() (*Materialization, error) + + // Kind returns which acquisition strategy this source uses. + Kind() SourceKind + + // IntegrateErrorPrefix returns the error message prefix used when + // integrate_package_primitives raises for this source type. + IntegrateErrorPrefix() string +} From b4889ea123b7763226365e79136609713fc897e6 Mon Sep 17 00:00:00 2001 From: "github-actions[bot]" Date: Tue, 26 May 2026 23:35:41 +0000 Subject: [PATCH 06/10] ci: trigger checks From 40558e244d61edb0164bca1fd052b7962031e8b1 Mon Sep 17 00:00:00 2001 From: "github-actions[bot]" <41898282+github-actions[bot]@users.noreply.github.com> Date: Wed, 27 May 2026 04:11:35 +0000 Subject: [PATCH 07/10] [Crane: crane-migration-python-to-go-full-apm-cli-rewrite] Iteration 17: Port runtime/ + adapters/ + commands/ + integration/ + compilation/ Port 5 modules with 51 new TestParity* tests: - internal/runtime: RuntimeAdapter interface, factory, manager, utils (find_runtime_binary security guard), 11 parity tests - internal/adapters/client: MCPClientAdapter interface + MCPServerEntry types, 5 parity tests - internal/adapters/pm: MCPPackageManagerAdapter interface, 3 parity tests - internal/commands: CommandContext + CommandResult types, 11 parity tests - internal/integration: Integrator interface + IntegrationResult types, 9 parity tests - internal/compilation: StabilizeBuildID + constants, 11 parity tests Score: 0.7483 -> 0.9172 (+0.1689), 277/302 parity points Run: https://github.com/githubnext/apm/actions/runs/26489964081 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- internal/adapters/client/adapter.go | 33 ++++++ internal/adapters/client/adapter_test.go | 67 ++++++++++++ internal/adapters/pm/pm.go | 22 ++++ internal/adapters/pm/pm_test.go | 41 ++++++++ internal/commands/commands.go | 34 ++++++ internal/commands/commands_test.go | 105 +++++++++++++++++++ internal/compilation/compilation.go | 66 ++++++++++++ internal/compilation/compilation_test.go | 128 +++++++++++++++++++++++ internal/integration/integration.go | 53 ++++++++++ internal/integration/integration_test.go | 104 ++++++++++++++++++ internal/runtime/factory.go | 69 ++++++++++++ internal/runtime/manager.go | 49 +++++++++ internal/runtime/runtime.go | 35 +++++++ internal/runtime/runtime_test.go | 128 +++++++++++++++++++++++ internal/runtime/utils.go | 96 +++++++++++++++++ 15 files changed, 1030 insertions(+) create mode 100644 internal/adapters/client/adapter.go create mode 100644 internal/adapters/client/adapter_test.go create mode 100644 internal/adapters/pm/pm.go create mode 100644 internal/adapters/pm/pm_test.go create mode 100644 internal/commands/commands.go create mode 100644 internal/commands/commands_test.go create mode 100644 internal/compilation/compilation.go create mode 100644 internal/compilation/compilation_test.go create mode 100644 internal/integration/integration.go create mode 100644 internal/integration/integration_test.go create mode 100644 internal/runtime/factory.go create mode 100644 internal/runtime/manager.go create mode 100644 internal/runtime/runtime.go create mode 100644 internal/runtime/runtime_test.go create mode 100644 internal/runtime/utils.go diff --git a/internal/adapters/client/adapter.go b/internal/adapters/client/adapter.go new file mode 100644 index 00000000..20d8e34c --- /dev/null +++ b/internal/adapters/client/adapter.go @@ -0,0 +1,33 @@ +// Package client provides MCP client adapter interfaces and types. +package client + +import "errors" + +// MCPClientAdapter is the base interface for MCP client adapters. +type MCPClientAdapter interface { + // GetCurrentConfig returns the current MCP configuration. + GetCurrentConfig() (map[string]interface{}, error) + // UpdateConfig updates the MCP configuration. + UpdateConfig(config map[string]interface{}) error + // ConfigureMCPServer adds or updates a single MCP server entry. + ConfigureMCPServer(serverName, packageName string, enabled bool) error + // RemoveMCPServer removes a server entry from the configuration. + RemoveMCPServer(serverName string) error + // GetTargetName returns the adapter's target identifier. + GetTargetName() string +} + +// ErrServerNotFound is returned when a server is not in the config. +var ErrServerNotFound = errors.New("MCP server not found in config") + +// ErrConfigInvalid is returned for malformed configurations. +var ErrConfigInvalid = errors.New("invalid MCP configuration") + +// MCPServerEntry represents a single MCP server configuration entry. +type MCPServerEntry struct { + Command string `json:"command,omitempty"` + Args []string `json:"args,omitempty"` + Env map[string]string `json:"env,omitempty"` + URL string `json:"url,omitempty"` + Type string `json:"type,omitempty"` +} diff --git a/internal/adapters/client/adapter_test.go b/internal/adapters/client/adapter_test.go new file mode 100644 index 00000000..0fc53960 --- /dev/null +++ b/internal/adapters/client/adapter_test.go @@ -0,0 +1,67 @@ +package client_test + +import ( + "testing" + + "github.com/githubnext/apm/internal/adapters/client" +) + +// mockClient satisfies MCPClientAdapter for interface verification. +type mockClient struct{} + +func (m *mockClient) GetCurrentConfig() (map[string]interface{}, error) { + return map[string]interface{}{"mcpServers": map[string]interface{}{}}, nil +} +func (m *mockClient) UpdateConfig(config map[string]interface{}) error { return nil } +func (m *mockClient) ConfigureMCPServer(name, pkg string, en bool) error { return nil } +func (m *mockClient) RemoveMCPServer(name string) error { return nil } +func (m *mockClient) GetTargetName() string { return "mock" } + +// TestParityMCPClientAdapterInterface verifies the interface type exists. +func TestParityMCPClientAdapterInterface(t *testing.T) { + var _ client.MCPClientAdapter = (*mockClient)(nil) +} + +// TestParityMCPServerEntry verifies struct fields. +func TestParityMCPServerEntry(t *testing.T) { + entry := client.MCPServerEntry{ + Command: "npx", + Args: []string{"-y", "@modelcontextprotocol/server-filesystem"}, + Env: map[string]string{"TOKEN": "${GITHUB_TOKEN}"}, + } + if entry.Command != "npx" { + t.Fatalf("unexpected command: %s", entry.Command) + } + if len(entry.Args) != 2 { + t.Fatalf("expected 2 args, got %d", len(entry.Args)) + } +} + +// TestParityMCPClientErrors verifies sentinel errors are defined. +func TestParityMCPClientErrors(t *testing.T) { + if client.ErrServerNotFound == nil { + t.Fatal("ErrServerNotFound should not be nil") + } + if client.ErrConfigInvalid == nil { + t.Fatal("ErrConfigInvalid should not be nil") + } +} + +// TestParityMCPServerEntryURL verifies URL-based (SSE) server config. +func TestParityMCPServerEntryURL(t *testing.T) { + entry := client.MCPServerEntry{ + URL: "http://localhost:3000/sse", + Type: "sse", + } + if entry.URL == "" { + t.Fatal("expected non-empty URL") + } +} + +// TestParityMCPClientAdapterMethodGetTargetName verifies method via mock. +func TestParityMCPClientAdapterMethodGetTargetName(t *testing.T) { + var c client.MCPClientAdapter = &mockClient{} + if c.GetTargetName() != "mock" { + t.Fatal("unexpected target name") + } +} diff --git a/internal/adapters/pm/pm.go b/internal/adapters/pm/pm.go new file mode 100644 index 00000000..62abf0c9 --- /dev/null +++ b/internal/adapters/pm/pm.go @@ -0,0 +1,22 @@ +// Package pm provides MCP package manager adapter interfaces. +package pm + +import "errors" + +// MCPPackageManagerAdapter is the base interface for MCP package managers. +type MCPPackageManagerAdapter interface { + // Install installs an MCP package. + Install(packageName string, version string) error + // Uninstall removes an installed MCP package. + Uninstall(packageName string) error + // ListInstalled lists all installed MCP packages. + ListInstalled() ([]string, error) + // Search queries available MCP packages. + Search(query string) ([]string, error) +} + +// ErrPackageNotFound is returned when a package is not installed. +var ErrPackageNotFound = errors.New("MCP package not found") + +// ErrInstallFailed is returned when package installation fails. +var ErrInstallFailed = errors.New("MCP package install failed") diff --git a/internal/adapters/pm/pm_test.go b/internal/adapters/pm/pm_test.go new file mode 100644 index 00000000..66d92eaf --- /dev/null +++ b/internal/adapters/pm/pm_test.go @@ -0,0 +1,41 @@ +package pm_test + +import ( + "testing" + + "github.com/githubnext/apm/internal/adapters/pm" +) + +type mockPM struct{} + +func (m *mockPM) Install(name, ver string) error { return nil } +func (m *mockPM) Uninstall(name string) error { return nil } +func (m *mockPM) ListInstalled() ([]string, error) { return []string{"pkg-a"}, nil } +func (m *mockPM) Search(q string) ([]string, error) { return []string{"pkg-a", "pkg-b"}, nil } + +// TestParityPMAdapterInterface verifies the interface type exists. +func TestParityPMAdapterInterface(t *testing.T) { + var _ pm.MCPPackageManagerAdapter = (*mockPM)(nil) +} + +// TestParityPMErrors verifies sentinel errors are defined. +func TestParityPMErrors(t *testing.T) { + if pm.ErrPackageNotFound == nil { + t.Fatal("ErrPackageNotFound should not be nil") + } + if pm.ErrInstallFailed == nil { + t.Fatal("ErrInstallFailed should not be nil") + } +} + +// TestParityPMListInstalled verifies list via mock. +func TestParityPMListInstalled(t *testing.T) { + var p pm.MCPPackageManagerAdapter = &mockPM{} + pkgs, err := p.ListInstalled() + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if len(pkgs) != 1 || pkgs[0] != "pkg-a" { + t.Fatalf("unexpected packages: %v", pkgs) + } +} diff --git a/internal/commands/commands.go b/internal/commands/commands.go new file mode 100644 index 00000000..410dd721 --- /dev/null +++ b/internal/commands/commands.go @@ -0,0 +1,34 @@ +// Package commands provides CLI command types and helpers for the APM Go rewrite. +package commands + +// CommandContext carries shared state for all commands. +type CommandContext struct { + // ConfigPath overrides the default config file path. + ConfigPath string + // Verbose enables verbose output. + Verbose bool + // Global applies operations globally rather than per-project. + Global bool + // DryRun prints what would be done without making changes. + DryRun bool +} + +// CommandResult is the result of executing a CLI command. +type CommandResult struct { + // ExitCode is the process exit code (0 = success). + ExitCode int + // Output contains the command's stdout. + Output string + // Error contains any error message. + Error string +} + +// NewCommandContext returns a CommandContext with default values. +func NewCommandContext() *CommandContext { + return &CommandContext{} +} + +// IsSuccess returns true when ExitCode == 0. +func (r *CommandResult) IsSuccess() bool { + return r.ExitCode == 0 +} diff --git a/internal/commands/commands_test.go b/internal/commands/commands_test.go new file mode 100644 index 00000000..442bf166 --- /dev/null +++ b/internal/commands/commands_test.go @@ -0,0 +1,105 @@ +package commands_test + +import ( + "testing" + + "github.com/githubnext/apm/internal/commands" +) + +// TestParityCommandContextFields verifies field parity with Python CommandContext. +func TestParityCommandContextFields(t *testing.T) { + ctx := commands.NewCommandContext() + if ctx.Verbose { + t.Fatal("expected Verbose=false") + } + if ctx.Global { + t.Fatal("expected Global=false") + } + if ctx.DryRun { + t.Fatal("expected DryRun=false") + } +} + +// TestParityCommandContextConfigPath verifies ConfigPath field. +func TestParityCommandContextConfigPath(t *testing.T) { + ctx := &commands.CommandContext{ConfigPath: "/tmp/apm.yml"} + if ctx.ConfigPath != "/tmp/apm.yml" { + t.Fatalf("unexpected ConfigPath: %s", ctx.ConfigPath) + } +} + +// TestParityCommandResultSuccess verifies IsSuccess. +func TestParityCommandResultSuccess(t *testing.T) { + r := &commands.CommandResult{ExitCode: 0} + if !r.IsSuccess() { + t.Fatal("expected IsSuccess for exit code 0") + } +} + +// TestParityCommandResultFailure verifies failure detection. +func TestParityCommandResultFailure(t *testing.T) { + r := &commands.CommandResult{ExitCode: 1, Error: "something failed"} + if r.IsSuccess() { + t.Fatal("expected !IsSuccess for exit code 1") + } +} + +// TestParityCommandResultOutput verifies Output field. +func TestParityCommandResultOutput(t *testing.T) { + r := &commands.CommandResult{ExitCode: 0, Output: "[+] Done"} + if r.Output != "[+] Done" { + t.Fatalf("unexpected output: %s", r.Output) + } +} + +// TestParityNewCommandContextDefaults verifies zero values. +func TestParityNewCommandContextDefaults(t *testing.T) { + ctx := commands.NewCommandContext() + if ctx == nil { + t.Fatal("NewCommandContext returned nil") + } + if ctx.ConfigPath != "" { + t.Fatalf("expected empty ConfigPath, got %s", ctx.ConfigPath) + } +} + +// TestParityCommandContextVerboseFlag verifies setting verbose. +func TestParityCommandContextVerboseFlag(t *testing.T) { + ctx := commands.NewCommandContext() + ctx.Verbose = true + if !ctx.Verbose { + t.Fatal("expected Verbose=true after set") + } +} + +// TestParityCommandContextDryRunFlag verifies DryRun field. +func TestParityCommandContextDryRunFlag(t *testing.T) { + ctx := &commands.CommandContext{DryRun: true} + if !ctx.DryRun { + t.Fatal("expected DryRun=true") + } +} + +// TestParityCommandContextGlobalFlag verifies Global field. +func TestParityCommandContextGlobalFlag(t *testing.T) { + ctx := &commands.CommandContext{Global: true} + if !ctx.Global { + t.Fatal("expected Global=true") + } +} + +// TestParityCommandResultErrorField verifies Error field. +func TestParityCommandResultErrorField(t *testing.T) { + r := &commands.CommandResult{ExitCode: 2, Error: "permission denied"} + if r.Error != "permission denied" { + t.Fatalf("unexpected error: %s", r.Error) + } +} + +// TestParityCommandResultExitCode verifies exit code field. +func TestParityCommandResultExitCode(t *testing.T) { + r := &commands.CommandResult{ExitCode: 42} + if r.ExitCode != 42 { + t.Fatalf("unexpected exit code: %d", r.ExitCode) + } +} diff --git a/internal/compilation/compilation.go b/internal/compilation/compilation.go new file mode 100644 index 00000000..8e8a2522 --- /dev/null +++ b/internal/compilation/compilation.go @@ -0,0 +1,66 @@ +// Package compilation provides compilation pipeline types and utilities for APM. +package compilation + +import ( + "crypto/sha256" + "fmt" + "strings" +) + +// BuildIDPlaceholder is the sentinel string inserted into compiled outputs. +const BuildIDPlaceholder = "" + +// ConstitutionMarkerBegin marks the start of an injected constitution block. +const ConstitutionMarkerBegin = "" + +// ConstitutionMarkerEnd marks the end of an injected constitution block. +const ConstitutionMarkerEnd = "" + +// ConstitutionRelativePath is the repo-root-relative path to the constitution file. +const ConstitutionRelativePath = ".specify/memory/constitution.md" + +// StabilizeBuildID replaces BuildIDPlaceholder in content with a deterministic +// 12-char SHA256 hash computed over the content with the placeholder line removed. +// +// Idempotent: returns content unchanged if no placeholder is present. +// Preserves a trailing newline when the input has one. +func StabilizeBuildID(content string) string { + lines := strings.Split(content, "\n") + + // Preserve trailing newline: splitlines leaves an empty last element + // if content ends with "\n". We track it but exclude from hash input. + trailingNL := strings.HasSuffix(content, "\n") + + idx := -1 + for i, line := range lines { + if line == BuildIDPlaceholder { + idx = i + break + } + } + if idx == -1 { + return content + } + + // Build hash input from all lines except the placeholder. + hashLines := make([]string, 0, len(lines)-1) + for i, line := range lines { + if i != idx { + hashLines = append(hashLines, line) + } + } + // Remove the empty trailing element that split adds for "\n"-terminated content. + if trailingNL && len(hashLines) > 0 && hashLines[len(hashLines)-1] == "" { + hashLines = hashLines[:len(hashLines)-1] + } + h := sha256.Sum256([]byte(strings.Join(hashLines, "\n"))) + buildID := fmt.Sprintf("%x", h)[:12] + + lines[idx] = fmt.Sprintf("", buildID) + + result := strings.Join(lines, "\n") + if trailingNL && !strings.HasSuffix(result, "\n") { + result += "\n" + } + return result +} diff --git a/internal/compilation/compilation_test.go b/internal/compilation/compilation_test.go new file mode 100644 index 00000000..09450b7c --- /dev/null +++ b/internal/compilation/compilation_test.go @@ -0,0 +1,128 @@ +package compilation_test + +import ( + "strings" + "testing" + + "github.com/githubnext/apm/internal/compilation" +) + +// TestParityBuildIDConstants verifies constant values match Python source. +func TestParityBuildIDConstants(t *testing.T) { + if compilation.BuildIDPlaceholder != "" { + t.Fatalf("unexpected BuildIDPlaceholder: %s", compilation.BuildIDPlaceholder) + } + if compilation.ConstitutionRelativePath != ".specify/memory/constitution.md" { + t.Fatalf("unexpected path: %s", compilation.ConstitutionRelativePath) + } +} + +// TestParityConstitutionMarkers verifies constitution marker constants. +func TestParityConstitutionMarkers(t *testing.T) { + if compilation.ConstitutionMarkerBegin != "" { + t.Fatalf("unexpected begin marker: %s", compilation.ConstitutionMarkerBegin) + } + if compilation.ConstitutionMarkerEnd != "" { + t.Fatalf("unexpected end marker: %s", compilation.ConstitutionMarkerEnd) + } +} + +// TestParityStabilizeBuildIDNoPlaceholder returns input unchanged when no placeholder. +func TestParityStabilizeBuildIDNoPlaceholder(t *testing.T) { + content := "# My doc\nSome content.\n" + got := compilation.StabilizeBuildID(content) + if got != content { + t.Fatalf("expected unchanged content, got: %s", got) + } +} + +// TestParityStabilizeBuildIDReplacesPlaceholder verifies placeholder is replaced. +func TestParityStabilizeBuildIDReplacesPlaceholder(t *testing.T) { + content := "# My doc\n" + compilation.BuildIDPlaceholder + "\nSome content.\n" + got := compilation.StabilizeBuildID(content) + if strings.Contains(got, "__BUILD_ID__") { + t.Fatal("placeholder was not replaced") + } + if !strings.Contains(got, "" + const prefix = "" + idx := strings.Index(got, prefix) + if idx == -1 { + t.Fatalf("no Build ID comment found in: %s", got) + } + start := idx + len(prefix) + end := strings.Index(got[start:], suffix) + if end == -1 { + t.Fatalf("malformed Build ID comment: %s", got) + } + hash := got[start : start+end] + if len(hash) != 12 { + t.Fatalf("expected 12-char hash, got %d chars: %s", len(hash), hash) + } +} + +// TestParityStabilizeBuildIDIdempotent verifies second call is idempotent. +func TestParityStabilizeBuildIDIdempotent(t *testing.T) { + content := "abc\n" + compilation.BuildIDPlaceholder + "\ndef\n" + once := compilation.StabilizeBuildID(content) + twice := compilation.StabilizeBuildID(once) + if once != twice { + t.Fatalf("not idempotent: %q vs %q", once, twice) + } +} + +// TestParityStabilizeBuildIDTrailingNewline verifies trailing newline preservation. +func TestParityStabilizeBuildIDTrailingNewline(t *testing.T) { + content := "# doc\n" + compilation.BuildIDPlaceholder + "\ncontent\n" + got := compilation.StabilizeBuildID(content) + if !strings.HasSuffix(got, "\n") { + t.Fatal("trailing newline was lost") + } +} + +// TestParityStabilizeBuildIDNoTrailingNewline verifies non-trailing-newline input. +func TestParityStabilizeBuildIDNoTrailingNewline(t *testing.T) { + content := "# doc\n" + compilation.BuildIDPlaceholder + got := compilation.StabilizeBuildID(content) + if strings.HasSuffix(got, "\n") { + t.Fatal("unexpected trailing newline added") + } +} + +// TestParityStabilizeBuildIDDifferentContentDifferentHash verifies hash changes. +func TestParityStabilizeBuildIDDifferentContentDifferentHash(t *testing.T) { + c1 := "aaa\n" + compilation.BuildIDPlaceholder + "\nbbb\n" + c2 := "xxx\n" + compilation.BuildIDPlaceholder + "\nyyy\n" + r1 := compilation.StabilizeBuildID(c1) + r2 := compilation.StabilizeBuildID(c2) + if r1 == r2 { + t.Fatal("different content should produce different hash") + } +} + +// TestParityStabilizeBuildIDEmptyContent handles edge case of just placeholder. +func TestParityStabilizeBuildIDEmptyContent(t *testing.T) { + got := compilation.StabilizeBuildID(compilation.BuildIDPlaceholder) + if strings.Contains(got, "__BUILD_ID__") { + t.Fatal("placeholder should be replaced even for minimal content") + } +} diff --git a/internal/integration/integration.go b/internal/integration/integration.go new file mode 100644 index 00000000..edac3173 --- /dev/null +++ b/internal/integration/integration.go @@ -0,0 +1,53 @@ +// Package integration provides file-level integrator interfaces and types for APM. +package integration + +import "errors" + +// IntegrationResult holds the outcome of a file-integration operation. +type IntegrationResult struct { + // FilesIntegrated is the number of files written. + FilesIntegrated int + // FilesUpdated is kept for CLI compat, always 0 today. + FilesUpdated int + // FilesSkipped is the number of files that were unchanged. + FilesSkipped int + // LinksResolved is the number of inter-file links resolved. + LinksResolved int + // ScriptsCopied is the number of hook scripts copied. + ScriptsCopied int + // SubSkillsPromoted is the number of sub-skills promoted. + SubSkillsPromoted int + // SkillCreated is true if a new skill was created. + SkillCreated bool + // FilesAdopted is the number of byte-identical files adopted silently. + FilesAdopted int +} + +// Integrator is the base interface for file-level integrators. +type Integrator interface { + // Integrate performs the integration and returns a result. + Integrate(opts IntegrateOptions) (IntegrationResult, error) + // Name returns the integrator's identifier. + Name() string +} + +// IntegrateOptions carries options for an integration run. +type IntegrateOptions struct { + // DryRun skips writing files to disk. + DryRun bool + // Force overwrites even byte-identical files. + Force bool + // Global applies the operation globally. + Global bool +} + +// ErrIntegrationConflict is returned when a file conflict cannot be resolved. +var ErrIntegrationConflict = errors.New("integration conflict") + +// ErrIntegrationSkipped is returned when integration was intentionally skipped. +var ErrIntegrationSkipped = errors.New("integration skipped") + +// Total returns the total number of files touched (integrated + skipped + adopted). +func (r IntegrationResult) Total() int { + return r.FilesIntegrated + r.FilesSkipped + r.FilesAdopted +} diff --git a/internal/integration/integration_test.go b/internal/integration/integration_test.go new file mode 100644 index 00000000..7c01782a --- /dev/null +++ b/internal/integration/integration_test.go @@ -0,0 +1,104 @@ +package integration_test + +import ( + "testing" + + "github.com/githubnext/apm/internal/integration" +) + +// TestParityIntegrationResultFields verifies field parity with Python IntegrationResult. +func TestParityIntegrationResultFields(t *testing.T) { + r := integration.IntegrationResult{ + FilesIntegrated: 3, + FilesSkipped: 1, + FilesAdopted: 2, + LinksResolved: 5, + } + if r.FilesIntegrated != 3 { + t.Fatalf("expected 3, got %d", r.FilesIntegrated) + } + if r.FilesSkipped != 1 { + t.Fatalf("expected 1 skipped, got %d", r.FilesSkipped) + } +} + +// TestParityIntegrationResultTotal verifies Total() helper. +func TestParityIntegrationResultTotal(t *testing.T) { + r := integration.IntegrationResult{ + FilesIntegrated: 3, + FilesSkipped: 1, + FilesAdopted: 2, + } + if r.Total() != 6 { + t.Fatalf("expected total 6, got %d", r.Total()) + } +} + +// TestParityIntegrationResultZero verifies zero value is valid. +func TestParityIntegrationResultZero(t *testing.T) { + var r integration.IntegrationResult + if r.Total() != 0 { + t.Fatalf("expected 0, got %d", r.Total()) + } +} + +// TestParityIntegrationErrors verifies sentinel errors. +func TestParityIntegrationErrors(t *testing.T) { + if integration.ErrIntegrationConflict == nil { + t.Fatal("ErrIntegrationConflict should not be nil") + } + if integration.ErrIntegrationSkipped == nil { + t.Fatal("ErrIntegrationSkipped should not be nil") + } +} + +// TestParityIntegrateOptionsFields verifies option struct. +func TestParityIntegrateOptionsFields(t *testing.T) { + opts := integration.IntegrateOptions{DryRun: true, Force: false, Global: true} + if !opts.DryRun { + t.Fatal("expected DryRun=true") + } + if !opts.Global { + t.Fatal("expected Global=true") + } +} + +// TestParityIntegratorInterface verifies the Integrator interface. +func TestParityIntegratorInterface(t *testing.T) { + var _ integration.Integrator = (*mockIntegrator)(nil) +} + +type mockIntegrator struct{} + +func (m *mockIntegrator) Integrate(opts integration.IntegrateOptions) (integration.IntegrationResult, error) { + return integration.IntegrationResult{FilesIntegrated: 1}, nil +} +func (m *mockIntegrator) Name() string { return "mock" } + +// TestParityIntegratorMockRun verifies the interface can be called. +func TestParityIntegratorMockRun(t *testing.T) { + var i integration.Integrator = &mockIntegrator{} + result, err := i.Integrate(integration.IntegrateOptions{}) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if result.FilesIntegrated != 1 { + t.Fatalf("expected 1 file integrated, got %d", result.FilesIntegrated) + } +} + +// TestParityIntegrationResultSkillCreated verifies SkillCreated field. +func TestParityIntegrationResultSkillCreated(t *testing.T) { + r := integration.IntegrationResult{SkillCreated: true} + if !r.SkillCreated { + t.Fatal("expected SkillCreated=true") + } +} + +// TestParityIntegrationResultSubSkills verifies SubSkillsPromoted field. +func TestParityIntegrationResultSubSkills(t *testing.T) { + r := integration.IntegrationResult{SubSkillsPromoted: 3} + if r.SubSkillsPromoted != 3 { + t.Fatalf("expected 3, got %d", r.SubSkillsPromoted) + } +} diff --git a/internal/runtime/factory.go b/internal/runtime/factory.go new file mode 100644 index 00000000..5410af34 --- /dev/null +++ b/internal/runtime/factory.go @@ -0,0 +1,69 @@ +package runtime + +import ( + "fmt" + "os/exec" +) + +// knownAdapterNames lists the order of preference for runtime adapters. +var knownAdapterNames = []string{"copilot", "codex", "llm"} + +// GetAvailableRuntimes returns info for each runtime that is installed on PATH or ~/.apm/runtimes/. +func GetAvailableRuntimes() []RuntimeInfo { + var available []RuntimeInfo + for _, name := range knownAdapterNames { + entry, ok := SupportedRuntimes[name] + if !ok { + continue + } + path, _ := FindRuntimeBinary(entry.Binary) + if path == "" { + continue + } + available = append(available, RuntimeInfo{ + Name: name, + Available: true, + Description: entry.Description, + }) + } + return available +} + +// GetRuntimeByName returns a RuntimeInfo for a named runtime, or an error if +// not found or not available. +func GetRuntimeByName(name string) (RuntimeInfo, error) { + entry, ok := SupportedRuntimes[name] + if !ok { + return RuntimeInfo{}, fmt.Errorf("%w: %s", ErrRuntimeNotFound, name) + } + path, _ := FindRuntimeBinary(entry.Binary) + if path == "" { + return RuntimeInfo{}, fmt.Errorf("%w: %s", ErrRuntimeUnavailable, name) + } + return RuntimeInfo{ + Name: name, + Available: true, + Description: entry.Description, + }, nil +} + +// GetBestAvailableRuntime returns the highest-preference available runtime. +func GetBestAvailableRuntime() (RuntimeInfo, error) { + for _, name := range knownAdapterNames { + info, err := GetRuntimeByName(name) + if err == nil { + return info, nil + } + } + return RuntimeInfo{}, fmt.Errorf("no supported runtime found (tried: copilot, codex, llm)") +} + +// IsRuntimeBinaryAvailable returns true if the given runtime binary is reachable. +func IsRuntimeBinaryAvailable(binaryName string) bool { + p, _ := FindRuntimeBinary(binaryName) + if p != "" { + return true + } + _, err := exec.LookPath(binaryName) + return err == nil +} diff --git a/internal/runtime/manager.go b/internal/runtime/manager.go new file mode 100644 index 00000000..4df09970 --- /dev/null +++ b/internal/runtime/manager.go @@ -0,0 +1,49 @@ +package runtime + +import "strings" + +// SupportedRuntimeEntry describes a supported AI runtime. +type SupportedRuntimeEntry struct { + Script string + Description string + Binary string +} + +// SupportedRuntimes is the registry of known runtimes. +var SupportedRuntimes = map[string]SupportedRuntimeEntry{ + "copilot": { + Script: "setup-copilot", + Description: "GitHub Copilot CLI with native MCP integration", + Binary: "copilot", + }, + "codex": { + Script: "setup-codex", + Description: "OpenAI Codex CLI with GitHub Models support", + Binary: "codex", + }, + "llm": { + Script: "setup-llm", + Description: "Simon Willison's LLM library with multiple providers", + Binary: "llm", + }, + "gemini": { + Script: "setup-gemini", + Description: "Google Gemini CLI with MCP integration", + Binary: "gemini", + }, +} + +// IsKnownRuntime returns true if name is a known runtime. +func IsKnownRuntime(name string) bool { + _, ok := SupportedRuntimes[strings.ToLower(name)] + return ok +} + +// GetSupportedRuntimeNames returns a sorted list of known runtime names. +func GetSupportedRuntimeNames() []string { + names := make([]string, 0, len(SupportedRuntimes)) + for k := range SupportedRuntimes { + names = append(names, k) + } + return names +} diff --git a/internal/runtime/runtime.go b/internal/runtime/runtime.go new file mode 100644 index 00000000..6d82f7cb --- /dev/null +++ b/internal/runtime/runtime.go @@ -0,0 +1,35 @@ +// Package runtime provides runtime adapter interfaces and types for APM. +package runtime + +import "errors" + +// RuntimeAdapter is the base interface for LLM runtime adapters. +type RuntimeAdapter interface { + // ExecutePrompt executes a single prompt and returns the response. + ExecutePrompt(promptContent string, opts map[string]interface{}) (string, error) + // ListAvailableModels lists all available models in the runtime. + ListAvailableModels() (map[string]interface{}, error) + // GetRuntimeInfo returns information about this runtime. + GetRuntimeInfo() map[string]interface{} + // IsAvailable checks if this runtime is available on the system. + IsAvailable() bool + // GetRuntimeName returns the name of this runtime. + GetRuntimeName() string +} + +// ErrRuntimeNotFound is returned when a requested runtime cannot be found. +var ErrRuntimeNotFound = errors.New("runtime not found") + +// ErrRuntimeUnavailable is returned when a runtime is found but not available. +var ErrRuntimeUnavailable = errors.New("runtime not available") + +// RuntimeInfo holds metadata about a runtime adapter. +type RuntimeInfo struct { + Name string `json:"name"` + Type string `json:"type"` + Available bool `json:"available"` + CurrentModel string `json:"current_model,omitempty"` + Capabilities map[string]interface{} `json:"capabilities,omitempty"` + Description string `json:"description,omitempty"` + Error string `json:"error,omitempty"` +} diff --git a/internal/runtime/runtime_test.go b/internal/runtime/runtime_test.go new file mode 100644 index 00000000..6df53f33 --- /dev/null +++ b/internal/runtime/runtime_test.go @@ -0,0 +1,128 @@ +package runtime_test + +import ( + "strings" + "testing" + + "github.com/githubnext/apm/internal/runtime" +) + +// TestParityRuntimeAdapterInterface verifies the RuntimeAdapter interface is defined. +func TestParityRuntimeAdapterInterface(t *testing.T) { + // RuntimeAdapter is an interface -- verify it exists as a type + var _ runtime.RuntimeAdapter = (*mockAdapter)(nil) +} + +type mockAdapter struct{} + +func (m *mockAdapter) ExecutePrompt(prompt string, opts map[string]interface{}) (string, error) { + return "ok", nil +} +func (m *mockAdapter) ListAvailableModels() (map[string]interface{}, error) { + return map[string]interface{}{"default": true}, nil +} +func (m *mockAdapter) GetRuntimeInfo() map[string]interface{} { + return map[string]interface{}{"name": "mock"} +} +func (m *mockAdapter) IsAvailable() bool { return true } +func (m *mockAdapter) GetRuntimeName() string { return "mock" } + +// TestParityRuntimeErrors verifies sentinel errors exist. +func TestParityRuntimeErrors(t *testing.T) { + if runtime.ErrRuntimeNotFound == nil { + t.Fatal("ErrRuntimeNotFound should not be nil") + } + if runtime.ErrRuntimeUnavailable == nil { + t.Fatal("ErrRuntimeUnavailable should not be nil") + } +} + +// TestParityRuntimeInfo verifies RuntimeInfo struct fields. +func TestParityRuntimeInfo(t *testing.T) { + info := runtime.RuntimeInfo{ + Name: "llm", + Available: true, + Type: "llm_library", + } + if info.Name != "llm" { + t.Fatalf("expected llm got %s", info.Name) + } + if !info.Available { + t.Fatal("expected Available=true") + } +} + +// TestParitySupportedRuntimes verifies the four known runtimes are registered. +func TestParitySupportedRuntimes(t *testing.T) { + for _, name := range []string{"copilot", "codex", "llm", "gemini"} { + if !runtime.IsKnownRuntime(name) { + t.Errorf("expected %s to be a known runtime", name) + } + } +} + +// TestParityIsKnownRuntimeCaseInsensitive verifies case-insensitive lookup. +func TestParityIsKnownRuntimeCaseInsensitive(t *testing.T) { + if !runtime.IsKnownRuntime("LLM") { + t.Fatal("IsKnownRuntime should be case-insensitive") + } +} + +// TestParityGetSupportedRuntimeNames verifies names are returned. +func TestParityGetSupportedRuntimeNames(t *testing.T) { + names := runtime.GetSupportedRuntimeNames() + if len(names) < 4 { + t.Fatalf("expected at least 4 runtime names, got %d", len(names)) + } +} + +// TestParityFindRuntimeBinaryRejectsPathTraversal verifies security guard. +func TestParityFindRuntimeBinaryRejectsPathTraversal(t *testing.T) { + _, err := runtime.FindRuntimeBinary("../etc/passwd") + if err == nil { + t.Fatal("expected error for path-traversal name") + } + if !strings.Contains(err.Error(), "path separator") { + t.Fatalf("unexpected error: %v", err) + } +} + +// TestParityFindRuntimeBinaryRejectsEmpty verifies empty name is rejected. +func TestParityFindRuntimeBinaryRejectsEmpty(t *testing.T) { + _, err := runtime.FindRuntimeBinary("") + if err == nil { + t.Fatal("expected error for empty name") + } +} + +// TestParityFindRuntimeBinaryUnknown returns empty string (not error) for unknown. +func TestParityFindRuntimeBinaryUnknown(t *testing.T) { + path, err := runtime.FindRuntimeBinary("definitely-not-a-real-binary-apm-test") + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if path != "" { + t.Fatalf("expected empty path for unknown binary, got %s", path) + } +} + +// TestParityGetAvailableRuntimes returns a slice (may be empty in CI). +func TestParityGetAvailableRuntimes(t *testing.T) { + infos := runtime.GetAvailableRuntimes() + _ = infos // may be empty in CI -- just verify it doesn't panic +} + +// TestParityGetRuntimeByNameUnknown returns ErrRuntimeNotFound. +func TestParityGetRuntimeByNameUnknown(t *testing.T) { + _, err := runtime.GetRuntimeByName("nonexistent-runtime") + if err == nil { + t.Fatal("expected error for unknown runtime") + } +} + +// TestParityIsRuntimeBinaryAvailableFalse returns false for nonexistent binary. +func TestParityIsRuntimeBinaryAvailableFalse(t *testing.T) { + if runtime.IsRuntimeBinaryAvailable("definitely-not-installed-apm-test") { + t.Fatal("expected false for unavailable binary") + } +} diff --git a/internal/runtime/utils.go b/internal/runtime/utils.go new file mode 100644 index 00000000..aef58116 --- /dev/null +++ b/internal/runtime/utils.go @@ -0,0 +1,96 @@ +package runtime + +import ( + "fmt" + "os" + "os/exec" + "path/filepath" + "runtime" +) + +// FindRuntimeBinary resolves the path to a runtime binary. +// Priority: +// 1. ~/.apm/runtimes/ (APM-managed, executable) +// 2. PATH lookup via exec.LookPath +// +// Returns empty string if not found. +// Returns error if name contains path-traversal sequences. +func FindRuntimeBinary(name string) (string, error) { + // Security: reject names with path separators + for _, ch := range []string{"/", "\\"} { + if containsStr(name, ch) { + return "", fmt.Errorf("invalid runtime name %q: must be a plain binary name without path separators", name) + } + } + if name == "" || name == "." || name == ".." { + return "", fmt.Errorf("invalid runtime name %q", name) + } + + homeDir, err := os.UserHomeDir() + if err == nil { + apmRuntimes := filepath.Join(homeDir, ".apm", "runtimes") + if runtime.GOOS == "windows" { + candidate := filepath.Join(apmRuntimes, name+".exe") + if isExecutableFile(candidate, apmRuntimes) { + return candidate, nil + } + } + candidate := filepath.Join(apmRuntimes, name) + if isExecutableFile(candidate, apmRuntimes) { + return candidate, nil + } + } + + // PATH fallback + p, err := exec.LookPath(name) + if err != nil { + return "", nil + } + return p, nil +} + +func isExecutableFile(path, base string) bool { + // Ensure path is within base (security guard) + abs, err := filepath.Abs(path) + if err != nil { + return false + } + absBase, err := filepath.Abs(base) + if err != nil { + return false + } + rel, err := filepath.Rel(absBase, abs) + if err != nil { + return false + } + if len(rel) >= 2 && rel[:2] == ".." { + return false + } + info, err := os.Stat(path) + if err != nil { + return false + } + if info.IsDir() { + return false + } + // Check executable bit (non-Windows) + if runtime.GOOS != "windows" { + if info.Mode()&0o111 == 0 { + return false + } + } + return true +} + +func containsStr(s, sub string) bool { + return len(sub) > 0 && len(s) >= len(sub) && (s == sub || len(s) > 0 && containsRune(s, rune(sub[0]))) +} + +func containsRune(s string, r rune) bool { + for _, c := range s { + if c == r { + return true + } + } + return false +} From 7e4f2823c7096a50ec754b447d4ddcbf501ca84d Mon Sep 17 00:00:00 2001 From: "github-actions[bot]" Date: Wed, 27 May 2026 04:11:37 +0000 Subject: [PATCH 08/10] ci: trigger checks From 7dbed8b27dfe8318f6a49852b76b74243b0d554f Mon Sep 17 00:00:00 2001 From: "github-actions[bot]" <41898282+github-actions[bot]@users.noreply.github.com> Date: Wed, 27 May 2026 05:58:46 +0000 Subject: [PATCH 09/10] [Crane: crane-migration-python-to-go-full-apm-cli-rewrite] Iteration 18: Port policy/ + security/ -- 304 parity tests, score 1.0 Run: https://github.com/githubnext/apm/actions/runs/26493354341 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- internal/policy/matcher.go | 34 +++++ internal/policy/models.go | 93 ++++++++++++ internal/policy/policy_test.go | 236 +++++++++++++++++++++++++++++ internal/policy/schema.go | 129 ++++++++++++++++ internal/security/gate.go | 120 +++++++++++++++ internal/security/security_test.go | 158 +++++++++++++++++++ 6 files changed, 770 insertions(+) create mode 100644 internal/policy/matcher.go create mode 100644 internal/policy/models.go create mode 100644 internal/policy/policy_test.go create mode 100644 internal/policy/schema.go create mode 100644 internal/security/gate.go create mode 100644 internal/security/security_test.go diff --git a/internal/policy/matcher.go b/internal/policy/matcher.go new file mode 100644 index 00000000..b5cf07d3 --- /dev/null +++ b/internal/policy/matcher.go @@ -0,0 +1,34 @@ +// Package policy -- glob-style pattern matcher for policy allow/deny lists. +// Mirrors Python apm_cli.policy.matcher. +package policy + +import "path/filepath" + +// MatchesAny returns true if the candidate matches any of the given glob patterns. +// Patterns use filepath.Match semantics (shell glob, no path separator in *). +func MatchesAny(candidate string, patterns []string) bool { + for _, p := range patterns { + if ok, _ := filepath.Match(p, candidate); ok { + return true + } + } + return false +} + +// MatchesAllowList returns true if candidate is permitted by the allowlist. +// nil allowlist = "no opinion" (allow all). Empty allowlist = "allow nothing". +func MatchesAllowList(candidate string, allow []string) bool { + if allow == nil { + return true + } + return MatchesAny(candidate, allow) +} + +// MatchesDenyList returns true if candidate is blocked by the denylist. +// nil denylist = "no opinion" (deny nothing). Empty = deny nothing. +func MatchesDenyList(candidate string, deny []string) bool { + if deny == nil || len(deny) == 0 { + return false + } + return MatchesAny(candidate, deny) +} diff --git a/internal/policy/models.go b/internal/policy/models.go new file mode 100644 index 00000000..54109812 --- /dev/null +++ b/internal/policy/models.go @@ -0,0 +1,93 @@ +// Package policy provides data models for CI/policy audit checks. +// It mirrors the Python apm_cli.policy.models module. +package policy + +// CheckResult is the result of a single CI check. +type CheckResult struct { + Name string // e.g., "lockfile-exists" + Passed bool + Message string // human-readable description + Details []string // individual violations +} + +// CIAuditResult is the aggregate result of all CI checks. +type CIAuditResult struct { + Checks []CheckResult +} + +// Passed returns true if all checks passed. +func (r *CIAuditResult) Passed() bool { + for _, c := range r.Checks { + if !c.Passed { + return false + } + } + return true +} + +// FailedChecks returns only the checks that did not pass. +func (r *CIAuditResult) FailedChecks() []CheckResult { + var out []CheckResult + for _, c := range r.Checks { + if !c.Passed { + out = append(out, c) + } + } + return out +} + +// ToJSON serializes to a JSON-compatible map. +func (r *CIAuditResult) ToJSON() map[string]interface{} { + checks := make([]map[string]interface{}, len(r.Checks)) + passed, failed := 0, 0 + for i, c := range r.Checks { + checks[i] = map[string]interface{}{ + "name": c.Name, + "passed": c.Passed, + "message": c.Message, + "details": c.Details, + } + if c.Passed { + passed++ + } else { + failed++ + } + } + return map[string]interface{}{ + "passed": r.Passed(), + "checks": checks, + "summary": map[string]interface{}{ + "total": len(r.Checks), + "passed": passed, + "failed": failed, + }, + } +} + +// CheckArtifactMap maps check names to their most relevant artifact. +var CheckArtifactMap = map[string]string{ + "lockfile-exists": "apm.lock.yaml", + "ref-consistency": "apm.lock.yaml", + "deployed-files-present": "apm.lock.yaml", + "no-orphaned-packages": "apm.lock.yaml", + "config-consistency": "apm.lock.yaml", + "content-integrity": "apm.lock.yaml", + "dependency-allowlist": "apm.yml", + "dependency-denylist": "apm.yml", + "required-packages": "apm.yml", + "required-packages-deployed": "apm.lock.yaml", + "required-package-version": "apm.lock.yaml", + "transitive-depth": "apm.lock.yaml", + "mcp-allowlist": "apm.yml", + "mcp-denylist": "apm.yml", + "mcp-transport": "apm.yml", + "mcp-self-defined": "apm.yml", + "compilation-target": "apm.yml", + "compilation-strategy": "apm.yml", + "source-attribution": "apm.yml", + "required-manifest-fields": "apm.yml", + "scripts-policy": "apm.yml", + "unmanaged-files": "apm.yml", + "manifest-parse": "apm.yml", + "manifest-missing": "apm.yml", +} diff --git a/internal/policy/policy_test.go b/internal/policy/policy_test.go new file mode 100644 index 00000000..17527ec2 --- /dev/null +++ b/internal/policy/policy_test.go @@ -0,0 +1,236 @@ +package policy_test + +import ( + "testing" + + "github.com/githubnext/apm/internal/policy" +) + +// TestParityCheckResult validates CheckResult structure mirrors Python models.CheckResult. +func TestParityCheckResult(t *testing.T) { + cr := policy.CheckResult{ + Name: "lockfile-exists", + Passed: true, + Message: "Lockfile found", + Details: []string{}, + } + if cr.Name != "lockfile-exists" { + t.Errorf("expected name lockfile-exists, got %s", cr.Name) + } + if !cr.Passed { + t.Errorf("expected passed=true") + } +} + +// TestParityCIAuditResultPassed verifies all-pass aggregate. +func TestParityCIAuditResultPassed(t *testing.T) { + r := &policy.CIAuditResult{ + Checks: []policy.CheckResult{ + {Name: "lockfile-exists", Passed: true, Message: "ok"}, + {Name: "ref-consistency", Passed: true, Message: "ok"}, + }, + } + if !r.Passed() { + t.Errorf("expected all checks to pass") + } + if len(r.FailedChecks()) != 0 { + t.Errorf("expected 0 failed checks") + } +} + +// TestParityCIAuditResultFailed verifies failed checks aggregation. +func TestParityCIAuditResultFailed(t *testing.T) { + r := &policy.CIAuditResult{ + Checks: []policy.CheckResult{ + {Name: "lockfile-exists", Passed: true, Message: "ok"}, + {Name: "ref-consistency", Passed: false, Message: "mismatch", Details: []string{"sha mismatch on pkg-a"}}, + }, + } + if r.Passed() { + t.Errorf("expected not all passed") + } + failed := r.FailedChecks() + if len(failed) != 1 { + t.Errorf("expected 1 failed check, got %d", len(failed)) + } + if failed[0].Name != "ref-consistency" { + t.Errorf("expected ref-consistency to fail") + } +} + +// TestParityCIAuditResultToJSON verifies JSON serialization mirrors Python. +func TestParityCIAuditResultToJSON(t *testing.T) { + r := &policy.CIAuditResult{ + Checks: []policy.CheckResult{ + {Name: "lockfile-exists", Passed: true, Message: "ok", Details: []string{}}, + {Name: "ref-consistency", Passed: false, Message: "mismatch", Details: []string{"detail1"}}, + }, + } + j := r.ToJSON() + if j["passed"] != false { + t.Errorf("expected passed=false") + } + summary, ok := j["summary"].(map[string]interface{}) + if !ok { + t.Fatalf("summary not a map") + } + if summary["total"] != 2 { + t.Errorf("expected total=2, got %v", summary["total"]) + } + if summary["passed"] != 1 { + t.Errorf("expected passed=1") + } + if summary["failed"] != 1 { + t.Errorf("expected failed=1") + } +} + +// TestParityCheckArtifactMap validates the artifact mapping mirrors Python. +func TestParityCheckArtifactMap(t *testing.T) { + cases := map[string]string{ + "lockfile-exists": "apm.lock.yaml", + "dependency-allowlist": "apm.yml", + "mcp-transport": "apm.yml", + "ref-consistency": "apm.lock.yaml", + "compilation-target": "apm.yml", + "required-manifest-fields": "apm.yml", + } + for check, want := range cases { + got, ok := policy.CheckArtifactMap[check] + if !ok { + t.Errorf("check %s missing from artifact map", check) + continue + } + if got != want { + t.Errorf("check %s: want %s, got %s", check, want, got) + } + } +} + +// TestParityDefaultDependencyPolicy mirrors Python DependencyPolicy defaults. +func TestParityDefaultDependencyPolicy(t *testing.T) { + dp := policy.DefaultDependencyPolicy() + if dp.MaxDepth != 50 { + t.Errorf("expected MaxDepth=50, got %d", dp.MaxDepth) + } + if dp.RequireResolution != policy.RequireResolutionProjectWins { + t.Errorf("expected project-wins resolution") + } +} + +// TestParityDependencyPolicyEffectiveDeny verifies nil -> empty semantics. +func TestParityDependencyPolicyEffectiveDeny(t *testing.T) { + dp := policy.DependencyPolicy{Deny: nil} + if len(dp.EffectiveDeny()) != 0 { + t.Errorf("nil deny should return empty slice") + } + dp2 := policy.DependencyPolicy{Deny: []string{"bad-pkg"}} + if len(dp2.EffectiveDeny()) != 1 { + t.Errorf("expected 1 deny entry") + } +} + +// TestParityDependencyPolicyEffectiveRequire mirrors Python require semantics. +func TestParityDependencyPolicyEffectiveRequire(t *testing.T) { + dp := policy.DependencyPolicy{Require: nil} + if len(dp.EffectiveRequire()) != 0 { + t.Errorf("nil require should return empty slice") + } + dp2 := policy.DependencyPolicy{Require: []string{"required-pkg"}} + if len(dp2.EffectiveRequire()) != 1 { + t.Errorf("expected 1 require entry") + } +} + +// TestParityDefaultPolicyCache verifies TTL default of 3600. +func TestParityDefaultPolicyCache(t *testing.T) { + c := policy.DefaultPolicyCache() + if c.TTL != 3600 { + t.Errorf("expected TTL=3600, got %d", c.TTL) + } +} + +// TestParityDefaultOutcomeRouting verifies block-by-default routing. +func TestParityDefaultOutcomeRouting(t *testing.T) { + o := policy.DefaultOutcomeRouting() + if o.Default != policy.OutcomeBlock { + t.Errorf("expected default outcome=block") + } +} + +// TestParityOutcomeRoutingActionFor verifies per-check routing overrides. +func TestParityOutcomeRoutingActionFor(t *testing.T) { + o := policy.OutcomeRouting{ + Default: policy.OutcomeBlock, + Checks: map[string]policy.OutcomeAction{ + "lockfile-exists": policy.OutcomeWarn, + }, + } + if o.ActionFor("lockfile-exists") != policy.OutcomeWarn { + t.Errorf("expected warn for lockfile-exists") + } + if o.ActionFor("ref-consistency") != policy.OutcomeBlock { + t.Errorf("expected block for unknown check") + } +} + +// TestParityDefaultPolicyDocument verifies all defaults are set. +func TestParityDefaultPolicyDocument(t *testing.T) { + doc := policy.DefaultPolicyDocument() + if doc.Cache.TTL != 3600 { + t.Errorf("expected cache TTL=3600") + } + if doc.Dependencies.MaxDepth != 50 { + t.Errorf("expected deps MaxDepth=50") + } + if doc.Outcomes.Default != policy.OutcomeBlock { + t.Errorf("expected outcomes default=block") + } +} + +// TestParityMatcherMatchesAny verifies glob matching behavior. +func TestParityMatcherMatchesAny(t *testing.T) { + cases := []struct { + candidate string + patterns []string + want bool + }{ + {"pkg-a", []string{"pkg-*"}, true}, + {"pkg-a", []string{"other-*"}, false}, + {"pkg-a", []string{"pkg-b", "pkg-a"}, true}, + {"pkg-a", []string{}, false}, + {"anything", []string{"*"}, true}, + } + for _, tc := range cases { + got := policy.MatchesAny(tc.candidate, tc.patterns) + if got != tc.want { + t.Errorf("MatchesAny(%q, %v) = %v, want %v", tc.candidate, tc.patterns, got, tc.want) + } + } +} + +// TestParityMatcherAllowList verifies nil = allow-all semantics. +func TestParityMatcherAllowList(t *testing.T) { + if !policy.MatchesAllowList("anything", nil) { + t.Errorf("nil allowlist should permit everything") + } + if policy.MatchesAllowList("pkg-a", []string{}) { + t.Errorf("empty allowlist should permit nothing") + } + if !policy.MatchesAllowList("pkg-a", []string{"pkg-*"}) { + t.Errorf("pkg-a should match pkg-*") + } +} + +// TestParityMatcherDenyList verifies nil = deny-nothing semantics. +func TestParityMatcherDenyList(t *testing.T) { + if policy.MatchesDenyList("pkg-a", nil) { + t.Errorf("nil denylist should deny nothing") + } + if policy.MatchesDenyList("pkg-a", []string{}) { + t.Errorf("empty denylist should deny nothing") + } + if !policy.MatchesDenyList("bad-pkg", []string{"bad-*"}) { + t.Errorf("bad-pkg should match bad-*") + } +} diff --git a/internal/policy/schema.go b/internal/policy/schema.go new file mode 100644 index 00000000..b6840092 --- /dev/null +++ b/internal/policy/schema.go @@ -0,0 +1,129 @@ +// Package policy provides schema types for apm-policy.yml. +// It mirrors the Python apm_cli.policy.schema module. +package policy + +// PolicyCache holds cache configuration for remote policy resolution. +type PolicyCache struct { + TTL int // seconds, default 3600 +} + +// DefaultPolicyCache returns the default cache config. +func DefaultPolicyCache() PolicyCache { + return PolicyCache{TTL: 3600} +} + +// RequireResolution is the resolution mode for required packages. +type RequireResolution string + +const ( + RequireResolutionProjectWins RequireResolution = "project-wins" + RequireResolutionPolicyWins RequireResolution = "policy-wins" + RequireResolutionBlock RequireResolution = "block" +) + +// DependencyPolicy rules governing which APM dependencies are permitted. +type DependencyPolicy struct { + Allow []string // nil = no opinion; empty = explicitly nothing + Deny []string // nil = no opinion; empty = explicit empty + Require []string // nil = no opinion; empty = explicit empty + RequireResolution RequireResolution // default: project-wins + MaxDepth int // default: 50 +} + +// DefaultDependencyPolicy returns the default dependency policy. +func DefaultDependencyPolicy() DependencyPolicy { + return DependencyPolicy{ + RequireResolution: RequireResolutionProjectWins, + MaxDepth: 50, + } +} + +// EffectiveDeny returns the resolved deny list (nil -> empty slice). +func (d DependencyPolicy) EffectiveDeny() []string { + if d.Deny == nil { + return []string{} + } + return d.Deny +} + +// EffectiveRequire returns the resolved require list (nil -> empty slice). +func (d DependencyPolicy) EffectiveRequire() []string { + if d.Require == nil { + return []string{} + } + return d.Require +} + +// McpTransportPolicy describes allowed MCP transport protocols. +type McpTransportPolicy struct { + Allow []string // e.g. stdio, sse, http, streamable-http +} + +// McpPolicy rules governing MCP server references. +type McpPolicy struct { + Allow []string // nil = no opinion + Deny []string // nil = no opinion + Transport McpTransportPolicy + AllowSelf *bool // nil = no opinion +} + +// CompilationPolicy governs compilation targets and strategies. +type CompilationPolicy struct { + AllowedTargets []string // nil = no opinion + AllowedStrategies []string // nil = no opinion +} + +// ScriptsPolicy governs allowed/denied scripts. +type ScriptsPolicy struct { + Allow []string // nil = no opinion + Deny []string // nil = no opinion +} + +// OutcomeAction is the action taken when a policy check fails. +type OutcomeAction string + +const ( + OutcomeBlock OutcomeAction = "block" + OutcomeWarn OutcomeAction = "warn" + OutcomeAllow OutcomeAction = "allow" +) + +// OutcomeRouting maps check names to their outcome action. +type OutcomeRouting struct { + Default OutcomeAction + Checks map[string]OutcomeAction +} + +// DefaultOutcomeRouting returns routing that blocks on failure. +func DefaultOutcomeRouting() OutcomeRouting { + return OutcomeRouting{Default: OutcomeBlock} +} + +// ActionFor returns the outcome action for a given check name. +func (o OutcomeRouting) ActionFor(checkName string) OutcomeAction { + if action, ok := o.Checks[checkName]; ok { + return action + } + return o.Default +} + +// PolicyDocument is the top-level apm-policy.yml structure. +type PolicyDocument struct { + Version string + Extends []string + Cache PolicyCache + Dependencies DependencyPolicy + Mcp McpPolicy + Compilation CompilationPolicy + Scripts ScriptsPolicy + Outcomes OutcomeRouting +} + +// DefaultPolicyDocument returns a policy document with all defaults. +func DefaultPolicyDocument() PolicyDocument { + return PolicyDocument{ + Cache: DefaultPolicyCache(), + Dependencies: DefaultDependencyPolicy(), + Outcomes: DefaultOutcomeRouting(), + } +} diff --git a/internal/security/gate.go b/internal/security/gate.go new file mode 100644 index 00000000..793537a3 --- /dev/null +++ b/internal/security/gate.go @@ -0,0 +1,120 @@ +// Package security provides types for the security scanning gate. +// Mirrors Python apm_cli.security.gate. +package security + +// ScanFinding represents a single security finding from content scanning. +type ScanFinding struct { + Rule string // rule identifier + Severity string // "critical", "warning", "info" + Message string // human-readable description + Line int // line number (0 = unknown) + Column int // column (0 = unknown) +} + +// ScanPolicy declares how a command handles security findings. +type ScanPolicy struct { + OnCritical string // "block", "warn", "ignore" + ForceOverrides bool // when true, --force downgrades block to warn +} + +// BlockPolicy blocks on critical findings; force can override. +var BlockPolicy = ScanPolicy{OnCritical: "block", ForceOverrides: true} + +// WarnPolicy warns on critical findings; force has no effect. +var WarnPolicy = ScanPolicy{OnCritical: "warn", ForceOverrides: false} + +// ReportPolicy collects findings silently. +var ReportPolicy = ScanPolicy{OnCritical: "ignore", ForceOverrides: false} + +// EffectiveBlock returns true when this policy would block deployment. +func (p ScanPolicy) EffectiveBlock(force bool) bool { + return p.OnCritical == "block" && !(p.ForceOverrides && force) +} + +// ScanVerdict is the result of a SecurityGate check. +type ScanVerdict struct { + FindingsByFile map[string][]ScanFinding + HasCritical bool + ShouldBlock bool + CriticalCount int + WarningCount int + FilesScanned int +} + +// HasFindings returns true if any file has findings. +func (v *ScanVerdict) HasFindings() bool { + return len(v.FindingsByFile) > 0 +} + +// AllFindings returns a flat list of all findings across files. +func (v *ScanVerdict) AllFindings() []ScanFinding { + var out []ScanFinding + for _, findings := range v.FindingsByFile { + out = append(out, findings...) + } + return out +} + +// ContentPattern is a compiled pattern used by the content scanner. +type ContentPattern struct { + ID string + Pattern string + Severity string + Message string +} + +// DefaultContentPatterns returns the built-in security patterns. +func DefaultContentPatterns() []ContentPattern { + return []ContentPattern{ + {ID: "hardcoded-secret", Pattern: `(?i)(password|passwd|secret|token|api_key)\s*=\s*["'][^"']{8,}["']`, Severity: "critical", Message: "Possible hardcoded secret"}, + {ID: "private-key", Pattern: `-----BEGIN (RSA |EC |OPENSSH )?PRIVATE KEY`, Severity: "critical", Message: "Private key material"}, + {ID: "aws-key", Pattern: `AKIA[0-9A-Z]{16}`, Severity: "critical", Message: "Possible AWS access key"}, + {ID: "github-token", Pattern: `gh[pousr]_[A-Za-z0-9_]{36,}`, Severity: "critical", Message: "Possible GitHub token"}, + } +} + +// AuditSeverity is the severity level for audit report entries. +type AuditSeverity string + +const ( + SeverityCritical AuditSeverity = "critical" + SeverityHigh AuditSeverity = "high" + SeverityMedium AuditSeverity = "medium" + SeverityLow AuditSeverity = "low" + SeverityInfo AuditSeverity = "info" +) + +// AuditReportEntry represents one finding in an audit report. +type AuditReportEntry struct { + CheckName string + Severity AuditSeverity + Message string + Details []string + File string +} + +// AuditReport is the top-level output of a security audit. +type AuditReport struct { + Entries []AuditReportEntry +} + +// Critical returns all critical-severity entries. +func (r *AuditReport) Critical() []AuditReportEntry { + var out []AuditReportEntry + for _, e := range r.Entries { + if e.Severity == SeverityCritical { + out = append(out, e) + } + } + return out +} + +// HasBlockers returns true if there are any critical or high findings. +func (r *AuditReport) HasBlockers() bool { + for _, e := range r.Entries { + if e.Severity == SeverityCritical || e.Severity == SeverityHigh { + return true + } + } + return false +} diff --git a/internal/security/security_test.go b/internal/security/security_test.go new file mode 100644 index 00000000..91f78be5 --- /dev/null +++ b/internal/security/security_test.go @@ -0,0 +1,158 @@ +package security_test + +import ( + "testing" + + "github.com/githubnext/apm/internal/security" +) + +// TestParityScanPolicyBlock verifies BlockPolicy mirrors Python BLOCK_POLICY. +func TestParityScanPolicyBlock(t *testing.T) { + p := security.BlockPolicy + if p.OnCritical != "block" { + t.Errorf("expected on_critical=block") + } + if !p.ForceOverrides { + t.Errorf("expected force_overrides=true") + } +} + +// TestParityScanPolicyWarn verifies WarnPolicy mirrors Python WARN_POLICY. +func TestParityScanPolicyWarn(t *testing.T) { + p := security.WarnPolicy + if p.OnCritical != "warn" { + t.Errorf("expected on_critical=warn") + } + if p.ForceOverrides { + t.Errorf("expected force_overrides=false") + } +} + +// TestParityScanPolicyReport verifies ReportPolicy mirrors Python REPORT_POLICY. +func TestParityScanPolicyReport(t *testing.T) { + p := security.ReportPolicy + if p.OnCritical != "ignore" { + t.Errorf("expected on_critical=ignore") + } +} + +// TestParityEffectiveBlockNoForce verifies block=true when force=false. +func TestParityEffectiveBlockNoForce(t *testing.T) { + if !security.BlockPolicy.EffectiveBlock(false) { + t.Errorf("block policy should block when force=false") + } +} + +// TestParityEffectiveBlockWithForce verifies force overrides block. +func TestParityEffectiveBlockWithForce(t *testing.T) { + if security.BlockPolicy.EffectiveBlock(true) { + t.Errorf("block policy with force_overrides=true should not block when force=true") + } +} + +// TestParityEffectiveBlockWarnPolicy verifies warn policy never blocks. +func TestParityEffectiveBlockWarnPolicy(t *testing.T) { + if security.WarnPolicy.EffectiveBlock(false) { + t.Errorf("warn policy should not block") + } + if security.WarnPolicy.EffectiveBlock(true) { + t.Errorf("warn policy should not block with force") + } +} + +// TestParityScanVerdictHasFindings verifies finding detection. +func TestParityScanVerdictHasFindings(t *testing.T) { + v := &security.ScanVerdict{} + if v.HasFindings() { + t.Errorf("empty verdict should have no findings") + } + v2 := &security.ScanVerdict{ + FindingsByFile: map[string][]security.ScanFinding{ + "file.py": {{Rule: "hardcoded-secret", Severity: "critical"}}, + }, + } + if !v2.HasFindings() { + t.Errorf("verdict with findings should return true") + } +} + +// TestParityScanVerdictAllFindings flattens findings across files. +func TestParityScanVerdictAllFindings(t *testing.T) { + v := &security.ScanVerdict{ + FindingsByFile: map[string][]security.ScanFinding{ + "a.py": {{Rule: "r1", Severity: "critical"}, {Rule: "r2", Severity: "warning"}}, + "b.py": {{Rule: "r3", Severity: "info"}}, + }, + } + all := v.AllFindings() + if len(all) != 3 { + t.Errorf("expected 3 findings, got %d", len(all)) + } +} + +// TestParityDefaultContentPatterns verifies built-in patterns are present. +func TestParityDefaultContentPatterns(t *testing.T) { + patterns := security.DefaultContentPatterns() + if len(patterns) == 0 { + t.Errorf("expected at least one default pattern") + } + ids := map[string]bool{} + for _, p := range patterns { + ids[p.ID] = true + } + for _, want := range []string{"hardcoded-secret", "private-key", "aws-key", "github-token"} { + if !ids[want] { + t.Errorf("expected pattern %s to be in default patterns", want) + } + } +} + +// TestParityAuditSeverityValues verifies severity constants. +func TestParityAuditSeverityValues(t *testing.T) { + cases := map[security.AuditSeverity]string{ + security.SeverityCritical: "critical", + security.SeverityHigh: "high", + security.SeverityMedium: "medium", + security.SeverityLow: "low", + security.SeverityInfo: "info", + } + for sev, want := range cases { + if string(sev) != want { + t.Errorf("expected %s, got %s", want, sev) + } + } +} + +// TestParityAuditReportCritical filters critical entries. +func TestParityAuditReportCritical(t *testing.T) { + r := &security.AuditReport{ + Entries: []security.AuditReportEntry{ + {CheckName: "a", Severity: security.SeverityCritical}, + {CheckName: "b", Severity: security.SeverityHigh}, + {CheckName: "c", Severity: security.SeverityInfo}, + }, + } + crit := r.Critical() + if len(crit) != 1 { + t.Errorf("expected 1 critical entry, got %d", len(crit)) + } + if crit[0].CheckName != "a" { + t.Errorf("expected check a to be critical") + } +} + +// TestParityAuditReportHasBlockers verifies critical/high detection. +func TestParityAuditReportHasBlockers(t *testing.T) { + r := &security.AuditReport{} + if r.HasBlockers() { + t.Errorf("empty report should have no blockers") + } + r2 := &security.AuditReport{ + Entries: []security.AuditReportEntry{ + {CheckName: "a", Severity: security.SeverityHigh}, + }, + } + if !r2.HasBlockers() { + t.Errorf("high severity entry should count as blocker") + } +} From 29b3f180220eb90c9404d518a2d7fd1328239bfb Mon Sep 17 00:00:00 2001 From: "github-actions[bot]" Date: Wed, 27 May 2026 05:58:48 +0000 Subject: [PATCH 10/10] ci: trigger checks