From 3e68dcb2039ec407dfa0290a83b5171cf0e93dfa Mon Sep 17 00:00:00 2001 From: Joshua Temple Date: Thu, 9 Jul 2026 03:59:01 -0400 Subject: [PATCH] feat(config): shared-path change semantics via extra_paths and shared_paths Per-component versioning scoped commits to a component's own path, so a change to a shared library or root config outside every component path bumped no component and released nothing. Add two additive, optional fields: per-component extra_paths (paths beyond the component's own that both trigger and version it) and top-level shared_paths (fanned into every component). Both are threaded to all three sinks so a shared change is not silently half-handled: the emitted push-paths filter (the workflow fires), the version calculation (GetCommitsForPaths includes the extra paths so a breaking shared commit bumps correctly), and detectChanges (extra paths union into each build and deploy's non-empty triggers, so a shared change does not bump a version whose builds then skip; empty run-always triggers are preserved). shared_paths is rejected per component. When both fields are unset the resolved paths, emitted output, and version calc are unchanged, so single-component output stays byte-identical. Also correct a now-false doc-comment about extra_triggers.merge_group. Signed-off-by: Joshua Temple --- CONTRIBUTING.md | 1 + docs/public/manifest.schema.json | 6 + docs/src/content/docs/guides/components.md | 35 ++++ docs/src/content/docs/reference/manifest.md | 45 ++++- e2e/scenarios/56-component-shared-paths.yaml | 75 ++++++++ internal/config/components.go | 61 +++++- .../config/components_shared_paths_test.go | 102 ++++++++++ internal/config/types.go | 14 ++ internal/generate/merge_queue.go | 8 +- internal/orchestrate/orchestrator.go | 51 ++++- .../orchestrate/shared_path_version_test.go | 179 ++++++++++++++++++ internal/schema/manifest.schema.json | 6 + schema/manifest.schema.json | 6 + 13 files changed, 576 insertions(+), 13 deletions(-) create mode 100644 e2e/scenarios/56-component-shared-paths.yaml create mode 100644 internal/config/components_shared_paths_test.go create mode 100644 internal/orchestrate/shared_path_version_test.go diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 0bae2cfb..1ba9f109 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -52,6 +52,7 @@ Public APIs follow a functional-options style: required inputs are positional an cascade holds to a few conventions in its own codebase and in the workflows it generates: - **Additive manifest changes**: new fields are always optional with sensible defaults, so existing manifest files keep working across minor version bumps. +- **Path fields reach every path sink**: a manifest field that widens which files a component reacts to must thread through all three places a path is consumed, or it is a silent bug. The emitted `on: push` paths filter fires the workflow, per-callback change detection decides which builds and deploys run, and the version commit range decides the bump. A field that reaches only some of these triggers a run that then no-ops, or bumps a version whose builds skip as unchanged. When you add such a field, add a test that asserts the shared path reaches each sink. - **Callback isolation**: generated workflows call your workflows via `workflow_call`, and cascade never reaches into your callback logic. - **Metadata courier**: cascade passes artifact identifiers and versions between stages. It never touches your container registry, package registry, or the systems you deploy to directly. diff --git a/docs/public/manifest.schema.json b/docs/public/manifest.schema.json index 0eb2a1a4..130b5d09 100644 --- a/docs/public/manifest.schema.json +++ b/docs/public/manifest.schema.json @@ -70,6 +70,11 @@ "items": { "type": "string" }, "description": "Global path patterns for the orchestrate workflow paths filter. When set, these are used exclusively instead of per-callback triggers." }, + "shared_paths": { + "type": "array", + "items": { "type": "string" }, + "description": "Repo-relative globs every component depends on, such as a shared library or a root build file. Each is fanned into every component's effective path set, so a commit touching only a shared path fires each component's orchestrate workflow and counts toward each component's version bump. Sugar for repeating the same glob in every component's extra_paths; applies only when a components block is present." + }, "release_trigger": { "type": "string", "enum": ["push", "dispatch"], @@ -548,6 +553,7 @@ "deployments": { "$ref": "#/definitions/deploymentsConfig" }, "environment_config": { "type": "object", "additionalProperties": { "$ref": "#/definitions/environmentConfig" }, "description": "Overrides the shared per-environment settings for this component." }, "triggers": { "type": "array", "items": { "type": "string" }, "description": "Overrides the shared orchestrate path filter for this component." }, + "extra_paths": { "type": "array", "items": { "type": "string" }, "description": "Repo-relative globs beyond this component's own path that both fire its orchestrate workflow and count toward its version bump, so a change to a shared dependency this component consumes bumps it correctly. Additive to path and to any top-level shared_paths." }, "release_token": { "type": "string", "description": "Overrides the shared release-operations token expression for this component." }, "release_token_app": { "$ref": "#/definitions/appTokenSource" } } diff --git a/docs/src/content/docs/guides/components.md b/docs/src/content/docs/guides/components.md index 8c652137..86cad232 100644 --- a/docs/src/content/docs/guides/components.md +++ b/docs/src/content/docs/guides/components.md @@ -107,6 +107,41 @@ so a repository without a `components:` block reads its tags exactly as before. [Per-component versioning](/cascade/reference/versioning/#per-component-versioning) for the tag-namespace rules in full. +## Share code across components + +Scoping the commit walk to each component's `path` is the isolation invariant, but +a monorepo also has code every component shares: a common library, a proto package, +a root build file. A change there sits outside every component's `path`, so on its +own it fires nothing and bumps nothing. Two additive fields let a component opt into +the shared code it depends on: + +```yaml +ci: + config: + environments: [dev, prod] + shared_paths: + - libs/common/** # every component depends on this + components: + api: + path: services/api + tag_prefix: api- + extra_paths: + - libs/proto/** # only api depends on the proto package + web: + path: services/web + tag_prefix: web- +``` + +`extra_paths` widens one component's scope; top-level `shared_paths` widens every +component's scope and is sugar for adding the same glob to each component's +`extra_paths`. A component's effective scope is its `path` plus both. That scope +reaches every place a path matters: the workflow's `push` filter fires on a shared +change, change detection runs the affected builds and deploys, and the version walk +counts the shared commit. A breaking (`feat!:`) commit under `libs/common/` bumps +both `api` and `web`; a breaking commit under `libs/proto/` bumps only `api`. When +you declare neither field, each component's scope is just its `path`, exactly as +before. + ## How each component promotes independently Each component gets its own promote workflow and its own concurrency lane. Cascade diff --git a/docs/src/content/docs/reference/manifest.md b/docs/src/content/docs/reference/manifest.md index 498eeb9a..4e299b7c 100644 --- a/docs/src/content/docs/reference/manifest.md +++ b/docs/src/content/docs/reference/manifest.md @@ -800,6 +800,7 @@ any inheritable field it overrides. |-------|--------|------|----------|-------------| | `path` | emitted (behavior) | string | Yes | The subtree this component owns. Relative, with no `..` segments. Scopes the component's version commit walk and its default push-paths trigger. | | `tag_prefix` | emitted (behavior) | string | Yes | The component's version-tag prefix. Must be distinct from every other component's prefix so their tag namespaces never collide. | +| `extra_paths` | emitted (behavior) | list | No | Additional globs beyond `path` that both fire this component's orchestrate workflow and count toward its version bump. Use it when a component depends on a shared library or a root build file outside its own subtree. See [Shared paths](#shared-paths). | ### Inheritable overrides @@ -824,8 +825,48 @@ because they describe the repository or the single writer of shared state rather than one component: `schema_version`, `trunk_branch`, `cli_version`, `cli_version_sha`, `state_token`, `state_token_app`, `manifest_file`, `manifest_key`, `action_folder`, `git`, `drift_check`, `reconcile`, `pin_mode`, -`action_pins`, `telemetry`, and `merge_queue`. Setting any of them under a -component is a parse error, as is any unknown field. +`action_pins`, `telemetry`, `merge_queue`, and `shared_paths`. Setting any of them +under a component is a parse error, as is any unknown field. + +### Shared paths + +A change to a shared dependency, a common library, a proto package, or a root +build file, lives outside every component's own `path`. Without help it fires no +component and bumps no version, so a busy shared subtree ships nothing. Two fields +close that gap by widening a component's effective path set beyond its `path`: + +- `extra_paths` on a component adds globs that only that component depends on. +- `shared_paths` at the top level adds globs that every component depends on. It + is sugar for repeating the same glob in every component's `extra_paths`, and + applies only when a `components:` block is present. + +Each glob in a component's effective set (its `path`, its `extra_paths`, and the +top-level `shared_paths`) reaches all three places a path matters: the emitted +`push` paths filter that fires the workflow, the per-callback change detection that +decides which builds and deploys run, and the commit range that computes the +version bump. A breaking (`feat!:` or `fix!:`) commit under a shared path bumps the +major of exactly the components that declare it, and leaves the rest untouched. + +```yaml +ci: + config: + environments: [dev, prod] + shared_paths: + - libs/common/** # every component depends on this + components: + api: + path: services/api + tag_prefix: api- + extra_paths: + - libs/proto/** # only api depends on the proto package + web: + path: services/web + tag_prefix: web- +``` + +Here a commit under `libs/common/` bumps both `api` and `web`; a commit under +`libs/proto/` bumps only `api`. When neither field is set the effective path set is +just each component's `path`, byte-identical to before the fields existed. ### Validation rules diff --git a/e2e/scenarios/56-component-shared-paths.yaml b/e2e/scenarios/56-component-shared-paths.yaml new file mode 100644 index 00000000..3be682c5 --- /dev/null +++ b/e2e/scenarios/56-component-shared-paths.yaml @@ -0,0 +1,75 @@ +name: "Per-Component Shared Path Threading" +description: | + Proves a component's extra_paths and the top-level shared_paths sugar both reach + the emitted orchestrate push-paths filter, so a commit touching a shared + dependency fires the workflows of the components that declare it. + + The manifest declares two components. api adds libs/proto to its own extra_paths; + the top-level shared_paths adds libs/common to every component. web declares + neither, so it inherits only libs/common (the shared sugar) plus its own path. + + Generation fans the orchestrate lane out to one orchestrate-.yaml per + component. The proof is the emitted push-paths filter of each: + - orchestrate-api.yaml lists libs/common (shared), libs/proto (its extra_paths), + and services/api (its own path). + - orchestrate-web.yaml lists libs/common (shared) and services/web (its own + path), and NOT libs/proto (a path only api declares), so a non-consumer is not + triggered by another component's private dependency. + + The version-commit-range and change-detection sinks of the same effective path + set are covered by the orchestrate unit tests (a breaking shared-path commit + bumps only the declaring component). This scenario pins the generated-output + sink. + +config: + trunk_branch: main + environments: [dev, prod] + shared_paths: + - libs/common/** + builds: + - name: app + workflow: build.yaml + triggers: ["services/**"] + deploys: + - name: app + workflow: deploy.yaml + triggers: ["services/**"] + components: + api: + path: services/api + tag_prefix: api- + extra_paths: + - libs/proto/** + web: + path: services/web + tag_prefix: web- + +steps: + - name: "Seed both component subtrees and assert the shared-path push filters" + action: commit + commit: + message: "feat: seed component sources" + files: + services/api/main.go: | + package main + + func main() {} + services/web/main.go: | + package main + + func main() {} + expect: + workflow_files: + - path: ".github/workflows/orchestrate-api.yaml" + contains: + - " paths:\n" + - " - 'libs/common/**'\n" + - " - 'libs/proto/**'\n" + - " - 'services/api/**'\n" + - path: ".github/workflows/orchestrate-web.yaml" + contains: + - " paths:\n" + - " - 'libs/common/**'\n" + - " - 'services/web/**'\n" + not_contains: + - "libs/proto" diff --git a/internal/config/components.go b/internal/config/components.go index 2dba7228..d5c8d090 100644 --- a/internal/config/components.go +++ b/internal/config/components.go @@ -3,6 +3,7 @@ package config import ( "encoding/json" "fmt" + "sort" "strings" "github.com/stablekernel/cascade/internal/taggrammar" @@ -89,9 +90,16 @@ func (c *TrunkConfig) GetComponentTagPrefix(name string) (string, error) { // per-component axis with no home on TrunkConfig, so it is carried here rather // than folded into Config; downstream stages scope version and state work to it. type ResolvedComponent struct { - Name string - Path string - Config *TrunkConfig + Name string + Path string + // ExtraPaths is the component's effective additional path set: its own + // extra_paths unioned with the manifest's top-level shared_paths, deduplicated + // and deterministically ordered. It is additive to Path. Version derivation and + // change detection scope to Path plus ExtraPaths so a shared-dependency change + // bumps and fires this component; it is empty when neither field is declared, + // keeping the single-component and no-shared-path shapes unchanged. + ExtraPaths []string + Config *TrunkConfig } // ResolveComponent returns the effective configuration for the named component: @@ -117,7 +125,8 @@ func (c *TrunkConfig) ResolveComponent(name string) (*ResolvedComponent, error) if err != nil { return nil, err } - eff.Components = nil // an effective per-component config has no nested components + eff.Components = nil // an effective per-component config has no nested components + eff.SharedPaths = nil // top-level sugar, expanded into per-component extra paths below // Required per-component tag namespace. eff.TagPrefix = comp.TagPrefix @@ -218,7 +227,48 @@ func (c *TrunkConfig) ResolveComponent(name string) (*ResolvedComponent, error) eff.Triggers = []string{strings.TrimRight(comp.Path, "/") + "/**"} } - return &ResolvedComponent{Name: name, Path: comp.Path, Config: eff}, nil + // Effective additional paths: the component's own extra_paths unioned with the + // manifest's top-level shared_paths, deduplicated and sorted for a single stable + // downstream representation. These fire the workflow (folded into the push + // filter below) and, threaded by the orchestrator, count toward the version bump + // and change detection. When both are empty this is nil and nothing changes. + extraPaths := mergePaths(comp.ExtraPaths, c.SharedPaths) + + // Fold the extra paths into the push-paths filter so a shared-dependency change + // fires this component's orchestrate workflow. GetAllTriggers reads eff.Triggers, + // so appending here reaches the emitted on.push.paths block. Deduplicated so an + // extra path that already equals a trigger is not repeated. + if len(extraPaths) > 0 { + eff.Triggers = mergePaths(eff.Triggers, extraPaths) + } + + return &ResolvedComponent{Name: name, Path: comp.Path, ExtraPaths: extraPaths, Config: eff}, nil +} + +// mergePaths returns the union of the given path lists with duplicates removed and +// a deterministic (sorted) order, or nil when the union is empty. It backs the +// shared_paths fan-out and the push-filter fold so every downstream sink sees one +// stable representation of a component's effective additional paths. +func mergePaths(lists ...[]string) []string { + seen := make(map[string]struct{}) + var out []string + for _, list := range lists { + for _, p := range list { + if p == "" { + continue + } + if _, ok := seen[p]; ok { + continue + } + seen[p] = struct{}{} + out = append(out, p) + } + } + if len(out) == 0 { + return nil + } + sort.Strings(out) + return out } // TagGrammarSpec returns the tag grammar a component reads and emits its versions @@ -260,4 +310,5 @@ var globalOnlyComponentFields = map[string]struct{}{ "telemetry": {}, "merge_queue": {}, "components": {}, + "shared_paths": {}, } diff --git a/internal/config/components_shared_paths_test.go b/internal/config/components_shared_paths_test.go new file mode 100644 index 00000000..0c4b2231 --- /dev/null +++ b/internal/config/components_shared_paths_test.go @@ -0,0 +1,102 @@ +package config + +import ( + "reflect" + "testing" +) + +// TestResolveComponent_ExtraPaths proves a per-component extra_paths list is +// carried onto the resolved component (for version and change-detection scoping) +// and folded into the push-paths filter, additive to the component's own path. +func TestResolveComponent_ExtraPaths(t *testing.T) { + c := baseComponentConfig() + comp := c.Components["api"] + comp.ExtraPaths = []string{"libs/proto/**"} + c.Components["api"] = comp + + api, err := c.ResolveComponent("api") + if err != nil { + t.Fatalf("ResolveComponent(api): %v", err) + } + + if want := []string{"libs/proto/**"}; !reflect.DeepEqual(api.ExtraPaths, want) { + t.Errorf("api.ExtraPaths = %v, want %v", api.ExtraPaths, want) + } + // Push filter (GetAllTriggers) must include the component path and the extra path. + got := api.Config.GetAllTriggers() + want := []string{"libs/proto/**", "services/api/**"} + if !reflect.DeepEqual(got, want) { + t.Errorf("api triggers = %v, want %v", got, want) + } + + // A sibling without extra_paths is unaffected. + web, err := c.ResolveComponent("web") + if err != nil { + t.Fatalf("ResolveComponent(web): %v", err) + } + if len(web.ExtraPaths) != 0 { + t.Errorf("web.ExtraPaths = %v, want empty", web.ExtraPaths) + } + if g := web.Config.GetAllTriggers(); len(g) != 1 || g[0] != "services/web/**" { + t.Errorf("web triggers = %v, want [services/web/**]", g) + } +} + +// TestResolveComponent_SharedPathsFanOut proves a top-level shared_paths entry is +// fanned into every component's effective path set and push filter, and unions +// with a component's own extra_paths without duplication. +func TestResolveComponent_SharedPathsFanOut(t *testing.T) { + c := baseComponentConfig() + c.SharedPaths = []string{"go.mod", "libs/shared/**"} + comp := c.Components["api"] + comp.ExtraPaths = []string{"libs/proto/**", "go.mod"} // go.mod overlaps shared_paths + c.Components["api"] = comp + + api, err := c.ResolveComponent("api") + if err != nil { + t.Fatalf("ResolveComponent(api): %v", err) + } + wantAPI := []string{"go.mod", "libs/proto/**", "libs/shared/**"} + if !reflect.DeepEqual(api.ExtraPaths, wantAPI) { + t.Errorf("api.ExtraPaths = %v, want %v (deduped, sorted)", api.ExtraPaths, wantAPI) + } + + web, err := c.ResolveComponent("web") + if err != nil { + t.Fatalf("ResolveComponent(web): %v", err) + } + wantWeb := []string{"go.mod", "libs/shared/**"} + if !reflect.DeepEqual(web.ExtraPaths, wantWeb) { + t.Errorf("web.ExtraPaths = %v, want %v", web.ExtraPaths, wantWeb) + } + // Push filter for web: its own path plus the shared paths. + if g := web.Config.GetAllTriggers(); !reflect.DeepEqual(g, []string{"go.mod", "libs/shared/**", "services/web/**"}) { + t.Errorf("web triggers = %v, want [go.mod libs/shared/** services/web/**]", g) + } +} + +// TestResolveComponent_NoSharedPathsByteIdentical proves that when neither +// extra_paths nor shared_paths is set, the resolved component carries no extra +// paths and the push filter is exactly the path-derived default, so the shape is +// unchanged from before the feature. +func TestResolveComponent_NoSharedPathsByteIdentical(t *testing.T) { + c := baseComponentConfig() + api, err := c.ResolveComponent("api") + if err != nil { + t.Fatalf("ResolveComponent(api): %v", err) + } + if api.ExtraPaths != nil { + t.Errorf("api.ExtraPaths = %v, want nil when unset", api.ExtraPaths) + } + if g := api.Config.GetAllTriggers(); len(g) != 1 || g[0] != "services/api/**" { + t.Errorf("api triggers = %v, want [services/api/**]", g) + } +} + +// TestSharedPathsRejectedPerComponent proves shared_paths is a top-level-only key: +// setting it under a component is a configuration error, not a silent no-op. +func TestSharedPathsRejectedPerComponent(t *testing.T) { + if _, ok := globalOnlyComponentFields["shared_paths"]; !ok { + t.Fatalf("shared_paths must be a global-only component field") + } +} diff --git a/internal/config/types.go b/internal/config/types.go index 97bb916c..1a74a148 100644 --- a/internal/config/types.go +++ b/internal/config/types.go @@ -189,6 +189,14 @@ type TrunkConfig struct { // one manifest (#176). v1 contract: parse + structural validation only; no // generator, state, or runtime behavior is attached. Absent by default. Components map[string]ComponentConfig `yaml:"components,omitempty" json:"components,omitempty"` + // SharedPaths lists repo-relative globs that every component depends on, such as + // a shared library or a root build file. Each entry is fanned into every + // component's effective path set at resolution, so a commit touching only a + // shared path fires each component's orchestrate workflow and counts toward each + // component's version bump. It is top-level sugar for repeating the same glob in + // every component's extra_paths, and applies only when a components block is + // present. Empty by default, so a manifest without it is unchanged. + SharedPaths []string `yaml:"shared_paths,omitempty" json:"shared_paths,omitempty"` } // ConcurrencyConfig overrides the default concurrency: block emitted on the @@ -1232,6 +1240,12 @@ type ComponentConfig struct { Deployments *DeploymentsConfig `yaml:"deployments,omitempty" json:"deployments,omitempty"` EnvironmentConfig map[string]EnvironmentConfig `yaml:"environment_config,omitempty" json:"environment_config,omitempty"` Triggers []string `yaml:"triggers,omitempty" json:"triggers,omitempty"` + // ExtraPaths lists repo-relative globs beyond this component's own path that both + // fire its orchestrate workflow and count toward its version bump, so a change to + // a shared dependency this component consumes bumps it correctly. It is additive + // to the component's path and to any top-level shared_paths; it never replaces the + // path-scoped default. Empty by default. + ExtraPaths []string `yaml:"extra_paths,omitempty" json:"extra_paths,omitempty"` ReleaseToken string `yaml:"release_token,omitempty" json:"release_token,omitempty"` ReleaseTokenApp *AppTokenSource `yaml:"release_token_app,omitempty" json:"release_token_app,omitempty"` diff --git a/internal/generate/merge_queue.go b/internal/generate/merge_queue.go index 3904f66b..cf7cd682 100644 --- a/internal/generate/merge_queue.go +++ b/internal/generate/merge_queue.go @@ -16,9 +16,11 @@ import ( // merge-group candidate ref. The lane is read-only (no state writes, no // releases, no deploys) and reports a status the merge queue can require. // -// This generator owns the LANE behavior. The raw merge_group trigger itself is -// expressible separately under extra_triggers.merge_group; the two are -// intentionally distinct. +// This generator owns the LANE behavior and is the only supported way to attach a +// merge_group trigger. Attaching the raw merge_group event through +// extra_triggers.merge_group is rejected at validation, because it would fire the +// side-effecting orchestrate workflow on a speculative merge-queue build with no +// gh-readonly-queue guard; merge_queue.enabled emits this read-only lane instead. type MergeQueueGenerator struct { config *config.TrunkConfig baseDir string diff --git a/internal/orchestrate/orchestrator.go b/internal/orchestrate/orchestrator.go index 8350c020..25a80841 100644 --- a/internal/orchestrate/orchestrator.go +++ b/internal/orchestrate/orchestrator.go @@ -109,11 +109,17 @@ func (o *Orchestrator) Setup(headSHA string) (*output.SetupResult, error) { baseSHAs := o.calculateBaseSHAs(envState) log.Debug("Base SHAs: %v", baseSHAs) + // Effective extra paths for the scoped component (its extra_paths unioned with + // top-level shared_paths), so a change touching only a shared dependency is not + // skipped as unchanged by a callback whose own triggers do not list it. Nil on + // the single-component path, keeping change detection byte-identical there. + extraPaths := o.componentExtraPaths() + // Detect which builds need to run runBuilds := make(map[string]bool) for _, build := range o.cicdFile.Config.Builds { baseSHA := baseSHAs["build_"+build.Name] - needsRun := o.detectChanges(baseSHA, headSHA, build.Triggers) + needsRun := o.detectChanges(baseSHA, headSHA, withExtraPaths(build.Triggers, extraPaths)) runBuilds[build.Name] = needsRun log.Debug("Build %s: needs_run=%v (base=%s)", build.Name, needsRun, truncateSHA(baseSHA)) } @@ -127,7 +133,7 @@ func (o *Orchestrator) Setup(headSHA string) (*output.SetupResult, error) { log.Debug("Deploy %s: pending (depends on builds)", deploy.Name) } else { baseSHA := baseSHAs["deploy_"+deploy.Name] - needsRun := o.detectChanges(baseSHA, headSHA, deploy.Triggers) + needsRun := o.detectChanges(baseSHA, headSHA, withExtraPaths(deploy.Triggers, extraPaths)) if needsRun { runDeploys[deploy.Name] = "true" } else { @@ -350,6 +356,39 @@ func (o *Orchestrator) detectChanges(baseSHA, headSHA string, triggers []string) return false } +// componentExtraPaths returns the effective extra path set for the scoped +// component: its extra_paths unioned with the manifest's top-level shared_paths. +// It returns nil on the single-component path (o.component == ""), so change +// detection there is byte-identical. A resolution error is logged and treated as +// no extra paths rather than failing setup. +func (o *Orchestrator) componentExtraPaths() []string { + if o.component == "" { + return nil + } + resolved, err := o.cicdFile.Config.ResolveComponent(o.component) + if err != nil { + log.Warn("Failed to resolve component %s for change detection: %v", o.component, err) + return nil + } + return resolved.ExtraPaths +} + +// withExtraPaths unions a callback's own triggers with the component's effective +// extra paths so a shared-dependency change is detected by that callback. It only +// augments a non-empty trigger list: an empty triggers list already means "run on +// any change", so the shared change is covered and adding paths would wrongly +// narrow it to those paths. It returns the original slice when there is nothing to +// add, keeping the single-component and no-extra-path shapes byte-identical. +func withExtraPaths(triggers, extraPaths []string) []string { + if len(triggers) == 0 || len(extraPaths) == 0 { + return triggers + } + out := make([]string, 0, len(triggers)) + out = append(out, triggers...) + out = append(out, extraPaths...) + return out +} + // calculateVersion calculates the next version for the environment. func (o *Orchestrator) calculateVersion() (string, error) { // A component-scoped orchestration derives its version from only its own @@ -508,9 +547,15 @@ func (o *Orchestrator) calculateComponentVersion() (string, error) { baseSHA, _ = git.GetInitialCommit() } + // Scope the commit range to the component's own path plus its effective extra + // paths (its extra_paths unioned with top-level shared_paths), so a commit that + // touches only a shared dependency this component declares still registers a + // bump. With no extra paths this is just the component path, unchanged. + versionPaths := append([]string{resolved.Path}, resolved.ExtraPaths...) + var commits []changelog.ConventionalCommit if baseSHA != "" { - gitCommits, cerr := git.GetCommitsForPaths(baseSHA, "HEAD", []string{resolved.Path}) + gitCommits, cerr := git.GetCommitsForPaths(baseSHA, "HEAD", versionPaths) if cerr != nil { log.Warn("Failed to get commits for component %s: %v", o.component, cerr) } else { diff --git a/internal/orchestrate/shared_path_version_test.go b/internal/orchestrate/shared_path_version_test.go new file mode 100644 index 00000000..3b0fd6c7 --- /dev/null +++ b/internal/orchestrate/shared_path_version_test.go @@ -0,0 +1,179 @@ +package orchestrate + +import ( + "os" + "path/filepath" + "testing" +) + +// sharedPathRepo initializes a git repo with a two-component manifest where the +// api component declares extra_paths covering a shared library and web does not. +// Both components are seeded with a v1.0.0 release tag at a shared base. topSugar +// selects the shared_paths top-level form instead of per-component extra_paths. +func sharedPathRepo(t *testing.T, topSugar bool) string { + t.Helper() + + dir := t.TempDir() + orig, err := os.Getwd() + if err != nil { + t.Fatalf("getwd: %v", err) + } + if err := os.Chdir(dir); err != nil { + t.Fatalf("chdir: %v", err) + } + t.Cleanup(func() { + if cerr := os.Chdir(orig); cerr != nil { + t.Fatalf("restore cwd: %v", cerr) + } + }) + + for _, args := range [][]string{ + {"init"}, + {"config", "user.email", "test@example.com"}, + {"config", "user.name", "Test User"}, + {"config", "commit.gpgsign", "false"}, + } { + runGitT(t, args...) + } + + configPath := filepath.Join(dir, ".github", "manifest.yaml") + if err := os.MkdirAll(filepath.Dir(configPath), 0o750); err != nil { + t.Fatalf("mkdir .github: %v", err) + } + + var manifest string + if topSugar { + // Top-level shared_paths fans out to every component (api and web). + manifest = `ci: + config: + trunk_branch: main + environments: + - dev + - prod + shared_paths: + - libs/shared/** + components: + api: + path: services/api + tag_prefix: api- + web: + path: services/web + tag_prefix: web- +` + } else { + // Only api declares the shared library under its extra_paths. + manifest = `ci: + config: + trunk_branch: main + environments: + - dev + - prod + components: + api: + path: services/api + tag_prefix: api- + extra_paths: + - libs/shared/** + web: + path: services/web + tag_prefix: web- +` + } + if err := os.WriteFile(configPath, []byte(manifest), 0o600); err != nil { + t.Fatalf("write manifest: %v", err) + } + + writeCommitT(t, "README.md", "root", "chore: seed") + runGitT(t, "tag", "api-1.0.0") + runGitT(t, "tag", "web-1.0.0") + + return configPath +} + +// TestCalculateComponentVersion_SharedPathBumpsConsumer proves a breaking change +// touching only a shared library the api component declares under extra_paths +// bumps api to a major version, while the non-consuming web component (which does +// not declare the shared path) stays at its base. This is the shared-path +// correctness fix: the commit is outside every component's own path, so before +// the extra_paths threading it bumped nothing. +func TestCalculateComponentVersion_SharedPathBumpsConsumer(t *testing.T) { + configPath := sharedPathRepo(t, false) + + // A breaking change under the shared library only, no service subtree touched. + writeCommitT(t, filepath.Join("libs", "shared", "proto.go"), "package shared", "feat!: change shared proto contract") + + if got := componentVersion(t, configPath, "api"); got != "api-2.0.0-rc.0" { + t.Errorf("api version = %q, want %q (major bump from breaking shared-path change)", got, "api-2.0.0-rc.0") + } + // web does not declare the shared path, so the shared commit is out of its + // range and it stays at its base. + if got := componentVersion(t, configPath, "web"); got != "web-1.0.0-rc.0" { + t.Errorf("web version = %q, want %q (non-consumer must not move)", got, "web-1.0.0-rc.0") + } +} + +// TestCalculateComponentVersion_SharedPathsSugarBumpsAll proves the top-level +// shared_paths sugar fans out to every component: a fix under the shared library +// bumps both api and web (both declare it via the sugar). +func TestCalculateComponentVersion_SharedPathsSugarBumpsAll(t *testing.T) { + configPath := sharedPathRepo(t, true) + + writeCommitT(t, filepath.Join("libs", "shared", "util.go"), "package shared", "fix: shared util bug") + + if got := componentVersion(t, configPath, "api"); got != "api-1.0.1-rc.0" { + t.Errorf("api version = %q, want %q (patch bump from shared_paths fan-out)", got, "api-1.0.1-rc.0") + } + if got := componentVersion(t, configPath, "web"); got != "web-1.0.1-rc.0" { + t.Errorf("web version = %q, want %q (patch bump from shared_paths fan-out)", got, "web-1.0.1-rc.0") + } +} + +// TestCalculateComponentVersion_SharedPathNotDeclaredNoBump is the control that +// pins the pre-fix behavior for an undeclared shared path: a component whose +// extra_paths does not cover the shared library sees no bump from a shared-only +// commit. It documents that threading extra_paths is exactly what turns the +// no-bump bug into the correct bump asserted above. +func TestCalculateComponentVersion_SharedPathNotDeclaredNoBump(t *testing.T) { + configPath := sharedPathRepo(t, false) + + writeCommitT(t, filepath.Join("libs", "shared", "proto.go"), "package shared", "feat!: change shared proto contract") + + // web does not declare libs/shared/** anywhere, so the shared commit is out of + // range: no bump, base version retained. + if got := componentVersion(t, configPath, "web"); got != "web-1.0.0-rc.0" { + t.Errorf("web version = %q, want %q (undeclared shared path yields no bump)", got, "web-1.0.0-rc.0") + } +} + +// TestDetectChanges_WithExtraPaths proves a callback whose own triggers do not +// list a shared path still runs when that shared path changes, once the +// component's extra paths are unioned in; and that an empty trigger list is left +// untouched (it already means run-on-any-change). +func TestDetectChanges_WithExtraPaths(t *testing.T) { + // Non-empty triggers gain the shared path. + got := withExtraPaths([]string{"services/api/**"}, []string{"libs/shared/**"}) + if want := []string{"services/api/**", "libs/shared/**"}; !equalStrings(got, want) { + t.Errorf("withExtraPaths(non-empty) = %v, want %v", got, want) + } + // Empty triggers are returned unchanged (run-on-any-change preserved). + if got := withExtraPaths(nil, []string{"libs/shared/**"}); got != nil { + t.Errorf("withExtraPaths(empty triggers) = %v, want nil (unchanged)", got) + } + // No extra paths: original returned unchanged. + orig := []string{"services/api/**"} + if got := withExtraPaths(orig, nil); !equalStrings(got, orig) { + t.Errorf("withExtraPaths(no extra) = %v, want %v", got, orig) + } +} + +func equalStrings(a, b []string) bool { + if len(a) != len(b) { + return false + } + for i := range a { + if a[i] != b[i] { + return false + } + } + return true +} diff --git a/internal/schema/manifest.schema.json b/internal/schema/manifest.schema.json index 0eb2a1a4..130b5d09 100644 --- a/internal/schema/manifest.schema.json +++ b/internal/schema/manifest.schema.json @@ -70,6 +70,11 @@ "items": { "type": "string" }, "description": "Global path patterns for the orchestrate workflow paths filter. When set, these are used exclusively instead of per-callback triggers." }, + "shared_paths": { + "type": "array", + "items": { "type": "string" }, + "description": "Repo-relative globs every component depends on, such as a shared library or a root build file. Each is fanned into every component's effective path set, so a commit touching only a shared path fires each component's orchestrate workflow and counts toward each component's version bump. Sugar for repeating the same glob in every component's extra_paths; applies only when a components block is present." + }, "release_trigger": { "type": "string", "enum": ["push", "dispatch"], @@ -548,6 +553,7 @@ "deployments": { "$ref": "#/definitions/deploymentsConfig" }, "environment_config": { "type": "object", "additionalProperties": { "$ref": "#/definitions/environmentConfig" }, "description": "Overrides the shared per-environment settings for this component." }, "triggers": { "type": "array", "items": { "type": "string" }, "description": "Overrides the shared orchestrate path filter for this component." }, + "extra_paths": { "type": "array", "items": { "type": "string" }, "description": "Repo-relative globs beyond this component's own path that both fire its orchestrate workflow and count toward its version bump, so a change to a shared dependency this component consumes bumps it correctly. Additive to path and to any top-level shared_paths." }, "release_token": { "type": "string", "description": "Overrides the shared release-operations token expression for this component." }, "release_token_app": { "$ref": "#/definitions/appTokenSource" } } diff --git a/schema/manifest.schema.json b/schema/manifest.schema.json index 0eb2a1a4..130b5d09 100644 --- a/schema/manifest.schema.json +++ b/schema/manifest.schema.json @@ -70,6 +70,11 @@ "items": { "type": "string" }, "description": "Global path patterns for the orchestrate workflow paths filter. When set, these are used exclusively instead of per-callback triggers." }, + "shared_paths": { + "type": "array", + "items": { "type": "string" }, + "description": "Repo-relative globs every component depends on, such as a shared library or a root build file. Each is fanned into every component's effective path set, so a commit touching only a shared path fires each component's orchestrate workflow and counts toward each component's version bump. Sugar for repeating the same glob in every component's extra_paths; applies only when a components block is present." + }, "release_trigger": { "type": "string", "enum": ["push", "dispatch"], @@ -548,6 +553,7 @@ "deployments": { "$ref": "#/definitions/deploymentsConfig" }, "environment_config": { "type": "object", "additionalProperties": { "$ref": "#/definitions/environmentConfig" }, "description": "Overrides the shared per-environment settings for this component." }, "triggers": { "type": "array", "items": { "type": "string" }, "description": "Overrides the shared orchestrate path filter for this component." }, + "extra_paths": { "type": "array", "items": { "type": "string" }, "description": "Repo-relative globs beyond this component's own path that both fire its orchestrate workflow and count toward its version bump, so a change to a shared dependency this component consumes bumps it correctly. Additive to path and to any top-level shared_paths." }, "release_token": { "type": "string", "description": "Overrides the shared release-operations token expression for this component." }, "release_token_app": { "$ref": "#/definitions/appTokenSource" } }