From 1fac1a7cc9efac87b5a64abc9bae5d6cd2eaee5d Mon Sep 17 00:00:00 2001 From: Antoine Toussaint <312536060+antoinetoussaint-byte@users.noreply.github.com> Date: Tue, 27 Oct 2026 15:30:06 -0400 Subject: [PATCH 1/7] fix(generate): runnable-bindings materializes pinned modules, so --check can run on a fresh checkout MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `codefly generate runnable-bindings` walks every composed module for its derived operations, but loaded the workspace with the plain loader. On a composition of pinned modules with no codefly.local.yaml — a CI job, or any fresh checkout — it refused before reading anything: module resolves to a pinned artifact (source "obin-ai/core-solutions", version "0a18964f…"); pinned modules are pulled by the CLI, not loadable as a local checkout so `--check`, the one drift check this generated group has, could not be run where drift is caught. The command now materializes pinned modules first, exactly as `codefly run`, `deploy gitops render` and `generate fixture-composition` do (common.LoadWorkspaceWithPinnedModules), and is free once the workspace is materialized. Verified on obin-ai/platform-obin main (694ac3f4), environment blue: before, the refusal above; after, the check reaches the regeneration and refuses with the composition's real contradiction — `--call-endpoint "saas/auth-gateway/hop": service saas/auth-gateway declares no endpoint hop (it declares [grpc rest workload])` — while the committed configurations/blue/runnable-bindings.env binds 17 operations to their owners' direct in-cluster addresses, two of them on module compute, which the composition does not compose. First slice of making render and deploy refuse committed generated output that no longer matches what the generator would produce; the generic check on the readiness gate follows (see the handoff). Co-Authored-By: Claude Fable 5.1 --- cmd/generate/runnable_bindings.go | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/cmd/generate/runnable_bindings.go b/cmd/generate/runnable_bindings.go index f6a376a0..b5b1c4c2 100644 --- a/cmd/generate/runnable_bindings.go +++ b/cmd/generate/runnable_bindings.go @@ -141,7 +141,12 @@ Examples: ctx, stop := common.SignalContext(ctx) defer stop() - workspace, err := common.LoadWorkspace(ctx) + // Every composed module is walked for its derived operations, so a + // pinned module is materialized first, as a run, a render and + // `generate fixture-composition` do: on a fresh checkout — a CI job — + // the plain loader refuses a pinned module as "not loadable as a local + // checkout", and --check never reaches the file it exists to judge. + workspace, err := common.LoadWorkspaceWithPinnedModules(ctx) if err != nil { return err } From 91ff4342230c34f32a1791c3713939dd28615923 Mon Sep 17 00:00:00 2001 From: Antoine Toussaint <312536060+antoinetoussaint-byte@users.noreply.github.com> Date: Tue, 27 Oct 2026 16:40:29 -0400 Subject: [PATCH 2/7] fix(release): verify artifacts and carry generated client versions --- cmd/doctor_workspace.go | 36 ++- cmd/doctor_workspace_binding_values_test.go | 73 +++++ cmd/doctor_workspace_exclusions_test.go | 35 +++ cmd/doctor_workspace_runnable_test.go | 33 ++ cmd/generate/runnable_bindings.go | 28 +- .../runnable_bindings_freshness_test.go | 104 +++++++ cmd/publish/agent_release.go | 56 ++-- cmd/publish/client_versions_test.go | 138 +++++++++ cmd/publish/module_services.go | 134 +++++++- cmd/publish/publication_credentials_test.go | 131 ++++++++ cmd/publish/release_checksums.go | 75 +++++ cmd/publish/release_checksums_test.go | 102 ++++++ cmd/publish/release_upload_test.go | 26 +- cmd/publish/workflow_release.go | 83 +++-- cmd/publish/workflow_release_test.go | 29 +- docs/cli-reference.md | 2 +- docs/commands.md | 27 +- docs/publish-client-versions.md | 25 ++ docs/publish-verification.md | 30 ++ go.mod | 8 +- go.sum | 10 + pkg/generators/client_release.go | 292 ++++++++++++++++++ pkg/generators/client_release_test.go | 209 +++++++++++++ 23 files changed, 1592 insertions(+), 94 deletions(-) create mode 100644 cmd/doctor_workspace_binding_values_test.go create mode 100644 cmd/doctor_workspace_exclusions_test.go create mode 100644 cmd/generate/runnable_bindings_freshness_test.go create mode 100644 cmd/publish/client_versions_test.go create mode 100644 cmd/publish/publication_credentials_test.go create mode 100644 cmd/publish/release_checksums.go create mode 100644 cmd/publish/release_checksums_test.go create mode 100644 docs/publish-client-versions.md create mode 100644 docs/publish-verification.md create mode 100644 pkg/generators/client_release.go create mode 100644 pkg/generators/client_release_test.go diff --git a/cmd/doctor_workspace.go b/cmd/doctor_workspace.go index 6717e468..8b0ed7fd 100644 --- a/cmd/doctor_workspace.go +++ b/cmd/doctor_workspace.go @@ -175,7 +175,7 @@ func workspaceReadiness(ctx context.Context, opts workspaceReadinessOptions) *wo // behind the materialization gate like every other check that loads them. checkRunnableBindingDeclines(ctx, ws, env, report) - scope, requiredBy := checkScope(ctx, ws, opts.module, opts.service, report) + scope, requiredBy := checkScope(ctx, ws, env, opts.module, opts.service, report) if scope == nil { return report } @@ -790,7 +790,11 @@ func checkSecretProviders(env *environments.Environment, report *workspaceReadin // acts on one unit must not be judged by a sibling unit's missing // configuration. --service wins when both are given, being the narrower of the // two. -func checkScope(ctx context.Context, ws *resources.Workspace, moduleName, serviceName string, report *workspaceReadinessReport) ([]*resources.Service, map[string][]string) { +func checkScope(ctx context.Context, ws *resources.Workspace, env *environments.Environment, moduleName, serviceName string, report *workspaceReadinessReport) ([]*resources.Service, map[string][]string) { + if err := environments.ValidateEnvironmentExclusions(ctx, ws, env); err != nil { + report.add(codeWorkspaceInvalid, "services", "fail", err.Error(), "correct the selected environment's exclude-services declarations") + return nil, nil + } services, err := ws.LoadServices(ctx) if err != nil { report.add(codeWorkspaceInvalid, "services", "fail", @@ -841,6 +845,21 @@ func checkScope(ctx context.Context, ws *resources.Workspace, moduleName, servic scope = []*resources.Service{svc} report.Service = serviceUnique(svc) } + excluded := env.ExcludedServiceUniques() + selected := make([]*resources.Service, 0, len(scope)) + for _, svc := range scope { + if slices.Contains(excluded, serviceUnique(svc)) { + if serviceName != "" { + report.add(codeServiceNotFound, "service", "fail", + fmt.Sprintf("environment %q excludes %s", env.Name, serviceUnique(svc)), + "select a service included in this environment") + return nil, nil + } + continue + } + selected = append(selected, svc) + } + scope = selected requiredBy := make(map[string][]string) for _, svc := range scope { for _, dep := range svc.WorkspaceConfigurationDependencies { @@ -1099,6 +1118,12 @@ func checkConfigurationSources(ctx context.Context, ws *resources.Workspace, env if checked != nil { for _, info := range checked.Infos { if _, authored := byName[info.Name]; authored { + // Generated binding decisions must judge the values the + // invocation delivers, including Core-applied overrides. + // Keep the authored inventory for provenance and conflicts. + if info.Name == generate.RunnableBindingsGroup { + byName[info.Name] = info + } continue } byName[info.Name] = info @@ -1261,7 +1286,7 @@ func checkRequiredWorkspaceConfiguration(ctx context.Context, ws *resources.Work // no runnable binding, so the group absent or holding no value is its answer. // A selected binding the group does not hold fails, naming the generation // that writes it. It reports whether it judged the group; a group that holds -// every selected binding is left to the checks every group gets. +// every selected binding must also match the generator's prepared values. func checkRunnableBindingsGroup(ctx context.Context, ws *resources.Workspace, env *environments.Environment, name string, info *basev0.ConfigurationInformation, relCfgDir, requiredBy string, report *workspaceReadinessReport) bool { selection, err := generate.SelectRunnableBindings(ctx, ws, env) if err != nil { @@ -1294,6 +1319,11 @@ func checkRunnableBindingsGroup(ctx context.Context, ws *resources.Workspace, en report.add("", "workspace configuration "+name, "ok", fmt.Sprintf("environment %s prepares no runnable binding, so %q holds none (required by %s)", env.Name, name, requiredBy), "") default: + if err := generate.CheckRunnableBindings(ctx, ws, env, info); err != nil { + report.add(codeConfigurationInvalid, "workspace configuration "+name, checkStatusFail, + fmt.Sprintf("cannot use generated runnable bindings: %v (required by %s)", err, requiredBy), remedy) + return true + } return false } return true diff --git a/cmd/doctor_workspace_binding_values_test.go b/cmd/doctor_workspace_binding_values_test.go new file mode 100644 index 00000000..5a399d91 --- /dev/null +++ b/cmd/doctor_workspace_binding_values_test.go @@ -0,0 +1,73 @@ +package cmd + +import ( + "context" + "os" + "path/filepath" + "strings" + "testing" + + "github.com/codefly-dev/cli/cmd/deploy" + "github.com/codefly-dev/cli/cmd/generate" + "github.com/codefly-dev/core/resources" + "github.com/stretchr/testify/require" +) + +func TestDoctorUsesEffectiveRunnableValueAndPreservesAuthoredFile(t *testing.T) { + artifact, err := os.ReadFile("generate/testdata/solution-runnables/api.swagger.json") + require.NoError(t, err) + dir := singleServiceWorkspace(t, testWorkspaceYAMLLocalDeclared, []string{generate.RunnableBindingsGroup}, map[string]string{ + "modules/backend/services/api/service.codefly.yaml": strings.NewReplacer("runtime::service", "codefly:service", "codefly.ai", "codefly.dev").Replace(testServiceYAML("api", generate.RunnableBindingsGroup)) + `endpoints: + - name: http + api: http + visibility: public + exposure: none +`, + "modules/backend/module.codefly.yaml": testModuleYAML("api") + `interface: + endpoints: + - service: api + endpoint: http + visibility: public +`, + "modules/backend/services/api/openapi/api.swagger.json": string(artifact), + }) + t.Chdir(dir) + require.NoError(t, generate.ContractsCmd.RunE(generate.ContractsCmd, []string{"backend"})) + require.NoError(t, generate.RunnablesCmd.RunE(generate.RunnablesCmd, []string{"backend"})) + require.NoError(t, generate.RunnableBindingsCmd.RunE(generate.RunnableBindingsCmd, nil)) + path := filepath.Join(dir, "configurations/local/runnable-bindings.env") + fresh, err := os.ReadFile(path) + require.NoError(t, err) + var key, value string + for _, line := range strings.Split(string(fresh), "\n") { + if !strings.HasPrefix(line, "#") && strings.Contains(line, "=") { + key, value, _ = strings.Cut(line, "=") + break + } + } + require.NotEmpty(t, key) + for _, validOverride := range []bool{false, true} { + t.Run(map[bool]string{false: "stale override", true: "valid replacement"}[validOverride], func(t *testing.T) { + authored, override := fresh, "{}" + if validOverride { + authored = []byte(strings.Replace(string(fresh), key+"="+value, key+"={}", 1)) + override = value + } + require.NoError(t, os.WriteFile(path, authored, 0o600)) + encoded, err := resources.EncodeWorkspaceConfigurationOverrides([]resources.WorkspaceConfigurationOverride{{Name: generate.RunnableBindingsGroup, Key: key, Value: override}}) + require.NoError(t, err) + t.Setenv(resources.WorkspaceConfigurationOverridesEnvironment, encoded) + report := runReadiness(t, workspaceReadinessOptions{dir: dir, module: "backend"}) + if validOverride { + require.Equal(t, readinessStatusReady, report.Status, reportJSON(t, report)) + require.NoError(t, deploy.WorkspaceReadiness(context.Background(), "local", "backend")) + } else { + requireCode(t, report, codeConfigurationInvalid, checkStatusFail) + require.ErrorContains(t, deploy.WorkspaceReadiness(context.Background(), "local", "backend"), "out of date") + } + after, err := os.ReadFile(path) + require.NoError(t, err) + require.Equal(t, authored, after) + }) + } +} diff --git a/cmd/doctor_workspace_exclusions_test.go b/cmd/doctor_workspace_exclusions_test.go new file mode 100644 index 00000000..823774b5 --- /dev/null +++ b/cmd/doctor_workspace_exclusions_test.go @@ -0,0 +1,35 @@ +package cmd + +import ( + "testing" + + "github.com/stretchr/testify/require" +) + +func TestDoctorJudgesOnlyServicesSelectedByTheEnvironment(t *testing.T) { + dir := writeTestWorkspace(t, map[string]string{ + "workspace.codefly.yaml": testWorkspaceYAML + `environments: + - name: local + - name: blue + exclude-services: + - service: backend/local +`, + "modules/backend/module.codefly.yaml": testModuleYAML("api", "local"), + "modules/backend/services/api/service.codefly.yaml": testServiceYAML("api", "active"), + "modules/backend/services/local/service.codefly.yaml": testServiceYAML("local", "fixture-composition"), + "configurations/blue/active.env": "VALUE=present\n", + "configurations/local/active.env": "VALUE=present\n", + }) + for _, module := range []string{"", "backend"} { + report := runReadiness(t, workspaceReadinessOptions{dir: dir, env: "blue", module: module}) + require.Equal(t, readinessStatusReady, report.Status, reportJSON(t, report)) + require.Contains(t, findCheck(report, "services").Message, "1 service(s)") + } + // The same service still imposes its requirements where it is included. + report := runReadiness(t, workspaceReadinessOptions{dir: dir, env: "local"}) + requireCode(t, report, codeConfigurationMissing, checkStatusFail) + // Selecting an excluded root explicitly must not claim it can run there. + report = runReadiness(t, workspaceReadinessOptions{dir: dir, env: "blue", service: "backend/local"}) + require.Equal(t, readinessStatusNotReady, report.Status) + require.Contains(t, reportJSON(t, report), "excludes backend/local") +} diff --git a/cmd/doctor_workspace_runnable_test.go b/cmd/doctor_workspace_runnable_test.go index 41f8d2c3..433bba0b 100644 --- a/cmd/doctor_workspace_runnable_test.go +++ b/cmd/doctor_workspace_runnable_test.go @@ -2,6 +2,10 @@ package cmd import ( "context" + "github.com/codefly-dev/core/resources" + "github.com/stretchr/testify/require" + "os" + "path/filepath" "strings" "testing" @@ -198,6 +202,17 @@ func TestDoctorWorkspaceJudgesTheRunnableBindingsGroupByWhatTheEnvironmentSelect t.Fatalf("want one failure, for the empty auth0 group: %+v", diagnostics) } }) + t.Run("obsolete binding after last operation disappears", func(t *testing.T) { + dir := singleServiceWorkspace(t, testWorkspaceYAML, deps, map[string]string{ + bindings: header + "OBSOLETE__OPERATION={}\n", + "configurations/local/auth0.env": "CLIENT_ID=public-id\n", + }) + report := runReadiness(t, workspaceReadinessOptions{dir: dir}) + diagnostic := requireCode(t, report, codeConfigurationInvalid, checkStatusFail) + if !strings.Contains(diagnostic.Message, "out of date") { + t.Fatalf("want stale output refusal: %+v", diagnostic) + } + }) } // The render's readiness gate is the doctor scoped to the module: a module @@ -214,3 +229,21 @@ func TestRenderReadinessAcceptsAnAbsentRunnableBindingsGroupWhenNoneIsSelected(t t.Fatalf("render was refused by readiness: %v\noutput: %s", err, out) } } + +func TestRunnableReadinessChecksInvocationValuesOverAuthoredGroup(t *testing.T) { + dir := singleServiceWorkspace(t, testWorkspaceYAML, []string{generate.RunnableBindingsGroup}, map[string]string{ + "configurations/local/runnable-bindings.env": "# no derived operations\n", + }) + encoded, err := resources.EncodeWorkspaceConfigurationOverrides([]resources.WorkspaceConfigurationOverride{ + {Name: generate.RunnableBindingsGroup, Key: "OBSOLETE__OPERATION", Value: "{}"}, + }) + require.NoError(t, err) + t.Setenv(resources.WorkspaceConfigurationOverridesEnvironment, encoded) + report := runReadiness(t, workspaceReadinessOptions{dir: dir, module: "backend"}) + requireCode(t, report, codeConfigurationInvalid, checkStatusFail) + t.Chdir(dir) + require.ErrorContains(t, deploy.WorkspaceReadiness(context.Background(), "local", "backend"), "out of date") + raw, err := os.ReadFile(filepath.Join(dir, "configurations/local/runnable-bindings.env")) + require.NoError(t, err) + require.Equal(t, "# no derived operations\n", string(raw)) +} diff --git a/cmd/generate/runnable_bindings.go b/cmd/generate/runnable_bindings.go index b5b1c4c2..5daaf89f 100644 --- a/cmd/generate/runnable_bindings.go +++ b/cmd/generate/runnable_bindings.go @@ -205,7 +205,7 @@ Examples: func init() { RunnableBindingsCmd.Flags().StringVar(&runnableBindingsEnv, "env", "local", "environment supplying resolved addresses and runnable-scope-selections") - RunnableBindingsCmd.Flags().BoolVar(&runnableBindingsCheck, "check", false, "do not write; exit 1 if the on-disk file differs from what would be generated") + RunnableBindingsCmd.Flags().BoolVar(&runnableBindingsCheck, "check", false, "leave binding output unchanged; materialize selected pins, then exit 1 if the output is stale") RunnableBindingsCmd.Flags().StringVar(&runnableBindingsNamingScope, "naming-scope", "", "`scope` addresses resolve in, as codefly run --naming-scope reads it: omitted keeps the environment's, empty clears it") RunnableBindingsCmd.Flags().StringVar(&runnableBindingsCallEndpoint, "call-endpoint", "", "declared HTTP-based endpoint, as `module/service/endpoint`, every prepared call is sent to instead of the owner's listener, overriding the environment's runnable-call-endpoint (empty: each owner's listener; the owner's operation, route, contract and policy are kept)") } @@ -261,6 +261,30 @@ type preparedRunnableBindings struct { content []byte count int declined []string + values map[string]string +} + +// CheckRunnableBindings checks the values a consumer would receive against +// the same preparation used by generate. It reads only; the caller supplies +// the effective configuration, including profile and composed-module selection. +// Comments and ordering are immaterial here; --check also checks file layout. +func CheckRunnableBindings(ctx context.Context, workspace *resources.Workspace, env *environments.Environment, info *basev0.ConfigurationInformation) error { + prepared, err := prepareRunnableBindings(ctx, workspace, env, env.RunnableCallEndpoint) + if err != nil { + return err + } + current := len(info.GetConfigurationValues()) == len(prepared.values) + for _, value := range info.GetConfigurationValues() { + expected, exists := prepared.values[value.GetKey()] + if !exists || value.GetValue() != expected { + current = false + } + delete(prepared.values, value.GetKey()) + } + if !current { + return fmt.Errorf("runnable-bindings is out of date: run `%s`", runnableBindingsRegenerate(env, env.RunnableCallEndpoint)) + } + return nil } // prepareRunnableBindings walks every module of the workspace and prepares a @@ -346,7 +370,7 @@ func prepareRunnableBindings(ctx context.Context, workspace *resources.Workspace for _, key := range keys { out.WriteString(key + "=" + lines[key] + "\n") } - return &preparedRunnableBindings{content: []byte(out.String()), count: len(lines), declined: declined}, nil + return &preparedRunnableBindings{content: []byte(out.String()), count: len(lines), declined: declined, values: lines}, nil } // declinedBindingComment is the line the file carries for a declined key. It diff --git a/cmd/generate/runnable_bindings_freshness_test.go b/cmd/generate/runnable_bindings_freshness_test.go new file mode 100644 index 00000000..17d6aeb9 --- /dev/null +++ b/cmd/generate/runnable_bindings_freshness_test.go @@ -0,0 +1,104 @@ +package generate + +import ( + "context" + "github.com/codefly-dev/cli/pkg/composition" + "os" + "os/exec" + "path/filepath" + "strings" + "testing" + + "github.com/codefly-dev/cli/pkg/environments" + basev0 "github.com/codefly-dev/core/generated/go/codefly/base/v0" + runnablev0 "github.com/codefly-dev/core/generated/go/codefly/runnable/v0" + "github.com/codefly-dev/core/resources" + corerunnable "github.com/codefly-dev/core/runnable" + "github.com/stretchr/testify/require" + "google.golang.org/protobuf/proto" +) + +func TestCheckRunnableBindingsRefusesStaleValuesWithUnchangedKeys(t *testing.T) { + ctx := context.Background() + root, moduleDir := saveRunnableFixture(t, ctx, descriptorSet(t, ingestionFile(conformingOperation())), "0.1.0", connectEndpoint()) + t.Chdir(root) + resetRunnablesFlags(t) + resetRunnableBindingsFlags(t) + require.NoError(t, RunnablesCmd.RunE(RunnablesCmd, []string{"documents"})) + publishFixtureConnectEndpoint(t, moduleDir, "connect", nil) + require.NoError(t, RunnableBindingsCmd.RunE(RunnableBindingsCmd, nil)) + ws, err := resources.LoadWorkspaceFromDir(ctx, root) + require.NoError(t, err) + env, err := environments.Select(ws, "local") + require.NoError(t, err) + target := filepath.Join(root, "configurations", "local", RunnableBindingsGroup+".env") + raw, err := os.ReadFile(target) + require.NoError(t, err) + info := &basev0.ConfigurationInformation{Name: RunnableBindingsGroup} + for _, line := range strings.Split(string(raw), "\n") { + if key, value, found := strings.Cut(line, "="); found && !strings.HasPrefix(line, "#") { + info.ConfigurationValues = append(info.ConfigurationValues, &basev0.ConfigurationValue{Key: key, Value: value}) + } + } + require.Len(t, info.ConfigurationValues, 1) + require.NoError(t, CheckRunnableBindings(ctx, ws, env, info)) + for name, change := range map[string]func(*runnablev0.PreparedBinding){ + "address": func(binding *runnablev0.PreparedBinding) { binding.Call.Address += "/stale" }, + "policy": func(binding *runnablev0.PreparedBinding) { binding.Policy.MaxAttempts++ }, + "contract": func(binding *runnablev0.PreparedBinding) { binding.Contract.Input.Fields[0].Name += "Old" }, + } { + t.Run(name, func(t *testing.T) { + stale := proto.CloneOf(info) + binding, err := corerunnable.DecodePrepared([]byte(stale.ConfigurationValues[0].Value)) + require.NoError(t, err) + change(binding) + binding.ContractDigest = "" + value, err := corerunnable.EncodePrepared(binding) + require.NoError(t, err) + stale.ConfigurationValues[0].Value = string(value) + require.ErrorContains(t, CheckRunnableBindings(ctx, ws, env, stale), "out of date") + }) + } + extra := proto.CloneOf(info) + extra.ConfigurationValues = append(extra.ConfigurationValues, &basev0.ConfigurationValue{Key: "OBSOLETE", Value: "old"}) + require.ErrorContains(t, CheckRunnableBindings(ctx, ws, env, extra), "out of date") + env.NamingScope = "another-run" + require.ErrorContains(t, CheckRunnableBindings(ctx, ws, env, info), "out of date") + // Checking never rewrites the consumer's file. + after, err := os.ReadFile(target) + require.NoError(t, err) + require.Equal(t, raw, after) +} + +func TestFreshCheckoutBindingCheckMaterializes(t *testing.T) { + t.Setenv(resources.CodeflyHomeEnv, t.TempDir()) + source := t.TempDir() + git := func(args ...string) { + cmd := exec.Command("git", args...) + cmd.Dir = source + if out, err := cmd.CombinedOutput(); err != nil { + t.Fatalf("git %v: %v: %s", args, err, out) + } + } + git("init", "--quiet") + git("config", "user.email", "review@example.invalid") + git("config", "user.name", "Review Fixture") + require.NoError(t, os.WriteFile(filepath.Join(source, resources.ModuleConfigurationName), []byte("name: sample\n"), 0600)) + git("add", ".") + git("-c", "commit.gpgsign=false", "commit", "--quiet", "-m", "fixture") + git("-c", "tag.gpgSign=false", "tag", "v0.0.1") + root := t.TempDir() + manifest := "name: review\nlayout: modules\n" + composition.ModuleResolutionKey + ":\n sample: git\nmodules:\n - name: sample\n source: file://" + source + "\n version: v0.0.1\n" + require.NoError(t, os.WriteFile(filepath.Join(root, resources.WorkspaceConfigurationName), []byte(manifest), 0600)) + t.Chdir(root) + resetRunnableBindingsFlags(t) + runnableBindingsCheck = true + require.NoFileExists(t, filepath.Join(root, resources.LocalOverlayConfigurationName)) + require.NoError(t, RunnableBindingsCmd.RunE(RunnableBindingsCmd, nil)) + require.FileExists(t, filepath.Join(root, resources.LocalOverlayConfigurationName)) + require.NoFileExists(t, filepath.Join(root, "configurations", "local", "runnable-bindings.env")) + ws, err := resources.LoadWorkspaceFromDir(context.Background(), root) + require.NoError(t, err) + _, err = ws.LoadModules(context.Background()) + require.NoError(t, err) +} diff --git a/cmd/publish/agent_release.go b/cmd/publish/agent_release.go index 20ac58f1..ebff2971 100644 --- a/cmd/publish/agent_release.go +++ b/cmd/publish/agent_release.go @@ -4,7 +4,6 @@ import ( "archive/tar" "compress/gzip" "context" - "crypto/sha256" "encoding/json" "fmt" "io" @@ -249,7 +248,7 @@ func collectLoaderAssets(reg *resources.AgentKindRegistration, ciOutput, name, v // expectedLoaderAssets names the archive every required loader platform must // carry on a release the agent's own workflow publishes. Nothing is staged: -// the workflow is the sole producer of those bytes, and verifyWorkflowRelease +// the workflow is the sole producer of those bytes, and verifyPublishedArchives // establishes them against the checksums that workflow published. func expectedLoaderAssets(reg *resources.AgentKindRegistration, name, version string) []loaderAsset { assets := make([]loaderAsset, 0, len(loaderPlatforms)) @@ -322,6 +321,10 @@ func createAndUploadRelease(ctx context.Context, client *github.Client, owner, r // publishReleaseAssets is createAndUploadRelease for an explicit release // request — the one upload path a semantic release and a dev build share. func publishReleaseAssets(ctx context.Context, client *github.Client, owner, repo string, request *github.CreateReleaseRequest, assets []loaderAsset) error { + files, err := stageReleaseChecksums(assets) + if err != nil { + return err + } release, err := getOrCreateRelease(ctx, client, owner, repo, request) if err != nil { return err @@ -330,15 +333,16 @@ func publishReleaseAssets(ctx context.Context, client *github.Client, owner, rep for _, asset := range release.Assets { existing[asset.GetName()] = asset } - for _, asset := range assets { - files := []string{asset.archivePath} - if asset.sbomPath != "" { - files = append(files, asset.sbomPath) + // Refuse a conflicting retry before uploading any new file, especially + // checksums describing bytes that this release can never accept. + for _, file := range files { + if err := validateExistingReleaseAsset(file, existing); err != nil { + return err } - for _, file := range files { - if err := uploadReleaseAsset(ctx, client, owner, repo, release.GetID(), file, existing); err != nil { - return err - } + } + for _, file := range files { + if err := uploadReleaseAsset(ctx, client, owner, repo, release.GetID(), file, existing); err != nil { + return err } } return nil @@ -397,15 +401,8 @@ func uploadReleaseAsset(ctx context.Context, client *github.Client, owner, repo return fmt.Errorf("open release asset %s: %w", path, err) } defer file.Close() - if asset, ok := existing[name]; ok { - digest := sha256.New() - if _, err := io.Copy(digest, file); err != nil { - return err - } - if asset.GetDigest() != fmt.Sprintf("sha256:%x", digest.Sum(nil)) { - return fmt.Errorf("release asset %s already exists with different or unverifiable bytes; published assets cannot be replaced", name) - } - return nil + if _, ok := existing[name]; ok { + return validateExistingReleaseAsset(path, existing) } if _, _, err := client.Repositories.UploadReleaseAsset(ctx, owner, repo, releaseID, &github.UploadOptions{Name: name}, file); err != nil { return fmt.Errorf("upload release asset %s: %w", name, err) @@ -650,7 +647,6 @@ func (r *agentReleaser) beforeCommit(ctx context.Context, newTag string) error { } func (r *agentReleaser) afterPush(ctx context.Context, newTag string) error { - version := strings.TrimPrefix(newTag, "v") owner := gh.Owner(r.publisher) repo := r.reg.GitHubRepository(r.name) client, err := gh.NewClient() @@ -673,7 +669,25 @@ func (r *agentReleaser) afterPush(ctx context.Context, newTag string) error { if err := publishReleaseAssets(ctx, client, owner, repo, releaseRequest(newTag, r.dev), r.assets); err != nil { return err } - return verifyReleaseAssets(ctx, client, r.reg, r.publisher, r.name, version, r.assets) + return r.verifyPublication(ctx, client, owner, repo, newTag) +} + +// verifyPublication separates the publisher's integrity proof from installer +// reachability. The latter uses Core's credential rule, never a gh CLI login +// that publishing may use. Public releases remain installable anonymously. +func (r *agentReleaser) verifyPublication(ctx context.Context, publisherClient *github.Client, owner, repo, tag string) error { + if err := verifyPublishedArchives(ctx, publisherClient, owner, repo, tag, r.assets); err != nil { + return err + } + options := []github.ClientOptionsFunc{} + if token := manager.GitHubReleaseToken(); token != "" { + options = append(options, github.WithAuthToken(token)) + } + installerClient, err := github.NewClient(options...) + if err != nil { + return err + } + return verifyReleaseAssets(ctx, installerClient, r.reg, r.publisher, r.name, strings.TrimPrefix(tag, "v"), r.assets) } // sourceTagReleaser publishes an immutable source release: it runs diff --git a/cmd/publish/client_versions_test.go b/cmd/publish/client_versions_test.go new file mode 100644 index 00000000..3093c5b9 --- /dev/null +++ b/cmd/publish/client_versions_test.go @@ -0,0 +1,138 @@ +package publish_test + +import ( + "context" + "errors" + "os" + "path/filepath" + "strings" + "testing" + + "github.com/codefly-dev/core/composition" + "github.com/stretchr/testify/require" +) + +// clientReleaseRepo includes a generated client outside libraries/, a foreign +// client, and an ignored cache. Only a tracked source owned by the releasing +// package moves, and its copied contract bytes remain identical. +func clientReleaseRepo(t *testing.T) (string, map[string]string) { + t.Helper() + dir, _, originals := moduleAgentRepo(t, "0.2.0") + contract := `{"openapi":"3.0.0","paths":{"/ready":{"get":{}}}}` + digest := composition.APIContractDigest([]byte(contract)) + endpoint := composition.APIContractEndpoint{ + Service: "api", Endpoint: "rest", API: "rest", Kind: composition.APIContractKindOpenAPI, + Package: "payments", Path: "contracts/api/api/rest/openapi.json", Digest: digest, + Routes: []composition.APIContractRoute{{Method: "GET", Path: "/ready"}}, + } + catalog := composition.APIContractCatalog{Schema: composition.APIContractCatalogSchema, Package: "acme/payments", Version: "0.0.0", Endpoints: []composition.APIContractEndpoint{endpoint}} + data, err := catalog.CanonicalBytes() + require.NoError(t, err) + packagePath := "modules/payments/module.package.codefly.yaml" + originals[packagePath] = strings.Replace(originals[packagePath], " - name: api\n", " - name: api\n endpoints: [rest]\n api-contracts:\n - endpoint: rest\n kind: openapi\n package: payments\n path: "+endpoint.Path+"\n digest: "+digest+"\n", 1) + originals["modules/payments/"+composition.APIContractCatalogFileName] = string(data) + originals["modules/payments/"+endpoint.Path] = contract + catalog.Endpoints[0].Path = "openapi.json" + data, err = catalog.CanonicalBytes() + require.NoError(t, err) + originals["clients/payments/contract/catalog.codefly.json"] = string(data) + originals["clients/payments/contract/openapi.json"] = contract + originals["clients/payments/library.codefly.yaml"] = `kind: library +name: payments +version: 0.0.0 +languages: + - name: typescript + path: typescript/ + - name: python + path: python/ +sources: + - package: acme/payments + version: 0.0.0 + module: payments + service: api + endpoint: rest + contract-digest: ` + digest + ` +generated-by: + codefly: 0.1.219 + companion: preserved + facade: false +` + originals["clients/payments/typescript/package.json"] = `{"name":"@acme/payments","version":"0.0.0","description":"preserved"}` + originals["clients/payments/python/pyproject.toml"] = "[project]\nname = \"payments\"\nversion = \"0.0.0\"\n" + originals["libraries/foreign/library.codefly.yaml"] = "kind: library\nname: foreign\nversion: 1.0.0\nsources:\n - package: acme/foreign\n version: 1.0.0\n" + originals[".gitignore"] = ".cache/\n" + for relative, content := range originals { + path := filepath.Join(dir, relative) + require.NoError(t, os.MkdirAll(filepath.Dir(path), 0o755)) + require.NoError(t, os.WriteFile(path, []byte(content), 0o600)) + } + require.NoError(t, os.MkdirAll(filepath.Join(dir, ".cache"), 0o755)) + require.NoError(t, os.WriteFile(filepath.Join(dir, ".cache", "library.codefly.yaml"), []byte("invalid YAML: ["), 0o600)) + require.NoError(t, runGit(dir, "add", ".")) + require.NoError(t, runGit(dir, "commit", "-m", "generated clients")) + require.NoError(t, runGit(dir, "push", "origin", "main")) + return dir, originals +} + +func TestModuleReleaseStampsTrackedClientsAtOwnerVersion(t *testing.T) { + dir, originals := clientReleaseRepo(t) + tag, err := moduleEngine(t, dir, "patch").Release(context.Background()) + require.NoError(t, err) + require.Equal(t, "v0.2.1", tag) + require.Contains(t, readRel(t, dir, "clients/payments/library.codefly.yaml"), "version: 0.2.1") + require.NotContains(t, readRel(t, dir, "clients/payments/library.codefly.yaml"), "version: 0.0.0") + require.Contains(t, readRel(t, dir, "clients/payments/library.codefly.yaml"), "companion: preserved") + require.Contains(t, readRel(t, dir, "clients/payments/contract/catalog.codefly.json"), `"version": "0.2.1"`) + require.Equal(t, strings.Replace(originals["clients/payments/typescript/package.json"], `"0.0.0"`, `"0.2.1"`, 1), readRel(t, dir, "clients/payments/typescript/package.json")) + require.Contains(t, readRel(t, dir, "clients/payments/python/pyproject.toml"), `version = "0.2.1"`) + for _, path := range []string{"clients/payments/contract/openapi.json", "libraries/foreign/library.codefly.yaml"} { + require.Equal(t, originals[path], readRel(t, dir, path)) + } + require.Equal(t, "invalid YAML: [", readRel(t, dir, ".cache/library.codefly.yaml")) + require.NoError(t, runGit(dir, "diff", "--exit-code", "HEAD"), "every stamped path belongs to the release commit") +} + +func TestModuleClientReleaseFailureRestoresEveryStamp(t *testing.T) { + dir, originals := clientReleaseRepo(t) + engine := moduleEngine(t, dir, "patch") + failure := errors.New("release gate rejected candidate") + engine.BeforeCommit = func(context.Context, string) error { + require.Contains(t, readRel(t, dir, "clients/payments/contract/catalog.codefly.json"), `"version": "0.2.1"`) + require.Contains(t, readRel(t, dir, "clients/payments/typescript/package.json"), `"version":"0.2.1"`) + return failure + } + _, err := engine.Release(context.Background()) + require.ErrorIs(t, err, failure) + for path, before := range originals { + require.Equal(t, before, readRel(t, dir, path), "%s was not restored", path) + } + require.NoError(t, runGit(dir, "diff", "--exit-code", "HEAD")) +} + +func TestModuleClientReleaseRefusesEscapingLanguagePath(t *testing.T) { + for _, symlink := range []bool{false, true} { + t.Run(map[bool]string{false: "traversal", true: "symlink"}[symlink], func(t *testing.T) { + dir, originals := clientReleaseRepo(t) + outside := t.TempDir() + outsideManifest := filepath.Join(outside, "package.json") + require.NoError(t, os.WriteFile(outsideManifest, []byte(`{"version":"9.0.0"}`), 0o600)) + if symlink { + require.NoError(t, os.Remove(filepath.Join(dir, "clients/payments/typescript/package.json"))) + require.NoError(t, os.Symlink(outsideManifest, filepath.Join(dir, "clients/payments/typescript/package.json"))) + } else { + manifest := strings.Replace(originals["clients/payments/library.codefly.yaml"], "path: typescript/", "path: ../../../../escape", 1) + require.NoError(t, os.WriteFile(filepath.Join(dir, "clients/payments/library.codefly.yaml"), []byte(manifest), 0o600)) + } + require.NoError(t, runGit(dir, "add", ".")) + require.NoError(t, runGit(dir, "commit", "-m", "escaping path")) + require.NoError(t, runGit(dir, "push", "origin", "main")) + _, err := moduleEngine(t, dir, "patch").Release(context.Background()) + require.ErrorContains(t, err, "escapes") + bytes, err := os.ReadFile(outsideManifest) + require.NoError(t, err) + require.Equal(t, `{"version":"9.0.0"}`, string(bytes)) + require.Equal(t, originals["agent.codefly.yaml"], readRel(t, dir, "agent.codefly.yaml")) + require.NoError(t, runGit(dir, "diff", "--exit-code", "HEAD")) + }) + } +} diff --git a/cmd/publish/module_services.go b/cmd/publish/module_services.go index 896ed24d..bddd9b5b 100644 --- a/cmd/publish/module_services.go +++ b/cmd/publish/module_services.go @@ -7,8 +7,10 @@ import ( "os" "path/filepath" "sort" + "strings" "github.com/codefly-dev/cli/cmd/generate" + "github.com/codefly-dev/cli/pkg/generators" "github.com/codefly-dev/core/composition" "github.com/codefly-dev/core/resources" "gopkg.in/yaml.v3" @@ -99,6 +101,8 @@ func resolveModuleRelease(ctx context.Context, agentManifest, repoRoot string) ( return contained, nil } var rekey []*resources.Module + packages := map[string]generators.ClientReleasePackage{} + packageIDs := map[*resources.Module]string{} for _, ref := range workspace.Modules { if ref.Source != "" || ref.Module != "" || ref.Version != "" { fmt.Printf("==> module %s is a pinned reference, released from its own source; its services keep their versions\n", ref.Name) @@ -152,23 +156,133 @@ func resolveModuleRelease(ctx context.Context, agentManifest, repoRoot string) ( } release.generated = append(release.generated, contained) } + id, producer, loadErr := loadClientReleasePackage(module.Dir()) + if loadErr != nil { + return nil, loadErr + } + packages[id] = producer + packageIDs[module] = id rekey = append(rekey, module) } + clients, err := planReleasedClients(ctx, repoRoot, packages) + if err != nil { + return nil, err + } + for _, client := range clients { + release.generated = append(release.generated, client.Paths...) + } sort.Strings(release.manifests) - if len(rekey) > 0 { - name := workspace.Name - release.regenerate = func(ctx context.Context) error { - for _, module := range rekey { - if err := generate.RekeyReleasedContracts(ctx, name, dir, module); err != nil { - return err - } - } - return nil - } + release.regenerate = func(ctx context.Context) error { + return regenerateModuleClients(ctx, workspace.Name, dir, rekey, packageIDs, clients) } return release, nil } +func regenerateModuleClients(ctx context.Context, name, dir string, modules []*resources.Module, packageIDs map[*resources.Module]string, clients []*generators.ClientRelease) error { + versions := map[string]string{} + for _, module := range modules { + if err := generate.RekeyReleasedContracts(ctx, name, dir, module); err != nil { + return err + } + version, err := readVersion(filepath.Join(module.Dir(), composition.PackageManifestFileName)) + if err != nil { + return err + } + versions[packageIDs[module]] = version.String() + } + for _, client := range clients { + if err := client.Write(ctx, versions); err != nil { + return err + } + } + return nil +} + +// A module may not export contracts yet. Read its schema identity without +// requiring an exported artifact tree, and fully verify catalogs that exist. +func loadClientReleasePackage(dir string) (string, generators.ClientReleasePackage, error) { + producer := generators.ClientReleasePackage{} + resolved, err := filepath.EvalSymlinks(dir) + if err != nil { + return "", producer, err + } + producer.Directory = resolved + data, err := os.ReadFile(filepath.Join(dir, composition.PackageManifestFileName)) //nolint:gosec // contained module package + if err != nil { + return "", producer, err + } + var manifest composition.PackageManifest + if err = yaml.Unmarshal(data, &manifest); err != nil { + return "", producer, err + } + if err = manifest.Validate(); err != nil { + return "", producer, err + } + producer.Catalog, err = composition.LoadAPIContractCatalog(dir) + if errors.Is(err, os.ErrNotExist) { + return manifest.ID, producer, nil + } + if err != nil { + return "", producer, err + } + if producer.Catalog.Package != manifest.ID || producer.Catalog.Version != manifest.Version { + return "", producer, fmt.Errorf("module %s catalog disagrees with its package; regenerate contracts before release", dir) + } + return manifest.ID, producer, composition.ValidatePackageAPIContracts(dir, &manifest, producer.Catalog) +} + +// planReleasedClients discovers only committed library manifests, wherever their +// owners placed them. Untracked caches, foreign sources and hand-written +// libraries have no release-owned source to stamp. +func planReleasedClients(ctx context.Context, repoRoot string, packages map[string]generators.ClientReleasePackage) ([]*generators.ClientRelease, error) { + if len(packages) == 0 { + return nil, nil + } + root, err := filepath.EvalSymlinks(repoRoot) + if err != nil { + return nil, err + } + files, err := (&Engine{WorkDir: root}).git(ctx, "ls-files", "-z") + if err != nil { + return nil, err + } + tracked := map[string]bool{} + for _, path := range strings.Split(files, "\x00") { + tracked[filepath.Join(root, path)] = true + } + check := func(path string) error { + relative, err := filepath.Rel(root, path) + if err != nil { + return err + } + contained, err := releaseManifestPath(root, relative) + if err != nil { + return err + } + if contained != path { + return fmt.Errorf("client release path %s follows a symlink", path) + } + if !tracked[path] { + return fmt.Errorf("client release path %s is not git-tracked", path) + } + return nil + } + var plans []*generators.ClientRelease + for _, relative := range strings.Split(files, "\x00") { + if filepath.Base(relative) != resources.LibraryConfigurationName { + continue + } + plan, err := generators.PlanClientRelease(filepath.Dir(filepath.Join(root, relative)), packages, check) + if err != nil { + return nil, fmt.Errorf("release client %s: %w", relative, err) + } + if plan != nil { + plans = append(plans, plan) + } + } + return plans, nil +} + // agentKind reads only the kind an agent manifest declares. It is lenient on // purpose: the release gate validates the rest of the manifest, and this only // decides whether there is a module release to carry. diff --git a/cmd/publish/publication_credentials_test.go b/cmd/publish/publication_credentials_test.go new file mode 100644 index 00000000..4f96462f --- /dev/null +++ b/cmd/publish/publication_credentials_test.go @@ -0,0 +1,131 @@ +package publish + +import ( + "context" + "fmt" + "io" + "net/http" + "os" + "path/filepath" + "strings" + "testing" + "time" + + "github.com/codefly-dev/cli/pkg/gh" + "github.com/codefly-dev/core/resources" + "github.com/stretchr/testify/require" +) + +type publicationTransport func(*http.Request) (*http.Response, error) + +func (f publicationTransport) RoundTrip(r *http.Request) (*http.Response, error) { return f(r) } + +// Exercise both real afterPush branches, including the workflow tag/commit +// match. A gh login can publish a private release but must not certify that an +// installer without that credential can fetch it; public downloads need none. +func TestPublicationOwnersUseInstallerCredentials(t *testing.T) { + for _, owner := range []string{publicationCLI, publicationWorkflow} { + for _, scenario := range []string{"anonymous public", "private with only gh login", "GH_TOKEN", "GITHUB_TOKEN wins", "bad archive"} { + t.Run(owner+"/"+scenario, func(t *testing.T) { + t.Setenv("GITHUB_TOKEN", "") + t.Setenv("GH_TOKEN", "") + t.Setenv("GITHUB_API_URL", "") + bin := t.TempDir() + require.NoError(t, os.WriteFile(filepath.Join(bin, "gh"), []byte("#!/bin/sh\nprintf publisher-login"), 0700)) + t.Setenv("PATH", bin+string(os.PathListSeparator)+os.Getenv("PATH")) + expectedInstaller := "" + if scenario == "GH_TOKEN" { + t.Setenv("GH_TOKEN", "installer-gh") + expectedInstaller = "Bearer installer-gh" + } + if scenario == "GITHUB_TOKEN wins" { + t.Setenv("GH_TOKEN", "ignored-gh") + t.Setenv("GITHUB_TOKEN", "installer-github") + expectedInstaller = "Bearer installer-github" + } + reg, err := resources.AgentKindRegistrationFor(resources.AgentKind("codefly:service")) + require.NoError(t, err) + target := platform{os: "linux", arch: "amd64"} + name := loaderArchiveName(®, "go", "0.0.16", target) + assets := stageAssets(t, name) + assets[0].platform = target + files, err := stageReleaseChecksums(assets) + require.NoError(t, err) + payloads := make([]string, len(files)) + var entries []string + for i, file := range files { + raw, err := os.ReadFile(file) + require.NoError(t, err) + payloads[i] = string(raw) + url := "https://github.com/codefly-dev/service-go/releases/download/v0.0.16/" + filepath.Base(file) + entries = append(entries, fmt.Sprintf(`{"id":%d,"name":%q,"size":%d,"digest":%q,"browser_download_url":%q}`, i+1, filepath.Base(file), len(raw), digestOf(string(raw)), url)) + } + dir, _, _ := releaseRepo(t, "0.0.16") + gitIn(t, dir, "tag", "v0.0.16") + head := gitIn(t, dir, "rev-parse", "HEAD") + var installerSeen, publisherSeen bool + original := http.DefaultTransport + http.DefaultTransport = publicationTransport(func(req *http.Request) (*http.Response, error) { + require.Equal(t, "api.github.com", req.URL.Host, "unexpected network call") + require.Equal(t, http.MethodGet, req.Method, "complete release retry must not mutate") + auth := req.Header.Get("Authorization") + if auth == "Bearer publisher-login" { + publisherSeen = true + } + if auth == expectedInstaller { + installerSeen = true + } + status, contentType, body := http.StatusOK, "application/json", "" + if scenario == "private with only gh login" && auth == "" { + status, body = http.StatusNotFound, `{"message":"not found"}` + } else { + switch req.URL.Path { + case "/repos/codefly-dev/service-go/releases/tags/v0.0.16": + body = fmt.Sprintf(`{"id":42,"tag_name":"v0.0.16","assets":[%s]}`, strings.Join(entries, ",")) + case "/repos/codefly-dev/service-go/releases/assets/1": + contentType, body = "application/octet-stream", payloads[0] + if scenario == "bad archive" { + body = "corrupted" + } + case "/repos/codefly-dev/service-go/releases/assets/2": + contentType, body = "application/octet-stream", payloads[1] + case "/repos/codefly-dev/service-go/actions/workflows/release.yml/runs": + body = fmt.Sprintf(`{"workflow_runs":[{"id":1,"head_sha":%q,"head_branch":"v0.0.16","event":"push","status":"completed","conclusion":"success"}]}`, head) + default: + t.Fatalf("unexpected request: %s", req.URL) + } + } + return &http.Response{StatusCode: status, Header: http.Header{"Content-Type": {contentType}}, Body: io.NopCloser(strings.NewReader(body)), Request: req}, nil + }) + t.Cleanup(func() { http.DefaultTransport = original }) + r := &agentReleaser{agentDir: dir, reg: ®, publisher: "codefly.dev", name: "go", assets: assets, publication: agentPublication{Owner: owner, Workflow: "release.yml"}} + // afterPush detaches a caller timeout after a tag ships. For the + // reachability refusal, call its shared publication verifier with + // a short context so this regression never sleeps through retries. + if scenario == "private with only gh login" { + ctx, cancel := context.WithTimeout(t.Context(), 100*time.Millisecond) + defer cancel() + client, err := gh.NewClient() + require.NoError(t, err) + err = r.verifyPublication(ctx, client, "codefly-dev", "service-go", "v0.0.16") + require.Error(t, err) + } else { + err = r.afterPush(t.Context(), "v0.0.16") + if scenario == "bad archive" { + require.ErrorIs(t, err, errAssetVerdict) + } else { + require.NoError(t, err) + } + } + if expectedInstaller == "" { + require.True(t, publisherSeen, "fixture must exercise gh login publication") + } + if scenario == "bad archive" { + require.False(t, installerSeen, "invalid archive must fail before installer reachability") + } else { + require.True(t, installerSeen, "must check actual installer credential") + } + }) + } + } +} diff --git a/cmd/publish/release_checksums.go b/cmd/publish/release_checksums.go new file mode 100644 index 00000000..c3f5af52 --- /dev/null +++ b/cmd/publish/release_checksums.go @@ -0,0 +1,75 @@ +package publish + +import ( + "crypto/sha256" + "fmt" + "io" + "os" + "path/filepath" + "sort" + "strings" + + "github.com/google/go-github/v89/github" +) + +// stageReleaseChecksums supplies the same producer evidence as workflow-owned +// publication. Upload archives first and their complete checksum list last; +// retries use the same sorted bytes and never replace an existing asset. +func stageReleaseChecksums(assets []loaderAsset) ([]string, error) { + var files []string + for _, asset := range assets { + files = append(files, asset.archivePath) + if asset.sbomPath != "" { + files = append(files, asset.sbomPath) + } + } + if len(files) == 0 { + return nil, fmt.Errorf("a release publishes no assets to checksum") + } + sort.Slice(files, func(i, j int) bool { return filepath.Base(files[i]) < filepath.Base(files[j]) }) + var checksums strings.Builder + for i, file := range files { + name := filepath.Base(file) + if name == "checksums.txt" || (i > 0 && name == filepath.Base(files[i-1])) { + return nil, fmt.Errorf("duplicate release asset name %q", name) + } + digest, err := releaseFileDigest(file) + if err != nil { + return nil, err + } + fmt.Fprintf(&checksums, "%s %s\n", digest, name) + } + target := filepath.Join(filepath.Dir(files[0]), "checksums.txt") + if err := os.WriteFile(target, []byte(checksums.String()), 0o600); err != nil { + return nil, err + } + return append(files, target), nil +} + +func releaseFileDigest(path string) (string, error) { + file, err := os.Open(path) + if err != nil { + return "", err + } + defer file.Close() + digest := sha256.New() + if _, err := io.Copy(digest, file); err != nil { + return "", err + } + return fmt.Sprintf("%x", digest.Sum(nil)), nil +} + +func validateExistingReleaseAsset(path string, existing map[string]*github.ReleaseAsset) error { + asset, exists := existing[filepath.Base(path)] + if !exists { + return nil + } + digest, err := releaseFileDigest(path) + if err != nil { + return err + } + if asset.GetDigest() != "sha256:"+digest { + return fmt.Errorf("release asset %s already exists with different or unverifiable bytes; published assets cannot be replaced", filepath.Base(path)) + } + return nil +} diff --git a/cmd/publish/release_checksums_test.go b/cmd/publish/release_checksums_test.go new file mode 100644 index 00000000..f9da0674 --- /dev/null +++ b/cmd/publish/release_checksums_test.go @@ -0,0 +1,102 @@ +package publish + +import ( + "crypto/sha256" + "fmt" + "os" + "path/filepath" + "testing" + + "github.com/stretchr/testify/require" +) + +func TestStageReleaseChecksumsIsDeterministicAndComplete(t *testing.T) { + assets := stageAssets(t, "z.tar.gz", "a.tar.gz") + assets[0].sbomPath = stageFile(t, filepath.Dir(assets[0].archivePath), "z.cdx.json") + files, err := stageReleaseChecksums(assets) + require.NoError(t, err) + var names []string + for _, file := range files { + names = append(names, filepath.Base(file)) + } + require.Equal(t, []string{"a.tar.gz", "z.cdx.json", "z.tar.gz", "checksums.txt"}, names) + var expected string + for _, name := range names[:3] { + expected += fmt.Sprintf("%x %s\n", sha256.Sum256([]byte("payload-"+name)), name) + } + raw, err := os.ReadFile(files[3]) + require.NoError(t, err) + require.Equal(t, expected, string(raw)) + _, err = stageReleaseChecksums([]loaderAsset{assets[1], assets[0]}) + require.NoError(t, err) + retried, err := os.ReadFile(files[3]) + require.NoError(t, err) + require.Equal(t, raw, retried) +} + +func TestStageReleaseChecksumsRefusesAmbiguousNames(t *testing.T) { + for _, names := range [][]string{{"same.tar.gz", "same.tar.gz"}, {"checksums.txt"}} { + _, err := stageReleaseChecksums(stageAssets(t, names...)) + require.ErrorContains(t, err, "duplicate release asset name") + } +} + +func TestReleaseRetryPreflightsEveryImmutableAssetBeforeMutation(t *testing.T) { + const tag = "v0.0.16" + for _, scenario := range []string{"complete identical retry", "late archive conflict", "checksum conflict"} { + t.Run(scenario, func(t *testing.T) { + assets := stageAssets(t, "a.tar.gz", "z.tar.gz") + files, err := stageReleaseChecksums(assets) + require.NoError(t, err) + existing := map[string]int64{} + f := &fakeGitHub{digests: map[string]string{}} + for i, file := range files { + raw, err := os.ReadFile(file) + require.NoError(t, err) + existing[filepath.Base(file)] = int64(i + 1) + f.digests[filepath.Base(file)] = fmt.Sprintf("sha256:%x", sha256.Sum256(raw)) + } + switch scenario { + case "late archive conflict": + delete(existing, "a.tar.gz") // would upload before discovering z's conflict + require.NoError(t, os.WriteFile(assets[1].archivePath, []byte("different"), 0600)) + case "checksum conflict": + delete(existing, "a.tar.gz") + f.digests["checksums.txt"] = digestOf("old evidence") + } + client := f.server(t, tag, existing) + err = createAndUploadRelease(t.Context(), client, "codefly-dev", "service-go", tag, assets) + if scenario == "complete identical retry" { + require.NoError(t, err) + } else { + require.ErrorContains(t, err, "cannot be replaced") + } + require.False(t, f.created) + require.Empty(t, f.uploaded) + require.Empty(t, f.deleted) + }) + } +} + +func TestVerifyPublishedArchivesChecksExplicitSBOM(t *testing.T) { + const sbom = "service-go.cdx.json" + for _, scenario := range []string{"present", "missing", "wrong bytes", "wrong producer digest"} { + t.Run(scenario, func(t *testing.T) { + p := publishedBy(t, map[string]string{testArchive: "archive bytes", sbom: "sbom bytes"}) + switch scenario { + case "missing": + delete(p.payloads, sbom) + case "wrong bytes": + p.payloads[sbom] = "different sbom" + case "wrong producer digest": + p.checksumDigests = map[string]string{sbom: fmt.Sprintf("%x", sha256.Sum256([]byte("old sbom")))} + } + err := p.withChecksums().verify([]loaderAsset{{archivePath: testArchive, sbomPath: sbom}}) + if scenario == "present" { + require.NoError(t, err) + } else { + require.ErrorContains(t, err, sbom) + } + }) + } +} diff --git a/cmd/publish/release_upload_test.go b/cmd/publish/release_upload_test.go index af2169d5..dc4fd3bb 100644 --- a/cmd/publish/release_upload_test.go +++ b/cmd/publish/release_upload_test.go @@ -18,9 +18,10 @@ import ( // fakeGitHub records the release-API calls createAndUploadRelease makes so the // create vs. clobber behavior can be asserted without reaching GitHub. type fakeGitHub struct { - uploaded []string // asset names uploaded, in order - deleted []int64 // asset ids deleted (the clobber step) - created bool // whether a release was created + uploaded []string // asset names uploaded, in order + deleted []int64 // asset ids deleted (the clobber step) + created bool // whether a release was created + digests map[string]string // explicit existing producer-evidence digests } // server stands up an httptest server speaking enough of the releases API and @@ -36,12 +37,12 @@ func (f *fakeGitHub) server(t *testing.T, tag string, existingAssets map[string] http.Error(w, "not found", http.StatusNotFound) return } - writeRelease(w, releaseID, existingAssets) + writeRelease(w, releaseID, existingAssets, f.digests) }) mux.HandleFunc("/repos/codefly-dev/service-go/releases", func(w http.ResponseWriter, r *http.Request) { require.Equal(t, http.MethodPost, r.Method) f.created = true - writeRelease(w, releaseID, nil) + writeRelease(w, releaseID, nil, nil) }) mux.HandleFunc(fmt.Sprintf("/repos/codefly-dev/service-go/releases/%d/assets", releaseID), func(w http.ResponseWriter, r *http.Request) { require.Equal(t, http.MethodPost, r.Method) @@ -65,10 +66,14 @@ func (f *fakeGitHub) server(t *testing.T, tag string, existingAssets map[string] return client } -func writeRelease(w http.ResponseWriter, id int64, assets map[string]int64) { +func writeRelease(w http.ResponseWriter, id int64, assets map[string]int64, digests map[string]string) { var parts []string for name, aid := range assets { - parts = append(parts, fmt.Sprintf(`{"id":%d,"name":%q,"digest":"sha256:%x"}`, aid, name, sha256.Sum256([]byte("payload-"+name)))) + digest := fmt.Sprintf("sha256:%x", sha256.Sum256([]byte("payload-"+name))) + if value, ok := digests[name]; ok { + digest = value + } + parts = append(parts, fmt.Sprintf(`{"id":%d,"name":%q,"digest":%q}`, aid, name, digest)) } fmt.Fprintf(w, `{"id":%d,"assets":[%s]}`, id, strings.Join(parts, ",")) } @@ -112,6 +117,7 @@ func TestCreateAndUploadRelease_CreatesWhenAbsent(t *testing.T) { require.ElementsMatch(t, []string{ "service-go_0.0.16_darwin_arm64.tar.gz", "service-go_0.0.16_linux_amd64.tar.gz", + "checksums.txt", }, f.uploaded) require.Empty(t, f.deleted, "nothing to clobber on a fresh release") } @@ -126,7 +132,7 @@ func TestCreateAndUploadRelease_ReusesIdenticalAssetWithoutMutation(t *testing.T require.False(t, f.created, "an existing release must be reused, not recreated") require.Empty(t, f.deleted) - require.Empty(t, f.uploaded) + require.Equal(t, []string{"checksums.txt"}, f.uploaded, "fill missing evidence without replacing the archive") } func TestCreateAndUploadRelease_UploadsSBOMAlongsideArchive(t *testing.T) { @@ -138,7 +144,7 @@ func TestCreateAndUploadRelease_UploadsSBOMAlongsideArchive(t *testing.T) { require.NoError(t, createAndUploadRelease(context.Background(), client, "codefly-dev", "service-go", "v0.0.16", assets)) - require.ElementsMatch(t, []string{archive, sbom}, f.uploaded, + require.ElementsMatch(t, []string{archive, sbom, "checksums.txt"}, f.uploaded, "both the archive and its SBOM must be uploaded") } @@ -153,7 +159,7 @@ func TestCreateAndUploadRelease_ReusesIdenticalSBOMWithoutMutation(t *testing.T) require.False(t, f.created, "an existing release must be reused, not recreated") require.Empty(t, f.deleted) - require.Empty(t, f.uploaded) + require.Equal(t, []string{"checksums.txt"}, f.uploaded, "fill missing evidence without replacing existing bytes") } func TestCreateAndUploadRelease_RejectsChangedBytesWithoutDeletingAsset(t *testing.T) { diff --git a/cmd/publish/workflow_release.go b/cmd/publish/workflow_release.go index 29bea2f7..e7892974 100644 --- a/cmd/publish/workflow_release.go +++ b/cmd/publish/workflow_release.go @@ -5,6 +5,7 @@ import ( "bytes" "context" "crypto/sha256" + "encoding/hex" "errors" "fmt" "io" @@ -122,10 +123,7 @@ func (r *agentReleaser) waitForWorkflowRelease(ctx context.Context, client *gith // failed, so what follows runs on a budget of its own. ctx, cancel := postPublicationContext(ctx) defer cancel() - if err := verifyWorkflowRelease(ctx, client, owner, repo, tag, r.assets); err != nil { - return err - } - return verifyReleaseAssets(ctx, client, r.reg, r.publisher, r.name, strings.TrimPrefix(tag, "v"), r.assets) + return r.verifyPublication(ctx, client, owner, repo, tag) } // releaseVerifyBudget sizes the work that follows a live tag: downloading every @@ -136,22 +134,21 @@ func postPublicationContext(ctx context.Context) (context.Context, context.Cance return context.WithTimeout(context.WithoutCancel(ctx), releaseVerifyBudget) } -// verifyWorkflowRelease confirms the release the owner workflow published -// carries every loader archive, and that each one is the build the workflow -// made. The workflow is the sole publisher of those assets, so every call here -// reads: nothing is created, replaced or deleted. +// verifyPublishedArchives checks both CLI-owned and workflow-owned publication +// against producer checksums, GitHub digests and the downloaded bytes. Every +// call here reads: nothing is created, replaced or deleted. // // The archive is the contract — core's downloader fetches exactly that name. // Whether a workflow also publishes an SBOM beside it, and under what name, is // its own GoReleaser configuration and nothing the manifest declares, so one is -// verified when the release carries it and never demanded. -func verifyWorkflowRelease(ctx context.Context, client *github.Client, owner, repo, tag string, staged []loaderAsset) error { +// verified when present. A CLI-staged SBOM has a known filename and is required. +func verifyPublishedArchives(ctx context.Context, client *github.Client, owner, repo, tag string, staged []loaderAsset) error { release, _, err := client.Repositories.GetReleaseByTag(ctx, owner, repo, tag) if err != nil { - return fmt.Errorf("read workflow-published release: %w", err) + return fmt.Errorf("read published release: %w", err) } if release.GetTagName() != tag || release.GetDraft() { - return fmt.Errorf("workflow did not publish release %s", tag) + return fmt.Errorf("release %s is absent or still draft", tag) } assets := make(map[string]*github.ReleaseAsset) for _, asset := range release.Assets { @@ -165,12 +162,19 @@ func verifyWorkflowRelease(ctx context.Context, client *github.Client, owner, re archiveName := filepath.Base(candidate.archivePath) archive, ok := assets[archiveName] if !ok { - return fmt.Errorf("workflow release %s is missing loader archive %s", tag, archiveName) + return fmt.Errorf("release %s is missing loader archive %s", tag, archiveName) } if err := verifyPublishedRelease(ctx, client, owner, repo, archive, checksums); err != nil { return err } - if sbom, ok := assets[archiveName+".sbom.json"]; ok { + sbomName := archiveName + ".sbom.json" + if candidate.sbomPath != "" { + sbomName = filepath.Base(candidate.sbomPath) + if assets[sbomName] == nil { + return fmt.Errorf("release %s is missing staged SBOM %s", tag, sbomName) + } + } + if sbom, ok := assets[sbomName]; ok { if err := verifyPublishedRelease(ctx, client, owner, repo, sbom, checksums); err != nil { return err } @@ -180,16 +184,16 @@ func verifyWorkflowRelease(ctx context.Context, client *github.Client, owner, re } // verifyPublishedRelease establishes one asset three ways: the digest the -// workflow computed from its own build output, the digest GitHub recorded when +// publisher computed from its own build output, the digest GitHub recorded when // it accepted the upload, and the bytes GitHub actually serves. Comparing the // served bytes to GitHub's own record alone would only show GitHub agreeing // with itself; the checksums file is the independent producer that makes the -// comparison mean something. Core's downloader verifies nothing at install -// time, so this is the only place in the chain the bytes are ever checked. +// comparison mean something. Install-time verification is owned separately +// by Core's downloader; publication does not replace that check. func verifyPublishedRelease(ctx context.Context, client *github.Client, owner, repo string, asset *github.ReleaseAsset, checksums map[string]string) error { built, ok := checksums[asset.GetName()] if !ok { - return fmt.Errorf("%w: %s is absent from the checksums the workflow published", errAssetVerdict, asset.GetName()) + return fmt.Errorf("%w: %s is absent from the published checksums", errAssetVerdict, asset.GetName()) } if recorded := asset.GetDigest(); recorded != "sha256:"+built { return fmt.Errorf("%w: %s was built as sha256:%s but the release records %s", @@ -200,10 +204,8 @@ func verifyPublishedRelease(ctx context.Context, client *github.Client, owner, r }) } -// publishedChecksums reads the digest list the release workflow computed from -// its own build output before uploading anything. It is required: without it -// the release carries no digest source independent of GitHub's own record, and -// nothing downstream ever checks these archives again. +// publishedChecksums reads the producer digest list. It is required: without +// it publication has no digest source independent of GitHub's upload record. func publishedChecksums(ctx context.Context, client *github.Client, owner, repo, tag string, assets map[string]*github.ReleaseAsset) (map[string]string, error) { var found *github.ReleaseAsset for name, asset := range assets { @@ -211,13 +213,16 @@ func publishedChecksums(ctx context.Context, client *github.Client, owner, repo, continue } if found != nil { - return nil, fmt.Errorf("workflow release %s publishes more than one checksums file (%s, %s); which one records the archives is ambiguous", + return nil, fmt.Errorf("release %s publishes more than one checksums file (%s, %s); which one records the archives is ambiguous", tag, found.GetName(), name) } found = asset } if found == nil { - return nil, fmt.Errorf("workflow release %s publishes no checksums file, so its loader archives have no digest source independent of GitHub's own record (GoReleaser emits one unless `checksum: disable: true`)", tag) + return nil, fmt.Errorf("release %s publishes no checksums file, so its loader archives have no digest source independent of GitHub's own record (GoReleaser emits one unless `checksum: disable: true`)", tag) + } + if found.GetSize() > 1<<20 { + return nil, fmt.Errorf("%w: release checksums exceed 1 MiB", errAssetVerdict) } var raw bytes.Buffer if err := retryTransientRead(ctx, func() error { @@ -226,20 +231,36 @@ func publishedChecksums(ctx context.Context, client *github.Client, owner, repo, }); err != nil { return nil, err } - return parseChecksums(raw.Bytes()), nil + return parseChecksums(raw.Bytes()) } -// parseChecksums reads ` ` lines. The leading `*` of sha256sum's -// binary mode is stripped so a workflow that emits it still resolves by name. -func parseChecksums(raw []byte) map[string]string { +// parseChecksums reads SHA-256 records and refuses ambiguous producer evidence. +// sha256sum's binary-mode marker is accepted alongside its text-mode form. +func parseChecksums(raw []byte) (map[string]string, error) { checksums := map[string]string{} scanner := bufio.NewScanner(bytes.NewReader(raw)) for scanner.Scan() { - if fields := strings.Fields(scanner.Text()); len(fields) == 2 { - checksums[strings.TrimPrefix(fields[1], "*")] = fields[0] + fields := strings.Fields(scanner.Text()) + if len(fields) == 0 { + continue + } + if len(fields) != 2 { + return nil, fmt.Errorf("%w: malformed checksum entry", errAssetVerdict) + } + digest, err := hex.DecodeString(fields[0]) + if err != nil || len(digest) != sha256.Size { + return nil, fmt.Errorf("%w: invalid SHA-256 checksum", errAssetVerdict) } + name := strings.TrimPrefix(fields[1], "*") + if _, exists := checksums[name]; exists { + return nil, fmt.Errorf("%w: duplicate checksum for %s", errAssetVerdict, name) + } + checksums[name] = strings.ToLower(fields[0]) + } + if err := scanner.Err(); err != nil { + return nil, fmt.Errorf("%w: read checksums: %v", errAssetVerdict, err) } - return checksums + return checksums, nil } // retryTransientRead re-runs read while it fails for a reason a re-read could diff --git a/cmd/publish/workflow_release_test.go b/cmd/publish/workflow_release_test.go index b81fc4a7..f4c8088e 100644 --- a/cmd/publish/workflow_release_test.go +++ b/cmd/publish/workflow_release_test.go @@ -231,7 +231,7 @@ func (p *publishedRelease) client() *github.Client { } func (p *publishedRelease) verify(staged []loaderAsset) error { - return verifyWorkflowRelease(p.t.Context(), p.client(), "codefly-dev", "service-go", p.tag, staged) + return verifyPublishedArchives(p.t.Context(), p.client(), "codefly-dev", "service-go", p.tag, staged) } func TestVerifyWorkflowReleaseReadsPublishedAssetsWithoutWriting(t *testing.T) { @@ -269,7 +269,7 @@ func TestVerifyWorkflowReleaseReadsPublishedAssetsWithoutWriting(t *testing.T) { r.draft = true return r }, - wantErr: "workflow did not publish release", + wantErr: "absent or still draft", }, { name: "no checksums file", @@ -296,7 +296,7 @@ func TestVerifyWorkflowReleaseReadsPublishedAssetsWithoutWriting(t *testing.T) { r.omitFromChecksums = map[string]bool{testArchive: true} return r.withChecksums() }, - wantErr: testArchive + " is absent from the checksums", + wantErr: testArchive + " is absent from the published checksums", }, { name: "release records a digest the workflow did not build", @@ -400,7 +400,7 @@ func TestPostPublicationVerificationOutlivesTheSpentPublishBudget(t *testing.T) ctx, cancel := postPublicationContext(spent) defer cancel() - err := verifyWorkflowRelease(ctx, client, "codefly-dev", "service-go", "v0.0.16", stageAssets(t, testArchive)) + err := verifyPublishedArchives(ctx, client, "codefly-dev", "service-go", "v0.0.16", stageAssets(t, testArchive)) require.NoError(t, err) require.ErrorIs(t, spent.Err(), context.Canceled, "the publish budget must be spent by the time verification finishes") @@ -410,10 +410,21 @@ func TestPostPublicationVerificationOutlivesTheSpentPublishBudget(t *testing.T) } func TestParseChecksumsReadsBothSha256sumForms(t *testing.T) { - raw := "aaaa text-mode.tar.gz\nbbbb *binary-mode.tar.gz\n\ngarbage\ncccc extra fields\n" + digest := strings.TrimPrefix(digestOf("archive"), "sha256:") + raw := digest + " text-mode.tar.gz\n" + strings.ToUpper(digest) + " *binary-mode.tar.gz\n\n" + parsed, err := parseChecksums([]byte(raw)) + require.NoError(t, err) + require.Equal(t, map[string]string{"text-mode.tar.gz": digest, "binary-mode.tar.gz": digest}, parsed) +} - require.Equal(t, map[string]string{ - "text-mode.tar.gz": "aaaa", - "binary-mode.tar.gz": "bbbb", - }, parseChecksums([]byte(raw))) +func TestParseChecksumsRefusesAmbiguousOrMalformedEvidence(t *testing.T) { + digest := strings.TrimPrefix(digestOf("archive"), "sha256:") + for _, raw := range []string{ + "garbage", "abc archive", strings.Repeat("x", 64) + " archive", + digest + " archive\n" + digest + " archive\n", + digest + " archive extra", strings.Repeat("x", 70<<10), + } { + _, err := parseChecksums([]byte(raw)) + require.ErrorIs(t, err, errAssetVerdict) + } } diff --git a/docs/cli-reference.md b/docs/cli-reference.md index 45bdc9f4..a87a9886 100644 --- a/docs/cli-reference.md +++ b/docs/cli-reference.md @@ -3036,7 +3036,7 @@ Flags: ``` --call-endpoint module/service/endpoint declared HTTP-based endpoint, as module/service/endpoint, every prepared call is sent to instead of the owner's listener, overriding the environment's runnable-call-endpoint (empty: each owner's listener; the owner's operation, route, contract and policy are kept) - --check do not write; exit 1 if the on-disk file differs from what would be generated + --check leave binding output unchanged; materialize selected pins, then exit 1 if the output is stale --env string environment supplying resolved addresses and runnable-scope-selections (default "local") --naming-scope scope scope addresses resolve in, as codefly run --naming-scope reads it: omitted keeps the environment's, empty clears it ``` diff --git a/docs/commands.md b/docs/commands.md index 42dd9b0c..e3cf95c1 100644 --- a/docs/commands.md +++ b/docs/commands.md @@ -2931,6 +2931,17 @@ publishing host, so on a linux/amd64 runner that qualification is the linux/amd64 build only. An agent with `release.owner: cli` uploads the archives itself and still requires a host that builds every platform. +Both publication owners verify the published archive bytes before reporting +success. CLI-owned uploads also stage deterministic checksums and refuse changed +existing assets before uploading anything. The final reachability check uses +installer credentials, independently of the publishing login. See +[publication verification](publish-verification.md). + +Module releases also carry their tracked generated client versions through the +existing release transaction; stale clients must be regenerated first. See +[generated client versions](publish-client-versions.md) for single-source and +aggregate behavior. + ### `codefly publish all [patch|minor|major]` Discover every git repository under the workspace that carries a codefly @@ -3902,7 +3913,7 @@ absent or empty exactly when the environment selects no binding. | Flag | Description | |------|-------------| | `--env` | Environment supplying resolved addresses and `runnable-scope-selections` (default: `local`) | -| `--check` | Regenerate in memory; exit non-zero if the output is stale, including after a selection or a decline changes; write nothing | +| `--check` | Regenerate in memory; exit non-zero if the output is stale, including after a selection or a decline changes. Leaves binding output unchanged; first materializes selected pins through the common loader, which may update the cache, local overlay and resolution receipts. | ## Infrastructure @@ -4210,7 +4221,11 @@ What it checks, in order: operations that cannot be read at all fail as `runnable_operations_unreadable`. Like the checks below, it needs every composed module materialized. -6. Every workspace configuration group services declare under +6. Services withdrawn by the selected environment's `exclude-services` impose + no configuration requirements, including in a module-scoped check. The same + exclusion validation used by run still refuses undeclared consumer withdrawals; + explicitly selecting an excluded service fails. + Every workspace configuration group remaining services declare under `workspace-configuration-dependencies` is provided and defines values. The doctor reads exactly what a run provisions: the workspace's own `configurations//*` composed with the groups each composed module @@ -4225,7 +4240,13 @@ What it checks, in order: (nothing derived, or every binding declined) the group absent or holding no value is ok; a selected binding the group does not hold fails as `configuration_missing` with `codefly generate runnable-bindings --env - ` as its remedy. + ` as its remedy. Existing binding values are also compared with the + generator's current preparation: changed addresses, contracts or policies, + obsolete extra bindings and output left after the last operation disappears + fail as `configuration_invalid`. This comparison checks effective values; + the generator's `--check` additionally checks exact file bytes. The delivery + readiness gate uses this check before build or push for modules consuming + the group; the explicit `--skip-workspace-readiness` override still applies. 7. Per-service `configurations/` files parse; duplicates are flagged. 8. Every `${endpoint://}` reference in a workspace configuration a service in scope declares names a service of the workspace diff --git a/docs/publish-client-versions.md b/docs/publish-client-versions.md new file mode 100644 index 00000000..fc934fe0 --- /dev/null +++ b/docs/publish-client-versions.md @@ -0,0 +1,25 @@ +# Generated client versions in a module release + +`codefly publish` includes committed generated clients whose `sources` name a +package released by the module workspace. It finds tracked +`library.codefly.yaml` files throughout the repository, including `clients/`; +ignored caches and dependencies are not release inputs. + +Before changing versions, publication checks that each selected source names the +owner's current package version and contract digest. It verifies copied bytes +and catalog metadata against the committed producer, including protobuf service +subsets through the same narrowing code used by generation. Stale clients must +be regenerated and committed +before releasing. Every selected artifact must be tracked and remain inside the +repository; symlinks are refused. + +The release commit updates the selected source versions and copied catalogs. A +single-source client also receives the release version in its library manifest +and TypeScript/Python package manifests. An aggregate keeps its own library and +language package versions; unrelated source entries stay unchanged. Contract +bytes and generation provenance are preserved. Go clients obtain their version +from the release tag. + +These changes use the existing release transaction. If regeneration or a +release gate fails before commit, all client version changes are restored with +the module manifests and generated contracts. diff --git a/docs/publish-verification.md b/docs/publish-verification.md new file mode 100644 index 00000000..6df7b804 --- /dev/null +++ b/docs/publish-verification.md @@ -0,0 +1,30 @@ +# Publication integrity and installer access + +For executable agent releases, `codefly publish` verifies the archives after +publication whether the manifest assigns publication to `cli` or `workflow`. +Each required loader archive must appear in the release's producer checksum +list. Its SHA-256 must agree with GitHub's upload digest, and the downloaded +bytes must match the recorded size and digest. A missing archive, draft release, +missing checksum evidence, or mismatched bytes makes publication verification +fail. A published tag is not rolled back when verification fails. + +When the CLI owns the uploads, it stages a deterministic `checksums.txt` +covering every archive and SBOM. Before uploading any file, it compares every +existing asset with the staged bytes. A retry can fill missing assets or reuse +identical assets, but cannot replace an existing archive, SBOM or checksum list. +It uploads the checksum list last, after the files it describes. A CLI-staged +SBOM is required and verified by its declared filename. Workflow-owned SBOMs +named `.sbom.json` are verified when present; a workflow need not publish +one. + +After integrity verification, publication checks that each exact loader URL is +reachable using Core's installer credential rule: `GITHUB_TOKEN`, otherwise +`GH_TOKEN`, otherwise anonymous access. Publishing may use a `gh auth login` +credential; installer verification deliberately does not. A public release can +pass without an environment token. A private release needs a credential that +the installer can actually use. + +These checks establish published archive integrity and installer access. They +do not start the agent or establish its runtime capabilities. Core owns download +verification and installed-cache admission; the release's agent CI owns runtime +qualification. diff --git a/go.mod b/go.mod index 513ccffe..1ed43ef7 100644 --- a/go.mod +++ b/go.mod @@ -16,7 +16,7 @@ require ( github.com/codefly-dev/authority/libraries/delivery/go v0.0.0-20261026023425-000d44495d93 github.com/codefly-dev/authority/libraries/fixture-composition/go v0.0.0-20261027171336-5178f01696e3 github.com/codefly-dev/authority/libraries/hop/go v0.0.0-20261026175541-3afad7224855 - github.com/codefly-dev/core v0.19.7-0.20261026194550-a8599bd3991a + github.com/codefly-dev/core v0.19.8-0.20261027203111-6eadd92dd156 github.com/codefly-dev/golor v0.1.3 github.com/codefly-dev/llm v0.1.7 github.com/codefly-dev/sdk-go/workcontext v0.1.1-0.20261018052022-70dc7a546458 @@ -175,7 +175,7 @@ require ( github.com/mitchellh/copystructure v1.2.0 // indirect github.com/mitchellh/mapstructure v1.5.0 // indirect github.com/mitchellh/reflectwalk v1.0.2 // indirect - github.com/moby/buildkit v0.32.2 // indirect + github.com/moby/buildkit v0.34.1 // indirect github.com/moby/docker-image-spec v1.3.1 // indirect github.com/monochromegane/go-gitignore v0.0.0-20200626010858-205db1a8cc00 // indirect github.com/muesli/ansi v0.0.0-20230316100256-276c6243b2f6 // indirect @@ -195,7 +195,7 @@ require ( github.com/sergi/go-diff v1.4.0 // indirect github.com/shoenig/go-m1cpu v0.2.2 // indirect github.com/shopspring/decimal v1.4.0 // indirect - github.com/sirupsen/logrus v1.10.0 // indirect + github.com/sirupsen/logrus v1.10.2 // indirect github.com/skeema/knownhosts v1.3.2 // indirect github.com/spf13/cast v1.10.0 // indirect github.com/standard-webhooks/standard-webhooks/libraries v0.0.1 // indirect @@ -243,7 +243,7 @@ require ( golang.org/x/exp v0.0.0-20260908205506-85c1c2202aba // indirect golang.org/x/net v0.60.0 // indirect golang.org/x/text v0.42.0 // indirect - golang.org/x/time v0.15.0 // indirect + golang.org/x/time v0.16.0 // indirect google.golang.org/genproto/googleapis/rpc v0.0.0-20261012220534-4341edf3f941 // indirect gopkg.in/evanphx/json-patch.v4 v4.13.0 // indirect gopkg.in/warnings.v0 v0.1.2 // indirect diff --git a/go.sum b/go.sum index 47505b10..b2dd79eb 100644 --- a/go.sum +++ b/go.sum @@ -117,6 +117,8 @@ github.com/codefly-dev/authority/libraries/resources/go v0.0.0-20261026023425-00 github.com/codefly-dev/authority/libraries/resources/go v0.0.0-20261026023425-000d44495d93/go.mod h1:1rmjrVyr0h7zOuwOHS9i+BoeS9dr3XNvtupLZFwY6As= github.com/codefly-dev/core v0.19.7-0.20261026194550-a8599bd3991a h1:ar8ERpN9n9ZL29LZ63mqBZNLPZ7OpcnQ1+D1g89DbPY= github.com/codefly-dev/core v0.19.7-0.20261026194550-a8599bd3991a/go.mod h1:1ina3kP+YSAL4tlZvnEYd4P8bFGnJECfkP86ojeeF1A= +github.com/codefly-dev/core v0.19.8-0.20261027203111-6eadd92dd156 h1:OLQ/LG4+KGRz3crTc8zgQlxTS/Ta3Yh7OUFu+12N5u4= +github.com/codefly-dev/core v0.19.8-0.20261027203111-6eadd92dd156/go.mod h1:NB7x1UZ01rYTfEZEl4vUFD1A+fgOXhsWbgA3M8x4ThA= github.com/codefly-dev/golor v0.1.3 h1:xmo+ceyJFRYZdvpWE2fNd0jeaadp/Ibm1BnganiGKOc= github.com/codefly-dev/golor v0.1.3/go.mod h1:sl/u/K1l7J0Pr3xyVZp8fOJYQItKKst1No9JqgzLLoY= github.com/codefly-dev/gortk v0.2.0 h1:7bOlS5valYz2zil+fZctQNcPCYBcPj86abcw9N8h1hQ= @@ -133,6 +135,7 @@ github.com/containerd/errdefs/pkg v0.3.0 h1:9IKJ06FvyNlexW690DXuQNx2KA2cUJXx151X github.com/containerd/errdefs/pkg v0.3.0/go.mod h1:NJw6s9HwNuRhnjJhM7pylWwMyAkmCQvQ4GpJHEqRLVk= github.com/containerd/log v0.1.0 h1:TCJt7ioM2cr/tfR8GPbGf9/VRAX8D2B4PjzCpfX540I= github.com/containerd/log v0.1.0/go.mod h1:VRRf09a7mHDIRezVKTRCrOq78v577GXq3bSa3EhrzVo= +github.com/containerd/log v0.2.0 h1:BewD/umNgVnoczglOpX8eRMyEy5t5iPlu5AIpnWDONc= github.com/containerd/typeurl/v2 v2.3.0 h1:HZHPhRWo5XMy3QGQoPrUzbW/2ckwjfweHmOwlkIrPAQ= github.com/containerd/typeurl/v2 v2.3.0/go.mod h1:Qk+PAdUYArVj41TnGi6rJ+48RF0PkcTc4i/taoBcK0w= github.com/cpuguy83/go-md2man/v2 v2.0.6/go.mod h1:oOW0eioCTA6cOiMLiUPZOpcVxMig6NIQQ7OS05n1F4g= @@ -332,6 +335,8 @@ github.com/mitchellh/reflectwalk v1.0.2 h1:G2LzWKi524PWgd3mLHV8Y5k7s6XUvT0Gef6zx github.com/mitchellh/reflectwalk v1.0.2/go.mod h1:mSTlrgnPZtwu0c4WaC2kGObEpuNDbx0jmZXqmk4esnw= github.com/moby/buildkit v0.32.2 h1:Sfy7+u6dUv/2yuBc9KCoK70Re8atuV8aPZ5UOC068Vc= github.com/moby/buildkit v0.32.2/go.mod h1:0GB/EJ1d+4VIVqIAgy3asaoGkVXy7IrDfVy7mPhOvg8= +github.com/moby/buildkit v0.34.1 h1:jVdxsXX0UpU0q1Et+9lFezPmh9sGE6vF0TrKZaGlfy4= +github.com/moby/buildkit v0.34.1/go.mod h1:e+Za70y2a3nL1U2Y0P8TvhPyKp2Ok2NS+Q4siSArDiM= github.com/moby/docker-image-spec v1.3.1 h1:jMKff3w6PgbfSa69GfNg+zN/XLhfXJGnEx3Nl2EsFP0= github.com/moby/docker-image-spec v1.3.1/go.mod h1:eKmb5VW8vQEh/BAr2yvVNvuiJuY6UIocYsFu/DxxRpo= github.com/moby/patternmatcher v0.6.1 h1:qlhtafmr6kgMIJjKJMDmMWq7WLkKIo23hsrpR3x084U= @@ -406,6 +411,8 @@ github.com/shopspring/decimal v1.4.0/go.mod h1:gawqmDU56v4yIKSwfBSFip1HdCCXN8/+D github.com/sirupsen/logrus v1.7.0/go.mod h1:yWOB1SBYBC5VeMP7gHvWumXLIWorT60ONWic61uBYv0= github.com/sirupsen/logrus v1.10.0 h1:T8MxJJXVZkfcC5zSRMRAg2F8+lxjmUCGGWPzFxO+Msc= github.com/sirupsen/logrus v1.10.0/go.mod h1:FXZFonkDAnFozmO+5hGAFvB0Yg9/j2SIhA/QuIkP180= +github.com/sirupsen/logrus v1.10.2 h1:G2SED73/qrAu6YwbdxOD6peLkCBI3z7L+ykJFTXJBBo= +github.com/sirupsen/logrus v1.10.2/go.mod h1:SLEg8TqYulVKKfIGHldVp2K2aYz2DKSVBq4g/H5bR7Q= github.com/skeema/knownhosts v1.3.2 h1:EDL9mgf4NzwMXCTfaxSD/o/a5fxDw/xL9nkU28JjdBg= github.com/skeema/knownhosts v1.3.2/go.mod h1:bEg3iQAuw+jyiw+484wwFJoKSLwcfd7fqRy+N0QTiow= github.com/spf13/cast v1.10.0 h1:h2x0u2shc1QuLHfxi+cTJvs30+ZAHOGRic8uyGTDWxY= @@ -514,6 +521,7 @@ go.opentelemetry.io/otel/exporters/otlp/otlptrace/otlptracegrpc v1.47.0 h1:UFpxO go.opentelemetry.io/otel/exporters/otlp/otlptrace/otlptracegrpc v1.47.0/go.mod h1:YBGjxe3lt0jtXoEXxGrGhv/jM7oUBUDVK7zhAvNyEjc= go.opentelemetry.io/otel/exporters/otlp/otlptrace/otlptracehttp v1.45.0 h1:QBajQ2SrwQijzHyZbQlPsuIzpl/ll8DY6wPWsajeGcI= go.opentelemetry.io/otel/exporters/otlp/otlptrace/otlptracehttp v1.45.0/go.mod h1:08ZQLjrPLQ6R4kAXvuOvODEer5Yh4CoFvll5qB2BCI8= +go.opentelemetry.io/otel/exporters/otlp/otlptrace/otlptracehttp v1.46.0 h1:KrC1YrQeSt46ITMWAbgQx1M1eV1/1TKzttrBzymPmss= go.opentelemetry.io/otel/exporters/stdout/stdouttrace v1.47.0 h1:N3YQCxjxQ/bMjyc3heladfRm9t9RTksGQH8z4w6yU/0= go.opentelemetry.io/otel/exporters/stdout/stdouttrace v1.47.0/go.mod h1:Mp8HOFqcaUyypCuGv9IhDdTHnJ56lSudSHMd+pVSCEA= go.opentelemetry.io/otel/log v1.47.0 h1:cOTS1CcLbSQeZKanGJ+0JpF/+t4PELi3O3bbl2lqCcI= @@ -581,6 +589,8 @@ golang.org/x/text v0.42.0 h1:JbOZXgfeCPU9gacVtYliJqOhD+zhrEqK4LfdpmlUZqI= golang.org/x/text v0.42.0/go.mod h1:ojzP1Z+2QtioaF8DTtO8K5q7JWVVYwZKenzujK0Zd0E= golang.org/x/time v0.15.0 h1:bbrp8t3bGUeFOx08pvsMYRTCVSMk89u4tKbNOZbp88U= golang.org/x/time v0.15.0/go.mod h1:Y4YMaQmXwGQZoFaVFk4YpCt4FLQMYKZe9oeV/f4MSno= +golang.org/x/time v0.16.0 h1:vMb6ptszcQMkcwiRTAuNNU50gom6++Q/6gY2hDM6VDE= +golang.org/x/time v0.16.0/go.mod h1:rVKOqvZeKvrDKTQiAHJ7wmwP0RzleSphoEA9RcdLA0s= golang.org/x/tools v0.0.0-20180917221912-90fa682c2a6e/go.mod h1:n7NCudcB/nEzxVGmLbDWY5pfWTLqBcC2KZ6jyYvM4mQ= golang.org/x/tools v0.51.0 h1:k4Xc/1Om9jwkBJBo4NVLMSARBoWtK10mx+W5BnXCeAI= golang.org/x/tools v0.51.0/go.mod h1:9eEncMayCV6zRMGhR5eZEC2iBx98qWcF1HZ9Z7wJOoA= diff --git a/pkg/generators/client_release.go b/pkg/generators/client_release.go new file mode 100644 index 00000000..4c4c0e61 --- /dev/null +++ b/pkg/generators/client_release.go @@ -0,0 +1,292 @@ +package generators + +import ( + "bytes" + "context" + "encoding/json" + "errors" + "fmt" + "io" + "os" + "path/filepath" + "slices" + "strings" + + "github.com/codefly-dev/core/composition" + "github.com/codefly-dev/core/languages" + "github.com/codefly-dev/core/resources" + "github.com/codefly-dev/core/shared" + "gopkg.in/yaml.v3" +) + +// ClientRelease is the version-only part of an existing generated library. +// Paths lets the release owner restore every write if its release gate fails. +// Contract bytes, generator provenance and unrelated sources stay put. +type ClientRelease struct { + Paths []string + dir string + document yaml.Node + sources []librarySource + catalogs map[int]*composition.APIContractCatalog + catalogPaths map[int]string + langs []generatedLanguageExport +} + +// ClientReleasePackage is the committed producer whose contract a client binds. +type ClientReleasePackage struct { + Directory string + Catalog *composition.APIContractCatalog +} + +// PlanClientRelease selects sources owned by the releasing packages. The owner +// supplies its committed catalogs and checks every path before it is read, using +// the same containment/tracking rules as the rest of its release. No file is +// written during planning; a stale source must be regenerated before release. +func PlanClientRelease(dir string, packages map[string]ClientReleasePackage, checkPath func(string) error) (*ClientRelease, error) { + manifest := filepath.Join(dir, resources.LibraryConfigurationName) + if err := checkPath(manifest); err != nil { + return nil, err + } + sources, err := readExistingLibrarySources(dir) + if err != nil { + return nil, err + } + matched := false + for i := range sources { + _, ok := packages[sources[i].Package] + matched = matched || ok + } + if !matched { + return nil, nil + } + plan := &ClientRelease{dir: dir, sources: sources, catalogs: map[int]*composition.APIContractCatalog{}, catalogPaths: map[int]string{}} + raw, err := os.ReadFile(manifest) //nolint:gosec // checked release path + if err != nil { + return nil, err + } + if err = yaml.Unmarshal(raw, &plan.document); err != nil { + return nil, err + } + plan.Paths = append(plan.Paths, manifest) + for i := range sources { + if producer, releasing := packages[sources[i].Package]; releasing { + if err = plan.addSource(i, producer, checkPath); err != nil { + return nil, err + } + } + } + if len(sources) == 1 { + plan.langs, err = readExistingLanguageExports(dir) + if err != nil { + return nil, err + } + for _, lang := range plan.langs { + if !filepath.IsLocal(lang.Path) { + return nil, fmt.Errorf("client language path %q escapes its library", lang.Path) + } + if path, ok := packageManifest(languages.Language(lang.Name), filepath.Join(dir, lang.Path)); ok { + if err := checkPath(path); err != nil { + return nil, err + } + if err := checkPackageVersionStampable(languages.Language(lang.Name), filepath.Dir(path)); err != nil { + return nil, err + } + plan.Paths = append(plan.Paths, path) + } + } + } + return plan, nil +} + +func (plan *ClientRelease) addSource(i int, producer ClientReleasePackage, checkPath func(string) error) error { + source := &plan.sources[i] + owner := producer.Catalog + manifest := filepath.Join(plan.dir, resources.LibraryConfigurationName) + if owner == nil || owner.Package != source.Package || owner.Version != source.Version || !sourceMatchesCatalog(source, owner) { + return fmt.Errorf("%s source %s/%s is stale; regenerate the client before release", manifest, source.Service, source.Endpoint) + } + contractDir := filepath.Join(plan.dir, "contract") + if len(plan.sources) > 1 { + entry := ContractEntry{ModuleName: source.Module, Endpoint: composition.APIContractEndpoint{Service: source.Service, Endpoint: source.Endpoint}} + name := contractEntryDirName(&entry) + if !filepath.IsLocal(name) || strings.ContainsAny(name, `/\`) { + return fmt.Errorf("invalid generated contract directory %q", name) + } + contractDir = filepath.Join(contractDir, name) + } + path := filepath.Join(contractDir, "catalog.codefly.json") + if err := checkPath(path); err != nil { + return err + } + catalog, err := readClientReleaseCatalog(path) + if err != nil { + return err + } + if catalog.Package != source.Package || catalog.Version != source.Version || len(catalog.Endpoints) != 1 || catalog.Endpoints[0].Service != source.Service || catalog.Endpoints[0].Endpoint != source.Endpoint { + return fmt.Errorf("%s disagrees with its library source; regenerate the client before release", path) + } + endpoint := &catalog.Endpoints[0] + if err := checkPath(filepath.Join(contractDir, filepath.FromSlash(endpoint.Path))); err != nil { + return err + } + if err := verifyReleaseClientContract(contractDir, source, producer, endpoint, checkPath); err != nil { + return err + } + plan.catalogs[i], plan.catalogPaths[i] = catalog, path + plan.Paths = append(plan.Paths, path) + return nil +} + +func sourceMatchesCatalog(source *librarySource, catalog *composition.APIContractCatalog) bool { + for i := range catalog.Endpoints { + endpoint := &catalog.Endpoints[i] + if endpoint.Service == source.Service && endpoint.Endpoint == source.Endpoint { + return endpoint.Digest == source.ContractDigest + } + } + return false +} + +// A copied protobuf contract can be a generator-narrowed subset: its digest +// then differs from the source digest. Reproduce that existing transformation +// from the committed producer rather than trusting a self-consistent copy. +func verifyReleaseClientContract(dir string, source *librarySource, producer ClientReleasePackage, copied *composition.APIContractEndpoint, checkPath func(string) error) error { + if !slices.Equal(source.Services, protobufServiceNames(copied.Services)) { + return fmt.Errorf("copied client service selection disagrees with its source; regenerate the client before release") + } + var endpoint composition.APIContractEndpoint + for i := range producer.Catalog.Endpoints { + candidate := &producer.Catalog.Endpoints[i] + if candidate.Service == source.Service && candidate.Endpoint == source.Endpoint { + endpoint = *candidate + break + } + } + if err := checkPath(filepath.Join(producer.Directory, filepath.FromSlash(endpoint.Path))); err != nil { + return err + } + original, err := ReadCatalogContractBytes(producer.Directory, &endpoint) + if err != nil { + return err + } + actual, err := ReadCatalogContractBytes(dir, copied) + if err != nil { + return err + } + if !bytes.Equal(actual, original) { + if len(source.Services) == 0 || endpoint.Kind != composition.APIContractKindProtobuf { + return fmt.Errorf("copied client contract differs from its producer; regenerate the client before release") + } + entries, narrowErr := narrowEntries([]ContractEntry{{Endpoint: endpoint, ContractBytes: original, Services: source.Services}}) + if narrowErr != nil { + return narrowErr + } + if !bytes.Equal(actual, entries[0].ContractBytes) { + return fmt.Errorf("copied client contract differs from its selected producer services; regenerate the client before release") + } + endpoint = entries[0].Endpoint + } + endpoint.Path = "contract.binpb" + if endpoint.Kind == composition.APIContractKindOpenAPI { + endpoint.Path = "openapi.json" + } + endpoint.Digest = composition.APIContractDigest(actual) + expected := &composition.APIContractCatalog{Schema: composition.APIContractCatalogSchema, Package: source.Package, Version: source.Version, Endpoints: []composition.APIContractEndpoint{endpoint}} + got := *expected + got.Endpoints = []composition.APIContractEndpoint{*copied} + expectedBytes, err := expected.CanonicalBytes() + if err != nil { + return err + } + actualBytes, err := got.CanonicalBytes() + if err != nil { + return err + } + if !bytes.Equal(expectedBytes, actualBytes) { + return fmt.Errorf("copied client catalog differs from its producer; regenerate the client before release") + } + return nil +} + +func readClientReleaseCatalog(path string) (*composition.APIContractCatalog, error) { + data, err := os.ReadFile(path) //nolint:gosec // checked release path + if err != nil { + return nil, err + } + catalog := &composition.APIContractCatalog{} + decoder := json.NewDecoder(bytes.NewReader(data)) + decoder.DisallowUnknownFields() + if err := decoder.Decode(catalog); err != nil { + return nil, err + } + var extra any + if err := decoder.Decode(&extra); !errors.Is(err, io.EOF) { + return nil, fmt.Errorf("%s has trailing JSON", path) + } + return catalog, catalog.Validate() +} + +// Write applies versions supplied by the updated owner package manifests. +// An aggregate keeps its own top-level and language versions, exactly as generate. +func (plan *ClientRelease) Write(ctx context.Context, versions map[string]string) error { + root := plan.document.Content[0] + sources := libraryField(root, "sources") + if sources == nil || len(sources.Content) != len(plan.sources) { + return fmt.Errorf("invalid library sources") + } + for i := range plan.sources { + catalog, releasing := plan.catalogs[i] + if !releasing { + continue + } + version := versions[plan.sources[i].Package] + if version == "" { + return fmt.Errorf("no released version for %s", plan.sources[i].Package) + } + catalog.Version = version + if err := catalog.Validate(); err != nil { + return err + } + data, err := catalog.CanonicalBytes() + if err != nil { + return err + } + if err := shared.WriteFileAtomic(ctx, plan.catalogPaths[i], data, 0o644); err != nil { + return err + } + field := libraryField(sources.Content[i], "version") + if field == nil { + return fmt.Errorf("library source has no version") + } + field.Value = version + if len(plan.sources) == 1 { + field := libraryField(root, "version") + if field == nil { + return fmt.Errorf("library has no version") + } + field.Value = version + for _, lang := range plan.langs { + if err := stampPackageVersion(languages.Language(lang.Name), filepath.Join(plan.dir, lang.Path), version); err != nil { + return err + } + } + } + } + data, err := yaml.Marshal(&plan.document) + if err != nil { + return err + } + return shared.WriteFileAtomic(ctx, filepath.Join(plan.dir, resources.LibraryConfigurationName), data, 0o644) +} + +func libraryField(mapping *yaml.Node, key string) *yaml.Node { + if mapping.Kind != yaml.MappingNode { + return nil + } + for i := 0; i+1 < len(mapping.Content); i += 2 { + if mapping.Content[i].Value == key { + return mapping.Content[i+1] + } + } + return nil +} diff --git a/pkg/generators/client_release_test.go b/pkg/generators/client_release_test.go new file mode 100644 index 00000000..ffdfd07d --- /dev/null +++ b/pkg/generators/client_release_test.go @@ -0,0 +1,209 @@ +package generators + +import ( + "os" + "path/filepath" + "testing" + + "github.com/codefly-dev/core/composition" + "github.com/codefly-dev/core/languages" + "github.com/codefly-dev/core/resources" + "github.com/stretchr/testify/require" + "gopkg.in/yaml.v3" +) + +func releaseClientEntry(module, version string) ContractEntry { + data := []byte(`{"openapi":"3.0.0","paths":{"/ready":{"get":{}}}}`) + return ContractEntry{ + PackageID: "acme/" + module, PackageVersion: version, ModuleName: module, ContractBytes: data, + Endpoint: composition.APIContractEndpoint{ + Service: "gateway", Endpoint: "rest", API: "rest", Kind: composition.APIContractKindOpenAPI, + Package: module, Path: "contracts/api/gateway/rest/openapi.json", Digest: composition.APIContractDigest(data), + Routes: []composition.APIContractRoute{{Method: "GET", Path: "/ready"}}, + }, + } +} + +func releaseClientFixture(t *testing.T, entries []ContractEntry) (string, []generatedLanguageExport) { + t.Helper() + dir := t.TempDir() + langs := []generatedLanguageExport{{Name: "typescript", Path: "typescript/"}, {Name: "python", Path: "python/"}, {Name: "go", Path: "go/"}} + require.NoError(t, writeContractFiles(t.Context(), dir, entries)) + for _, entry := range entries { + path := filepath.Join(dir, "producer", entry.Endpoint.Path) + require.NoError(t, os.MkdirAll(filepath.Dir(path), 0o755)) + require.NoError(t, os.WriteFile(path, entry.ContractBytes, 0o600)) + } + for _, lang := range langs { + require.NoError(t, os.MkdirAll(filepath.Join(dir, lang.Path), 0o755)) + } + require.NoError(t, writeTypeScriptScaffold(filepath.Join(dir, "typescript"), "@acme/client")) + require.NoError(t, writePythonScaffold(filepath.Join(dir, "python"), "acme_client")) + require.NoError(t, writeLibraryManifest(t.Context(), dir, "client", "0.1.0", entries, langs, false)) + return dir, langs +} + +func TestClientReleaseMatchesGeneratorVersionOutputs(t *testing.T) { + entry := releaseClientEntry("payments", "0.1.0") + dir, langs := releaseClientFixture(t, []ContractEntry{entry}) + owner := &composition.APIContractCatalog{Package: entry.PackageID, Version: entry.PackageVersion, Endpoints: []composition.APIContractEndpoint{entry.Endpoint}} + plan, err := PlanClientRelease(dir, map[string]ClientReleasePackage{entry.PackageID: {Directory: filepath.Join(dir, "producer"), Catalog: owner}}, func(string) error { return nil }) + require.NoError(t, err) + require.Len(t, plan.Paths, 4) + require.NoError(t, plan.Write(t.Context(), map[string]string{entry.PackageID: "0.2.0"})) + after := snapshotTree(t, dir) + + // Re-run the same version-writing paths as GenerateClientLibrary: nothing + // differs from the release output, including its copied contract catalog. + entry.PackageVersion = "0.2.0" + require.NoError(t, writeContractFiles(t.Context(), dir, []ContractEntry{entry})) + require.NoError(t, writeLibraryManifest(t.Context(), dir, "client", libraryVersion(dir, []ContractEntry{entry}), []ContractEntry{entry}, langs, false)) + for _, lang := range langs { + require.NoError(t, stampPackageVersion(languages.Language(lang.Name), filepath.Join(dir, lang.Path), "0.2.0")) + } + require.Equal(t, after, snapshotTree(t, dir)) +} + +func TestClientReleaseAggregateKeepsForeignSourceAndOwnVersion(t *testing.T) { + entries := []ContractEntry{releaseClientEntry("payments", "0.1.0"), releaseClientEntry("ledger", "3.4.0")} + dir, _ := releaseClientFixture(t, entries) + foreignPath := filepath.Join(dir, "contract", "ledger-gateway-rest", "catalog.codefly.json") + foreign, err := os.ReadFile(foreignPath) + require.NoError(t, err) + ts, err := os.ReadFile(filepath.Join(dir, "typescript", "package.json")) + require.NoError(t, err) + manifestPath := filepath.Join(dir, resources.LibraryConfigurationName) + manifest, err := os.ReadFile(manifestPath) + require.NoError(t, err) + require.NoError(t, os.WriteFile(manifestPath, append(manifest, []byte("custom-owner: keep\n")...), 0o600)) + + owner := &composition.APIContractCatalog{Package: "acme/payments", Version: "0.1.0", Endpoints: []composition.APIContractEndpoint{entries[0].Endpoint}} + plan, err := PlanClientRelease(dir, map[string]ClientReleasePackage{"acme/payments": {Directory: filepath.Join(dir, "producer"), Catalog: owner}}, func(string) error { return nil }) + require.NoError(t, err) + require.Len(t, plan.Paths, 2) + require.NoError(t, plan.Write(t.Context(), map[string]string{"acme/payments": "0.2.0"})) + got, err := os.ReadFile(manifestPath) + require.NoError(t, err) + var doc map[string]any + require.NoError(t, yaml.Unmarshal(got, &doc)) + require.Equal(t, "0.1.0", doc["version"]) + require.Equal(t, "keep", doc["custom-owner"]) + var original map[string]any + require.NoError(t, yaml.Unmarshal(manifest, &original)) + require.Equal(t, original["generated-by"], doc["generated-by"]) + sources, err := readExistingLibrarySources(dir) + require.NoError(t, err) + require.Equal(t, "0.2.0", sources[0].Version) + require.Equal(t, "3.4.0", sources[1].Version) + got, err = os.ReadFile(foreignPath) + require.NoError(t, err) + require.Equal(t, foreign, got) + got, err = os.ReadFile(filepath.Join(dir, "typescript", "package.json")) + require.NoError(t, err) + require.Equal(t, ts, got) +} + +func TestClientReleaseRefusesStaleArtifactsBeforeWriting(t *testing.T) { + for _, scenario := range []struct { + name string + mutate func(*testing.T, string, *composition.APIContractCatalog) + want string + }{ + {"producer changed", func(_ *testing.T, _ string, owner *composition.APIContractCatalog) { + owner.Endpoints[0].Digest = composition.APIContractDigest([]byte("changed")) + }, "is stale"}, + {"producer version changed", func(_ *testing.T, _ string, owner *composition.APIContractCatalog) { owner.Version = "0.2.0" }, "is stale"}, + {"copied contract changed", func(t *testing.T, dir string, _ *composition.APIContractCatalog) { + require.NoError(t, os.WriteFile(filepath.Join(dir, "contract", "openapi.json"), []byte("changed"), 0o600)) + }, "does not match catalog digest"}, + {"unknown catalog field", func(t *testing.T, dir string, _ *composition.APIContractCatalog) { + path := filepath.Join(dir, "contract", "catalog.codefly.json") + data, err := os.ReadFile(path) + require.NoError(t, err) + require.NoError(t, os.WriteFile(path, append([]byte(`{"unknown":true,`), data[1:]...), 0o600)) + }, "unknown field"}, + {"language version absent", func(t *testing.T, dir string, _ *composition.APIContractCatalog) { + require.NoError(t, os.WriteFile(filepath.Join(dir, "typescript", "package.json"), []byte(`{"name":"client"}`), 0o600)) + }, "cannot find a version"}, + } { + t.Run(scenario.name, func(t *testing.T) { + entry := releaseClientEntry("payments", "0.1.0") + dir, _ := releaseClientFixture(t, []ContractEntry{entry}) + owner := &composition.APIContractCatalog{Package: entry.PackageID, Version: entry.PackageVersion, Endpoints: []composition.APIContractEndpoint{entry.Endpoint}} + scenario.mutate(t, dir, owner) + before := snapshotTree(t, dir) + _, err := PlanClientRelease(dir, map[string]ClientReleasePackage{entry.PackageID: {Directory: filepath.Join(dir, "producer"), Catalog: owner}}, func(string) error { return nil }) + require.ErrorContains(t, err, scenario.want) + require.Equal(t, before, snapshotTree(t, dir)) + }) + } +} + +func TestClientReleaseDoesNotReadForeignCatalogs(t *testing.T) { + dir, _ := releaseClientFixture(t, []ContractEntry{releaseClientEntry("ledger", "1.0.0")}) + require.NoError(t, os.RemoveAll(filepath.Join(dir, "contract"))) + plan, err := PlanClientRelease(dir, map[string]ClientReleasePackage{"acme/payments": {}}, func(string) error { return nil }) + require.NoError(t, err) + require.Nil(t, plan) +} + +func TestClientReleaseRefusesSelfConsistentForeignContract(t *testing.T) { + entry := releaseClientEntry("payments", "0.1.0") + dir, _ := releaseClientFixture(t, []ContractEntry{entry}) + owner := &composition.APIContractCatalog{Package: entry.PackageID, Version: entry.PackageVersion, Endpoints: []composition.APIContractEndpoint{entry.Endpoint}} + copiedPath := filepath.Join(dir, "contract", "catalog.codefly.json") + copied, err := readClientReleaseCatalog(copiedPath) + require.NoError(t, err) + changed := []byte(`{"openapi":"3.0.0","paths":{"/different":{"get":{}}}}`) + copied.Endpoints[0].Digest = composition.APIContractDigest(changed) + copied.Endpoints[0].Routes[0].Path = "/different" + data, err := copied.CanonicalBytes() + require.NoError(t, err) + require.NoError(t, os.WriteFile(copiedPath, data, 0o600)) + require.NoError(t, os.WriteFile(filepath.Join(dir, "contract", "openapi.json"), changed, 0o600)) + before := snapshotTree(t, dir) + _, err = PlanClientRelease(dir, map[string]ClientReleasePackage{entry.PackageID: {Directory: filepath.Join(dir, "producer"), Catalog: owner}}, func(string) error { return nil }) + require.ErrorContains(t, err, "differs from its producer") + require.Equal(t, before, snapshotTree(t, dir)) +} + +func TestClientReleaseVerifiesNarrowedProducerContract(t *testing.T) { + entry := narrowEntry(narrowFixture(t), "acme.v1.Wanted") + entry.PackageID, entry.PackageVersion = "acme/payments", "0.1.0" + entry.Endpoint.API, entry.Endpoint.Path = "connect", "contracts/api/api/connect/contract.binpb" + owner := &composition.APIContractCatalog{Package: entry.PackageID, Version: entry.PackageVersion, Endpoints: []composition.APIContractEndpoint{entry.Endpoint}} + narrowed, err := narrowEntries([]ContractEntry{entry}) + require.NoError(t, err) + dir, _ := releaseClientFixture(t, narrowed) + require.NoError(t, os.WriteFile(filepath.Join(dir, "producer", entry.Endpoint.Path), entry.ContractBytes, 0o600)) + packages := map[string]ClientReleasePackage{entry.PackageID: {Directory: filepath.Join(dir, "producer"), Catalog: owner}} + plan, err := PlanClientRelease(dir, packages, func(string) error { return nil }) + require.NoError(t, err) + require.NotNil(t, plan) + // A different valid subset cannot be certified under the saved selection. + other := entry + other.Services = []string{"acme.v1.Other"} + otherEntries, err := narrowEntries([]ContractEntry{other}) + require.NoError(t, err) + require.NoError(t, writeContractFiles(t.Context(), dir, otherEntries)) + _, err = PlanClientRelease(dir, packages, func(string) error { return nil }) + require.ErrorContains(t, err, "service selection disagrees") +} + +func TestClientReleaseSamePackageAggregateKeepsAggregateVersion(t *testing.T) { + entries := []ContractEntry{releaseClientEntry("payments", "0.1.0"), releaseClientEntry("payments", "0.1.0")} + entries[1].Endpoint.Service = "worker" + entries[1].Endpoint.Path = "contracts/api/worker/rest/openapi.json" + dir, _ := releaseClientFixture(t, entries) + owner := &composition.APIContractCatalog{Package: "acme/payments", Version: "0.1.0", Endpoints: []composition.APIContractEndpoint{entries[0].Endpoint, entries[1].Endpoint}} + plan, err := PlanClientRelease(dir, map[string]ClientReleasePackage{"acme/payments": {Directory: filepath.Join(dir, "producer"), Catalog: owner}}, func(string) error { return nil }) + require.NoError(t, err) + require.Len(t, plan.Paths, 3) + require.NoError(t, plan.Write(t.Context(), map[string]string{"acme/payments": "0.2.0"})) + require.Equal(t, "0.1.0", existingTopVersion(dir)) + sources, err := readExistingLibrarySources(dir) + require.NoError(t, err) + for _, source := range sources { + require.Equal(t, "0.2.0", source.Version) + } +} From cefe53756907bce5e74941d5e84e10c9cc50aba7 Mon Sep 17 00:00:00 2001 From: Antoine Toussaint <312536060+antoinetoussaint-byte@users.noreply.github.com> Date: Tue, 27 Oct 2026 16:43:19 -0400 Subject: [PATCH 3/7] fix(stop): retain incompatible live process identity evidence --- cmd/stop_identity_test.go | 72 +++++++++++++++++++++ docs/orchestration.md | 8 +++ pkg/processgroup/boot_identity_test.go | 90 ++++++++++++++++++++++++++ pkg/processgroup/reaper.go | 7 ++ 4 files changed, 177 insertions(+) create mode 100644 cmd/stop_identity_test.go create mode 100644 pkg/processgroup/boot_identity_test.go diff --git a/cmd/stop_identity_test.go b/cmd/stop_identity_test.go new file mode 100644 index 00000000..dea9f8fb --- /dev/null +++ b/cmd/stop_identity_test.go @@ -0,0 +1,72 @@ +package cmd + +import ( + "bytes" + "context" + "encoding/json" + "fmt" + "os" + "os/exec" + "path/filepath" + "strings" + "syscall" + "testing" + "time" + + "github.com/codefly-dev/cli/pkg/processgroup" + runners "github.com/codefly-dev/core/runners/base" +) + +func TestStopReportsIncompatibleBootIdentityWithoutDiscardingEvidence(t *testing.T) { + home := t.TempDir() + t.Setenv("HOME", home) + workspace := psWorkspace(t) + t.Chdir(workspace) + child := exec.Command(os.Args[0], "-test.run=^TestPsHelper$") + child.Env = append(os.Environ(), psHelperEnv+"=1", processgroup.LaunchWorkspaceEnv+"="+workspace) + child.Dir = workspace + group, err := runners.StartTrackedProcessGroup(child) + if err != nil { + t.Fatal(err) + } + t.Cleanup(func() { _ = child.Process.Kill(); _ = child.Wait() }) + deadline := time.Now().Add(5 * time.Second) + for { + if found, ok := processgroup.LaunchWorkspaceOfProcess(child.Process.Pid); ok && found == workspace { + break + } + if time.Now().After(deadline) { + t.Fatal("child never inherited workspace") + } + time.Sleep(time.Millisecond) + } + path := filepath.Join(home, ".codefly", "runs", "authenticated-v1", fmt.Sprintf("%d.pgid", group.PGID())) + data, err := os.ReadFile(path) + if err != nil { + t.Fatal(err) + } + var record map[string]any + if err := json.Unmarshal(data, &record); err != nil { + t.Fatal(err) + } + for _, key := range []string{"leader", "owner"} { + record[key].(map[string]any)["boot_id"] = "incompatible-recorded-boot" + } + data, err = json.Marshal(record) + if err != nil { + t.Fatal(err) + } + if err := os.WriteFile(path, data, 0600); err != nil { + t.Fatal(err) + } + err = clearCommand(context.Background(), nil, stopClearOptions()) + if err == nil || !strings.Contains(err.Error(), "incompatible boot identity") { + t.Fatalf("stop reported %v", err) + } + if retained, err := os.ReadFile(path); err != nil || !bytes.Equal(retained, data) { + t.Fatalf("stop changed recovery evidence: %v", err) + } + if err := syscall.Kill(child.Process.Pid, 0); err != nil { + t.Fatalf("stop signaled unauthenticated process: %v", err) + } +} diff --git a/docs/orchestration.md b/docs/orchestration.md index b489e4f6..c9de2417 100644 --- a/docs/orchestration.md +++ b/docs/orchestration.md @@ -1129,6 +1129,14 @@ HTTP readiness deliberately stops at TCP reachability. Probing `/` would invent health contract and can itself keep a development server unready. ## Teardown +`codefly stop` retains an authenticated process-group record and returns an +error when a live leader has the recorded start identity but a different boot +identity. Older Darwin agents wrote a boot timestamp where current agents write +a boot-session UUID. This mismatch does not prove PID reuse: cleanup neither +signals the group nor discards its recovery evidence. Recover the owning run +and update its agent before restarting. A successful stop must not hide a +process it could not authenticate. + `Stop()` and `Shutdown()` do not fan every manager out at once. Launch order is not completion order, so stopping a database concurrently with the API draining into it can cut off that API's last write. Both walk **reverse topological diff --git a/pkg/processgroup/boot_identity_test.go b/pkg/processgroup/boot_identity_test.go new file mode 100644 index 00000000..2860867f --- /dev/null +++ b/pkg/processgroup/boot_identity_test.go @@ -0,0 +1,90 @@ +//go:build darwin + +package processgroup + +import ( + "bytes" + "context" + "encoding/json" + "fmt" + "io" + "os" + "os/exec" + "path/filepath" + "strconv" + "strings" + "syscall" + "testing" + + "golang.org/x/sys/unix" +) + +// Isolate the writer's pre-v0.19.7 BootID convention; leave the PID, start +// identity, executable, owner and authentication intact. +func TestOldDarwinBootIdentityRetainsLiveRecord(t *testing.T) { + home := t.TempDir() + t.Setenv("HOME", home) + dir := t.TempDir() + port := availablePort(t) + pidPath, readyPath := filepath.Join(dir, "pid"), filepath.Join(dir, "ready") + owner := exec.Command(os.Args[0], "-test.run=^TestProcessGroupHelper$") + owner.Env = append(os.Environ(), helperRoleEnv+"=authenticated-owner", helperPortEnv+"="+strconv.Itoa(port), helperPIDEnv+"="+pidPath, helperReadyEnv+"="+readyPath) + owner.Stdout, owner.Stderr = io.Discard, io.Discard + if err := owner.Start(); err != nil { + t.Fatal(err) + } + t.Cleanup(func() { _ = owner.Process.Kill(); _ = owner.Wait() }) + waitForFile(t, pidPath) + waitForFile(t, readyPath) + pid := readPID(t, pidPath) + t.Cleanup(func() { _ = syscall.Kill(-pid, syscall.SIGKILL) }) + path := filepath.Join(home, ".codefly", stateDirName, authenticatedDirName, fmt.Sprintf("%d.pgid", pid)) + original, err := os.ReadFile(path) + if err != nil { + t.Fatal(err) + } + if err := ReapStaleProcessGroups(context.Background()); err != nil { + t.Fatal(err) + } + if _, err := os.Stat(path); err != nil { + t.Fatalf("control removed current-format live record: %v", err) + } + assertPortHeld(t, port) + var record authenticatedRecord + if err := json.Unmarshal(original, &record); err != nil { + t.Fatal(err) + } + boot, err := unix.SysctlTimeval("kern.boottime") + if err != nil { + t.Fatal(err) + } + old := fmt.Sprintf("%d:%d", boot.Sec, boot.Usec) + record.Leader.BootID, record.Owner.BootID = old, old + data, err := json.Marshal(record) + if err != nil { + t.Fatal(err) + } + if err := os.WriteFile(path, data, 0600); err != nil { + t.Fatal(err) + } + for _, stop := range []bool{false, true} { + var evidence CleanupEvidence + var cleanupErr error + if stop { + evidence, cleanupErr = StopManagedProcessGroups(context.Background(), AllWorkspaces()) + } else { + evidence, cleanupErr = ReapStaleProcessGroupsWithEvidence(context.Background(), AllWorkspaces()) + } + if cleanupErr == nil || !strings.Contains(cleanupErr.Error(), "incompatible boot identity") { + t.Fatalf("stop=%v error=%v", stop, cleanupErr) + } + if evidence.Outcome != CleanupFailed { + t.Fatalf("stop=%v outcome=%v", stop, evidence.Outcome) + } + retained, err := os.ReadFile(path) + if err != nil || !bytes.Equal(retained, data) { + t.Fatalf("stop=%v changed incompatible record: %v", stop, err) + } + assertPortHeld(t, port) + } +} diff --git a/pkg/processgroup/reaper.go b/pkg/processgroup/reaper.go index 39f3e69f..23f40323 100644 --- a/pkg/processgroup/reaper.go +++ b/pkg/processgroup/reaper.go @@ -866,6 +866,13 @@ func authenticateAuthenticatedGroup(ctx context.Context, rec *authenticatedRecor } for _, member := range members { if member.pid == rec.PGID { + // A different boot representation with the same start identity is + // not proof of PID reuse. Older Darwin agents recorded a mutable + // boot timestamp instead of the boot-session UUID. Keep their + // evidence and refuse cleanup; never reinterpret it or signal it. + if member.startID == rec.Leader.StartID && member.bootID != rec.Leader.BootID { + return members, false, fmt.Errorf("process group %d has an incompatible boot identity; record retained without signaling; recover the owning run and update its agent before restarting", rec.PGID) + } return members, member.matches(&rec.Leader), nil } } From ddd27e67bff7e65721896de4a5f7ea58564159dd Mon Sep 17 00:00:00 2001 From: Antoine Toussaint <312536060+antoinetoussaint-byte@users.noreply.github.com> Date: Tue, 27 Oct 2026 16:47:38 -0400 Subject: [PATCH 4/7] fix(generators): stamp the Python project version by TOML ownership --- cmd/doctor_workspace_exclusions_test.go | 5 +- cmd/publish/client_versions_test.go | 14 +++++ cmd/publish/library.go | 48 ++------------- cmd/publish/library_test.go | 15 +++++ docs/publish-client-versions.md | 4 +- go.mod | 3 +- go.sum | 22 ++++--- pkg/generators/client_library.go | 10 +--- pkg/generators/client_version_test.go | 60 +++++++++++++++++++ pkg/generators/python_version.go | 80 +++++++++++++++++++++++++ 10 files changed, 192 insertions(+), 69 deletions(-) create mode 100644 pkg/generators/python_version.go diff --git a/cmd/doctor_workspace_exclusions_test.go b/cmd/doctor_workspace_exclusions_test.go index 823774b5..c4b2d23c 100644 --- a/cmd/doctor_workspace_exclusions_test.go +++ b/cmd/doctor_workspace_exclusions_test.go @@ -16,7 +16,7 @@ func TestDoctorJudgesOnlyServicesSelectedByTheEnvironment(t *testing.T) { `, "modules/backend/module.codefly.yaml": testModuleYAML("api", "local"), "modules/backend/services/api/service.codefly.yaml": testServiceYAML("api", "active"), - "modules/backend/services/local/service.codefly.yaml": testServiceYAML("local", "fixture-composition"), + "modules/backend/services/local/service.codefly.yaml": testServiceYAML("local", "fixture-composition", "local-only"), "configurations/blue/active.env": "VALUE=present\n", "configurations/local/active.env": "VALUE=present\n", }) @@ -25,7 +25,8 @@ func TestDoctorJudgesOnlyServicesSelectedByTheEnvironment(t *testing.T) { require.Equal(t, readinessStatusReady, report.Status, reportJSON(t, report)) require.Contains(t, findCheck(report, "services").Message, "1 service(s)") } - // The same service still imposes its requirements where it is included. + // The same service still imposes its ordinary requirements where included. + // Fixture composition itself is supplied by the dedicated empty merger. report := runReadiness(t, workspaceReadinessOptions{dir: dir, env: "local"}) requireCode(t, report, codeConfigurationMissing, checkStatusFail) // Selecting an excluded root explicitly must not claim it can run there. diff --git a/cmd/publish/client_versions_test.go b/cmd/publish/client_versions_test.go index 3093c5b9..143923a3 100644 --- a/cmd/publish/client_versions_test.go +++ b/cmd/publish/client_versions_test.go @@ -136,3 +136,17 @@ func TestModuleClientReleaseRefusesEscapingLanguagePath(t *testing.T) { }) } } + +func TestModuleClientReleasePythonVersionTargetsProjectTable(t *testing.T) { + dir, _ := clientReleaseRepo(t) + path := "clients/payments/python/pyproject.toml" + before := "[tool.example]\nversion = \"7.0.0\"\n\n[project] # package metadata\nname = 'payments'\nversion = \"0.0.0\" # release\n" + require.NoError(t, os.WriteFile(filepath.Join(dir, path), []byte(before), 0o600)) + require.NoError(t, runGit(dir, "commit", "-am", "customized Python manifest")) + require.NoError(t, runGit(dir, "push", "origin", "main")) + tag, err := moduleEngine(t, dir, "patch").Release(context.Background()) + require.NoError(t, err) + require.Equal(t, "v0.2.1", tag) + require.Equal(t, strings.Replace(before, `"0.0.0"`, `"0.2.1"`, 1), readRel(t, dir, path)) + require.NoError(t, runGit(dir, "diff", "--exit-code", "HEAD")) +} diff --git a/cmd/publish/library.go b/cmd/publish/library.go index cca84ebc..4bb4c699 100644 --- a/cmd/publish/library.go +++ b/cmd/publish/library.go @@ -11,6 +11,7 @@ import ( "github.com/Masterminds/semver" "github.com/codefly-dev/cli/cmd/common" + "github.com/codefly-dev/cli/pkg/generators" "github.com/codefly-dev/cli/pkg/librarystore" "github.com/codefly-dev/core/resources" "github.com/spf13/cobra" @@ -273,53 +274,12 @@ func readJSONVersionField(path string) (string, error) { return doc.Version, nil } -// readPyprojectVersion extracts `version = "X.Y.Z"` from the [project] table -// of a pyproject.toml. A hand-written line scan, not a TOML parser: the -// version field is the only value this command needs, and codefly does not -// otherwise depend on a TOML library. +// readPyprojectVersion uses the same project identity as generated-client +// stamping, including quoted/dotted TOML keys and comments. func readPyprojectVersion(path string) (string, error) { data, err := os.ReadFile(path) if err != nil { return "", fmt.Errorf("read %s: %w", path, err) } - inProject := false - for _, rawLine := range strings.Split(string(data), "\n") { - trimmed := strings.TrimSpace(stripTOMLComment(rawLine)) - if strings.HasPrefix(trimmed, "[") { - inProject = trimmed == "[project]" - continue - } - if !inProject || trimmed == "" { - continue - } - key, value, ok := strings.Cut(trimmed, "=") - if !ok || strings.TrimSpace(key) != "version" { - continue - } - return strings.Trim(strings.TrimSpace(value), `"'`), nil - } - return "", fmt.Errorf("%s: no version field in [project]", path) -} - -// stripTOMLComment removes a trailing, unquoted "#" comment from a TOML -// line — mirroring how parseGoModulePath strips "//" from a go.mod line — so -// a table header or value followed by a comment (`[project] # PEP 621` or -// `version = "1.0.0" # keep in sync`) is matched correctly instead of -// leaving the comment text fused onto the extracted value. A "#" inside a -// quoted string is left alone. -func stripTOMLComment(line string) string { - var quote byte - for i := 0; i < len(line); i++ { - switch c := line[i]; { - case quote != 0: - if c == quote { - quote = 0 - } - case c == '"' || c == '\'': - quote = c - case c == '#': - return line[:i] - } - } - return line + return generators.PythonProjectVersion(path, data) } diff --git a/cmd/publish/library_test.go b/cmd/publish/library_test.go index 332b3db8..5a67563d 100644 --- a/cmd/publish/library_test.go +++ b/cmd/publish/library_test.go @@ -202,3 +202,18 @@ func TestPublishLibraryUnknownNameReturnsError(t *testing.T) { cmd, _ := newTestCmd() require.Error(t, publishLibrary(cmd, "does-not-exist")) } + +func TestReadPyprojectVersionUsesTOMLProjectIdentity(t *testing.T) { + for _, before := range []string{ + "[tool.example]\nversion='7.0.0'\n[ 'project' ]\n'version'='0.1.0' # metadata\n", + "[tool.example]\ndescription='''\n[project]\nversion='7.0.0'\n'''\n[project]\nversion='0.1.0'\n", + "project.version='0.1.0'\n[tool.example]\nversion='7.0.0'\n", + "project={version='0.1.0'}\n", + } { + path := filepath.Join(t.TempDir(), "pyproject.toml") + require.NoError(t, os.WriteFile(path, []byte(before), 0o600)) + version, err := readPyprojectVersion(path) + require.NoError(t, err) + require.Equal(t, "0.1.0", version) + } +} diff --git a/docs/publish-client-versions.md b/docs/publish-client-versions.md index fc934fe0..32e2fa0a 100644 --- a/docs/publish-client-versions.md +++ b/docs/publish-client-versions.md @@ -15,7 +15,9 @@ repository; symlinks are refused. The release commit updates the selected source versions and copied catalogs. A single-source client also receives the release version in its library manifest -and TypeScript/Python package manifests. An aggregate keeps its own library and +and TypeScript/Python package manifests. Python stamping and publication both +read the TOML `project.version` field; tool versions, comments and unrelated +formatting remain unchanged. An aggregate keeps its own library and language package versions; unrelated source entries stay unchanged. Contract bytes and generation provenance are preserved. Go clients obtain their version from the release tag. diff --git a/go.mod b/go.mod index 1ed43ef7..b726f3e9 100644 --- a/go.mod +++ b/go.mod @@ -16,7 +16,7 @@ require ( github.com/codefly-dev/authority/libraries/delivery/go v0.0.0-20261026023425-000d44495d93 github.com/codefly-dev/authority/libraries/fixture-composition/go v0.0.0-20261027171336-5178f01696e3 github.com/codefly-dev/authority/libraries/hop/go v0.0.0-20261026175541-3afad7224855 - github.com/codefly-dev/core v0.19.8-0.20261027203111-6eadd92dd156 + github.com/codefly-dev/core v0.19.8-0.20261027204224-b14f7188c548 github.com/codefly-dev/golor v0.1.3 github.com/codefly-dev/llm v0.1.7 github.com/codefly-dev/sdk-go/workcontext v0.1.1-0.20261018052022-70dc7a546458 @@ -33,6 +33,7 @@ require ( github.com/moby/patternmatcher v0.6.1 github.com/opencontainers/go-digest v1.0.0 github.com/opencontainers/image-spec v1.1.1 + github.com/pelletier/go-toml/v2 v2.4.3 github.com/pmezard/go-difflib v1.0.1-0.20181226105442-5d4384ee4fb2 github.com/rs/cors v1.11.1 github.com/shirou/gopsutil/v3 v3.24.5 diff --git a/go.sum b/go.sum index b2dd79eb..ce5ab798 100644 --- a/go.sum +++ b/go.sum @@ -35,6 +35,8 @@ github.com/ProtonMail/go-crypto v1.4.1 h1:9RfcZHqEQUvP8RzecWEUafnZVtEvrBVL9BiF67 github.com/ProtonMail/go-crypto v1.4.1/go.mod h1:e1OaTyu5SYVrO9gKOEhTc+5UcXtTUa+P3uLudwcgPqo= github.com/VividCortex/ewma v1.2.0 h1:f58SaIzcDXrSy3kWaHNvuJgJ3Nmz59Zji6XoJR/q1ow= github.com/VividCortex/ewma v1.2.0/go.mod h1:nz4BbCtbLyFDeC9SUHbtcT5644juEuWfUAUnGx7j5l4= +github.com/agext/levenshtein v1.2.3 h1:YB2fHEn0UJagG8T1rrWknE3ZQzWM06O8AMAatNn7lmo= +github.com/agext/levenshtein v1.2.3/go.mod h1:JEDfjyjHDjOF/1e4FlBE/PkbqA9OfWu2ki2W0IB5558= github.com/alecthomas/assert/v2 v2.11.0 h1:2Q9r3ki8+JYXvGsDyBXwH3LcJ+WK5D0gc5E8vS6K3D0= github.com/alecthomas/assert/v2 v2.11.0/go.mod h1:Bze95FyfUr7x34QZrjL+XP+0qgp/zg8yS+TtBj1WA3k= github.com/alecthomas/chroma/v2 v2.27.0 h1:FodwmyOBgJULFYmDqibcp9pvfDLWdtPRh9v/r5BXYZs= @@ -115,10 +117,10 @@ github.com/codefly-dev/authority/libraries/hop/go v0.0.0-20261026175541-3afad722 github.com/codefly-dev/authority/libraries/hop/go v0.0.0-20261026175541-3afad7224855/go.mod h1:rfn41uiORQ36RrOeFEjXHPLC9NUi0j14AoYJtcovsn4= github.com/codefly-dev/authority/libraries/resources/go v0.0.0-20261026023425-000d44495d93 h1:4EygH7/J0E6aM7hUPdbQGfit1rsLUNCtWJ8kMh2b9PE= github.com/codefly-dev/authority/libraries/resources/go v0.0.0-20261026023425-000d44495d93/go.mod h1:1rmjrVyr0h7zOuwOHS9i+BoeS9dr3XNvtupLZFwY6As= -github.com/codefly-dev/core v0.19.7-0.20261026194550-a8599bd3991a h1:ar8ERpN9n9ZL29LZ63mqBZNLPZ7OpcnQ1+D1g89DbPY= -github.com/codefly-dev/core v0.19.7-0.20261026194550-a8599bd3991a/go.mod h1:1ina3kP+YSAL4tlZvnEYd4P8bFGnJECfkP86ojeeF1A= github.com/codefly-dev/core v0.19.8-0.20261027203111-6eadd92dd156 h1:OLQ/LG4+KGRz3crTc8zgQlxTS/Ta3Yh7OUFu+12N5u4= github.com/codefly-dev/core v0.19.8-0.20261027203111-6eadd92dd156/go.mod h1:NB7x1UZ01rYTfEZEl4vUFD1A+fgOXhsWbgA3M8x4ThA= +github.com/codefly-dev/core v0.19.8-0.20261027204224-b14f7188c548 h1:5wqbjiooPNttorByfEApw4Sxjp1VSVyDU6ciHFfiMkg= +github.com/codefly-dev/core v0.19.8-0.20261027204224-b14f7188c548/go.mod h1:NB7x1UZ01rYTfEZEl4vUFD1A+fgOXhsWbgA3M8x4ThA= github.com/codefly-dev/golor v0.1.3 h1:xmo+ceyJFRYZdvpWE2fNd0jeaadp/Ibm1BnganiGKOc= github.com/codefly-dev/golor v0.1.3/go.mod h1:sl/u/K1l7J0Pr3xyVZp8fOJYQItKKst1No9JqgzLLoY= github.com/codefly-dev/gortk v0.2.0 h1:7bOlS5valYz2zil+fZctQNcPCYBcPj86abcw9N8h1hQ= @@ -133,9 +135,8 @@ github.com/containerd/errdefs v1.0.0 h1:tg5yIfIlQIrxYtu9ajqY42W3lpS19XqdxRQeEwYG github.com/containerd/errdefs v1.0.0/go.mod h1:+YBYIdtsnF4Iw6nWZhJcqGSg/dwvV7tyJ/kCkyJ2k+M= github.com/containerd/errdefs/pkg v0.3.0 h1:9IKJ06FvyNlexW690DXuQNx2KA2cUJXx151Xdx3ZPPE= github.com/containerd/errdefs/pkg v0.3.0/go.mod h1:NJw6s9HwNuRhnjJhM7pylWwMyAkmCQvQ4GpJHEqRLVk= -github.com/containerd/log v0.1.0 h1:TCJt7ioM2cr/tfR8GPbGf9/VRAX8D2B4PjzCpfX540I= -github.com/containerd/log v0.1.0/go.mod h1:VRRf09a7mHDIRezVKTRCrOq78v577GXq3bSa3EhrzVo= github.com/containerd/log v0.2.0 h1:BewD/umNgVnoczglOpX8eRMyEy5t5iPlu5AIpnWDONc= +github.com/containerd/log v0.2.0/go.mod h1:/M7L7CXKcPTfNC74XzaK+5H5KbO5+4lJVpuVI6vRLoM= github.com/containerd/typeurl/v2 v2.3.0 h1:HZHPhRWo5XMy3QGQoPrUzbW/2ckwjfweHmOwlkIrPAQ= github.com/containerd/typeurl/v2 v2.3.0/go.mod h1:Qk+PAdUYArVj41TnGi6rJ+48RF0PkcTc4i/taoBcK0w= github.com/cpuguy83/go-md2man/v2 v2.0.6/go.mod h1:oOW0eioCTA6cOiMLiUPZOpcVxMig6NIQQ7OS05n1F4g= @@ -333,8 +334,6 @@ github.com/mitchellh/mapstructure v1.5.0 h1:jeMsZIYE/09sWLaz43PL7Gy6RuMjD2eJVyua github.com/mitchellh/mapstructure v1.5.0/go.mod h1:bFUtVrKA4DC2yAKiSyO/QUcy7e+RRV2QTWOzhPopBRo= github.com/mitchellh/reflectwalk v1.0.2 h1:G2LzWKi524PWgd3mLHV8Y5k7s6XUvT0Gef6zxSIeXaQ= github.com/mitchellh/reflectwalk v1.0.2/go.mod h1:mSTlrgnPZtwu0c4WaC2kGObEpuNDbx0jmZXqmk4esnw= -github.com/moby/buildkit v0.32.2 h1:Sfy7+u6dUv/2yuBc9KCoK70Re8atuV8aPZ5UOC068Vc= -github.com/moby/buildkit v0.32.2/go.mod h1:0GB/EJ1d+4VIVqIAgy3asaoGkVXy7IrDfVy7mPhOvg8= github.com/moby/buildkit v0.34.1 h1:jVdxsXX0UpU0q1Et+9lFezPmh9sGE6vF0TrKZaGlfy4= github.com/moby/buildkit v0.34.1/go.mod h1:e+Za70y2a3nL1U2Y0P8TvhPyKp2Ok2NS+Q4siSArDiM= github.com/moby/docker-image-spec v1.3.1 h1:jMKff3w6PgbfSa69GfNg+zN/XLhfXJGnEx3Nl2EsFP0= @@ -372,6 +371,8 @@ github.com/opencontainers/image-spec v1.1.1/go.mod h1:qpqAh3Dmcf36wStyyWU+kCeDgr github.com/pb33f/ordered-map/v2 v2.3.1 h1:5319HDO0aw4DA4gzi+zv4FXU9UlSs3xGZ40wcP1nBjY= github.com/pb33f/ordered-map/v2 v2.3.1/go.mod h1:qxFQgd0PkVUtOMCkTapqotNgzRhMPL7VvaHKbd1HnmQ= github.com/pborman/getopt v0.0.0-20170112200414-7148bc3a4c30/go.mod h1:85jBQOZwpVEaDAr341tbn15RS4fCAsIst0qp7i8ex1o= +github.com/pelletier/go-toml/v2 v2.4.3 h1:GTRvJQutkOSftxIFD5xw9aepkYNuPWmVJpffdDPYVpY= +github.com/pelletier/go-toml/v2 v2.4.3/go.mod h1:2gIqNv+qfxSVS7cM2xJQKtLSTLUE9V8t9Stt+h56mCY= github.com/pjbgf/sha1cd v0.7.0 h1:ZRNPKHj+gfkLBf0KJv/p1Hmz+7bqCT5o311YX0nq+DA= github.com/pjbgf/sha1cd v0.7.0/go.mod h1:pKR5Li+qTCo+ebqITZfwPlJGoCFCvSO00qxjbNtTUFQ= github.com/pkg/errors v0.9.1 h1:FEBLx1zS214owpjy7qsBeixbURkuhQAwrK5UwLGTwt4= @@ -409,8 +410,6 @@ github.com/shoenig/test v1.7.0/go.mod h1:UxJ6u/x2v/TNs/LoLxBNJRV9DiwBBKYxXSyczsB github.com/shopspring/decimal v1.4.0 h1:bxl37RwXBklmTi0C79JfXCEBD1cqqHt0bbgBAGFp81k= github.com/shopspring/decimal v1.4.0/go.mod h1:gawqmDU56v4yIKSwfBSFip1HdCCXN8/+DMd9qYNcwME= github.com/sirupsen/logrus v1.7.0/go.mod h1:yWOB1SBYBC5VeMP7gHvWumXLIWorT60ONWic61uBYv0= -github.com/sirupsen/logrus v1.10.0 h1:T8MxJJXVZkfcC5zSRMRAg2F8+lxjmUCGGWPzFxO+Msc= -github.com/sirupsen/logrus v1.10.0/go.mod h1:FXZFonkDAnFozmO+5hGAFvB0Yg9/j2SIhA/QuIkP180= github.com/sirupsen/logrus v1.10.2 h1:G2SED73/qrAu6YwbdxOD6peLkCBI3z7L+ykJFTXJBBo= github.com/sirupsen/logrus v1.10.2/go.mod h1:SLEg8TqYulVKKfIGHldVp2K2aYz2DKSVBq4g/H5bR7Q= github.com/skeema/knownhosts v1.3.2 h1:EDL9mgf4NzwMXCTfaxSD/o/a5fxDw/xL9nkU28JjdBg= @@ -448,6 +447,8 @@ github.com/tklauser/go-sysconf v0.4.0 h1:7H0uAN+7RkwWRaxhYXDLqa5V3LPrJeV8wmD9dRU github.com/tklauser/go-sysconf v0.4.0/go.mod h1:8mTNWyog7H+MpKijp4VmKJAd2bbYQ2zuUwkYRbUArPI= github.com/tklauser/numcpus v0.12.0 h1:NR85qdvHA9pFse3x3weVZ0r0ST8R6l5RHbZrlRaqob4= github.com/tklauser/numcpus v0.12.0/go.mod h1:ABHeXzJnr/qqwguhClkZKT1/8VABcYrsyUiUGobwWJg= +github.com/tonistiigi/go-csvvalue v0.0.0-20240814133006-030d3b2625d0 h1:2f304B10LaZdB8kkVEaoXvAMVan2tl9AiK4G0odjQtE= +github.com/tonistiigi/go-csvvalue v0.0.0-20240814133006-030d3b2625d0/go.mod h1:278M4p8WsNh3n4a1eqiFcV2FGk7wE5fwUpUom9mK9lE= github.com/tree-sitter-grammars/tree-sitter-kotlin v1.1.0 h1:SWIUDASa+WPhDDem1U5IJpYwQEezkqXrUI61OcnORzM= github.com/tree-sitter-grammars/tree-sitter-kotlin v1.1.0/go.mod h1:eH+flFf3QOa9c9BY9g3Bz02F7zTq30kGIG3cgB0lSlI= github.com/tree-sitter/go-tree-sitter v0.25.0 h1:sx6kcg8raRFCvc9BnXglke6axya12krCJF5xJ2sftRU= @@ -519,9 +520,8 @@ go.opentelemetry.io/otel/exporters/otlp/otlptrace v1.47.0 h1:julhjPeUH/q/7hinbSd go.opentelemetry.io/otel/exporters/otlp/otlptrace v1.47.0/go.mod h1:Ao2mz688LH/tFf0yMAenidq6k2YNSx6SIY2q6jDACck= go.opentelemetry.io/otel/exporters/otlp/otlptrace/otlptracegrpc v1.47.0 h1:UFpxOpYPmMNUtOWhdb+nC1WELIgVf8z9higURrbzYU4= go.opentelemetry.io/otel/exporters/otlp/otlptrace/otlptracegrpc v1.47.0/go.mod h1:YBGjxe3lt0jtXoEXxGrGhv/jM7oUBUDVK7zhAvNyEjc= -go.opentelemetry.io/otel/exporters/otlp/otlptrace/otlptracehttp v1.45.0 h1:QBajQ2SrwQijzHyZbQlPsuIzpl/ll8DY6wPWsajeGcI= -go.opentelemetry.io/otel/exporters/otlp/otlptrace/otlptracehttp v1.45.0/go.mod h1:08ZQLjrPLQ6R4kAXvuOvODEer5Yh4CoFvll5qB2BCI8= go.opentelemetry.io/otel/exporters/otlp/otlptrace/otlptracehttp v1.46.0 h1:KrC1YrQeSt46ITMWAbgQx1M1eV1/1TKzttrBzymPmss= +go.opentelemetry.io/otel/exporters/otlp/otlptrace/otlptracehttp v1.46.0/go.mod h1:zDSEzoEqsOrgBeGvH66KRgxh90VonFyJqBHA0Pk3+rM= go.opentelemetry.io/otel/exporters/stdout/stdouttrace v1.47.0 h1:N3YQCxjxQ/bMjyc3heladfRm9t9RTksGQH8z4w6yU/0= go.opentelemetry.io/otel/exporters/stdout/stdouttrace v1.47.0/go.mod h1:Mp8HOFqcaUyypCuGv9IhDdTHnJ56lSudSHMd+pVSCEA= go.opentelemetry.io/otel/log v1.47.0 h1:cOTS1CcLbSQeZKanGJ+0JpF/+t4PELi3O3bbl2lqCcI= @@ -587,8 +587,6 @@ golang.org/x/text v0.3.3/go.mod h1:5Zoc/QRtKVWzQhOtBMvqHzDpF6irO9z98xDceosuGiQ= golang.org/x/text v0.3.6/go.mod h1:5Zoc/QRtKVWzQhOtBMvqHzDpF6irO9z98xDceosuGiQ= golang.org/x/text v0.42.0 h1:JbOZXgfeCPU9gacVtYliJqOhD+zhrEqK4LfdpmlUZqI= golang.org/x/text v0.42.0/go.mod h1:ojzP1Z+2QtioaF8DTtO8K5q7JWVVYwZKenzujK0Zd0E= -golang.org/x/time v0.15.0 h1:bbrp8t3bGUeFOx08pvsMYRTCVSMk89u4tKbNOZbp88U= -golang.org/x/time v0.15.0/go.mod h1:Y4YMaQmXwGQZoFaVFk4YpCt4FLQMYKZe9oeV/f4MSno= golang.org/x/time v0.16.0 h1:vMb6ptszcQMkcwiRTAuNNU50gom6++Q/6gY2hDM6VDE= golang.org/x/time v0.16.0/go.mod h1:rVKOqvZeKvrDKTQiAHJ7wmwP0RzleSphoEA9RcdLA0s= golang.org/x/tools v0.0.0-20180917221912-90fa682c2a6e/go.mod h1:n7NCudcB/nEzxVGmLbDWY5pfWTLqBcC2KZ6jyYvM4mQ= diff --git a/pkg/generators/client_library.go b/pkg/generators/client_library.go index a24947bf..ae6d0e58 100644 --- a/pkg/generators/client_library.go +++ b/pkg/generators/client_library.go @@ -825,10 +825,6 @@ func writeTypeScriptScaffold(langDir, npmPackage string) error { return writeFileIfMissing(filepath.Join(langDir, "tsconfig.json"), []byte(tsconfig)) } -// pyprojectVersionRe matches pyproject.toml's project version line, the shape -// writePythonScaffold writes. -var pyprojectVersionRe = regexp.MustCompile(`(?m)^version = ("[^"]*")$`) - // stampPackageVersion sets the version a language's package manifest declares // to the library's own. The scaffold is written once and kept, so without this // every generated TypeScript and Python client stays at the scaffold's 0.0.1 @@ -904,11 +900,7 @@ func locatePackageVersion(lang languages.Language, path string, data []byte) (st if lang == languages.TYPESCRIPT { return locatePackageJSONVersion(path, data) } - match := pyprojectVersionRe.FindSubmatchIndex(data) - if match == nil { - return 0, 0, fmt.Errorf("cannot find a version in %s", path) - } - return match[2], match[3], nil + return locatePythonPackageVersion(path, data) } // locatePackageJSONVersion finds the value of package.json's top-level diff --git a/pkg/generators/client_version_test.go b/pkg/generators/client_version_test.go index 9713d98c..0a651b91 100644 --- a/pkg/generators/client_version_test.go +++ b/pkg/generators/client_version_test.go @@ -182,3 +182,63 @@ func TestCheckPackageVersionStampable(t *testing.T) { t.Fatalf("go: %v", err) } } + +// Source offsets belong to project.version, independent of where another +// table's version or a quoted/multiline TOML value appears in the file. +func TestPythonVersionStampOnlyChangesProjectVersion(t *testing.T) { + for _, test := range []struct { + name, before, oldValue string + }{ + {"tool first", "[tool.example]\nversion = \"7.0.0\"\n\n[project]\nversion = \"0.1.0\"\n", `"0.1.0"`}, + {"quoted table and literal key", "[ 'project' ] # metadata\n\t'version'\t=\t'0.1.0' # release\n[tool.example]\nversion='7.0.0'\n", `'0.1.0'`}, + {"escaped key and table", "[\"pro\\u006Aect\"]\r\n\"ver\\u0073ion\" = \"0.1.0\" # release\r\n", `"0.1.0"`}, + {"multiline decoy", "[tool.example]\ndescription = '''\n[project]\nversion = \"7.0.0\"\n'''\n[project]\nversion = \"0.1.0\"\n", `"0.1.0"`}, + {"nested project table", "[project.urls]\nversion = \"7.0.0\"\n[project]\nversion = \"0.1.0\"\n", `"0.1.0"`}, + {"dotted root key", "project.version = '0.1.0' # release\n[tool.example]\nversion='7.0.0'\n", `'0.1.0'`}, + {"inline root table", "project = { name='client', version = '0.1.0' } # release\n", `'0.1.0'`}, + } { + t.Run(test.name, func(t *testing.T) { + dir := t.TempDir() + path := filepath.Join(dir, "pyproject.toml") + if err := os.WriteFile(path, []byte(test.before), 0o600); err != nil { + t.Fatal(err) + } + if err := stampPackageVersion(languages.PYTHON, dir, "0.2.0"); err != nil { + t.Fatal(err) + } + after, err := os.ReadFile(path) + if err != nil { + t.Fatal(err) + } + want := strings.Replace(test.before, test.oldValue, `"0.2.0"`, 1) + if string(after) != want { + t.Fatalf("got:\n%s\nwant:\n%s", after, want) + } + }) + } +} + +func TestPythonVersionStampRefusesMissingOrInvalidProjectVersion(t *testing.T) { + for _, before := range []string{ + "[tool.example]\nversion = \"7.0.0\"\n[project]\ndynamic=['version']\n", + "[project]\nversion = 17\n", + "[project]\nversion = \"0.1.0\"\nversion = \"0.2.0\"\n", + "[project]\nversion = \"0.1.0\"\n[broken\n", + } { + dir := t.TempDir() + path := filepath.Join(dir, "pyproject.toml") + if err := os.WriteFile(path, []byte(before), 0o600); err != nil { + t.Fatal(err) + } + if err := stampPackageVersion(languages.PYTHON, dir, "0.2.0"); err == nil { + t.Fatalf("accepted invalid/missing project version:\n%s", before) + } + after, err := os.ReadFile(path) + if err != nil { + t.Fatal(err) + } + if string(after) != before { + t.Fatalf("refused stamp changed file:\n%s", after) + } + } +} diff --git a/pkg/generators/python_version.go b/pkg/generators/python_version.go new file mode 100644 index 00000000..fdc05f83 --- /dev/null +++ b/pkg/generators/python_version.go @@ -0,0 +1,80 @@ +package generators + +import ( + "fmt" + "slices" + + "github.com/pelletier/go-toml/v2" + "github.com/pelletier/go-toml/v2/unstable" +) + +// locatePythonPackageVersion uses TOML's semantic keys and source ranges to +// replace only project.version. Decoding first rejects invalid/duplicate tables; +// the AST retains comments, quoting and unrelated bytes instead of reformatting +// a manifest a developer owns. Strings containing apparent TOML stay strings. +func locatePythonPackageVersion(path string, data []byte) (int, int, error) { + if _, err := PythonProjectVersion(path, data); err != nil { + return 0, 0, err + } + var parser unstable.Parser + parser.Reset(data) + var table []string + for parser.NextExpression() { + node := parser.Expression() + switch node.Kind { + case unstable.Table, unstable.ArrayTable: + table = tomlKeyPath(nil, node) + case unstable.KeyValue: + if span, ok := pythonVersionRange(node, table); ok { + return int(span.Offset), int(span.Offset + span.Length), nil + } + } + } + if err := parser.Error(); err != nil { + return 0, 0, fmt.Errorf("cannot parse %s: %w", path, err) + } + return 0, 0, fmt.Errorf("cannot find a version in [project] in %s", path) +} + +// PythonProjectVersion reads the static PEP 621 metadata version used by both +// generated-client stamping and publication. A tool's version is unrelated. +func PythonProjectVersion(path string, data []byte) (string, error) { + var document map[string]any + if err := toml.Unmarshal(data, &document); err != nil { + return "", fmt.Errorf("cannot parse %s: %w", path, err) + } + project, _ := document["project"].(map[string]any) + version, ok := project["version"].(string) + if !ok { + return "", fmt.Errorf("cannot find a version string in [project] in %s", path) + } + return version, nil +} + +func tomlKeyPath(prefix []string, node *unstable.Node) []string { + path := slices.Clone(prefix) + keys := node.Key() + for keys.Next() { + path = append(path, string(keys.Node().Data)) + } + return path +} + +func pythonVersionRange(node *unstable.Node, table []string) (unstable.Range, bool) { + path := tomlKeyPath(table, node) + value := node.Value() + if slices.Equal(path, []string{"project", "version"}) && value.Kind == unstable.String { + return value.Raw, true + } + // TOML also allows project = { version = "..." }. Only descend into + // that one table, never unrelated tables or arrays containing versions. + if slices.Equal(path, []string{"project"}) && value.Kind == unstable.InlineTable { + children := value.Children() + for children.Next() { + if span, ok := pythonVersionRange(children.Node(), path); ok { + return span, true + } + } + } + return unstable.Range{}, false +} From 293c2919e11ccf14009725e0d03bc2451e27552a Mon Sep 17 00:00:00 2001 From: Antoine Toussaint <312536060+antoinetoussaint-byte@users.noreply.github.com> Date: Tue, 27 Oct 2026 17:10:22 -0400 Subject: [PATCH 5/7] fix(stop): preserve registry refusals through cleanup fallbacks --- cmd/clear.go | 11 ++- cmd/stop_identity_fallback_test.go | 117 +++++++++++++++++++++++++++++ cmd/stop_identity_test.go | 1 + docs/orchestration.md | 3 +- docs/supported-matrix.md | 34 ++------- go.mod | 2 +- go.sum | 2 + pkg/conformance/matrix.json | 2 +- pkg/processgroup/composed_test.go | 6 +- pkg/processgroup/launch.go | 14 ++-- 10 files changed, 150 insertions(+), 42 deletions(-) create mode 100644 cmd/stop_identity_fallback_test.go diff --git a/cmd/clear.go b/cmd/clear.go index 9fe07012..d7a63a77 100644 --- a/cmd/clear.go +++ b/cmd/clear.go @@ -219,9 +219,14 @@ func clearCommand(ctx context.Context, args []string, options clearOptions) (ret failures = append(failures, killCodeflyProcesses(ctx, w, options)...) } - failures = append(failures, sweepStaleState(ctx, w, options)...) - failures = append(failures, reapOrphanedDevServers(ctx, w, options)...) - failures = append(failures, reapOrphanedNativeServices(ctx, w, options)...) + managedFailures := sweepStaleState(ctx, w, options) + failures = append(failures, managedFailures...) + // The fallback scans must not turn a failed registry authentication into + // permission to signal the same group by its executable signature. + if len(managedFailures) == 0 { + failures = append(failures, reapOrphanedDevServers(ctx, w, options)...) + failures = append(failures, reapOrphanedNativeServices(ctx, w, options)...) + } if options.keepContainers { return errors.Join(failures...) diff --git a/cmd/stop_identity_fallback_test.go b/cmd/stop_identity_fallback_test.go new file mode 100644 index 00000000..4feba4a5 --- /dev/null +++ b/cmd/stop_identity_fallback_test.go @@ -0,0 +1,117 @@ +package cmd + +import ( + "bytes" + "context" + "encoding/json" + "fmt" + "github.com/codefly-dev/cli/pkg/processgroup" + runners "github.com/codefly-dev/core/runners/base" + "io" + "os" + "os/exec" + "path/filepath" + "strconv" + "strings" + "syscall" + "testing" + "time" +) + +func TestStopIdentityFallbackOwner(t *testing.T) { + target := os.Getenv("CODEFLY_TEST_STOP_IDENTITY_CHILD_PID") + if target == "" { + return + } + child := exec.Command(os.Getenv("CODEFLY_TEST_STOP_IDENTITY_CHILD_BINARY"), "-test.run=^TestPsHelper$") + child.Env = append(os.Environ(), psHelperEnv+"=1") + child.Stdout = io.Discard + child.Stderr = io.Discard + group, err := runners.StartTrackedProcessGroup(child) + if err != nil { + t.Fatal(err) + } + if err = os.WriteFile(target, []byte(strconv.Itoa(group.PGID())), 0600); err != nil { + t.Fatal(err) + } +} +func TestStopDoesNotFallbackAfterBootIdentityRefusal(t *testing.T) { + t.Setenv("HOME", t.TempDir()) + workspace := psWorkspace(t) + t.Chdir(workspace) + t.Setenv(processgroup.LaunchWorkspaceEnv, workspace) + binary := filepath.Join(t.TempDir(), ".cache", "native", "stop-child") + if err := os.MkdirAll(filepath.Dir(binary), 0700); err != nil { + t.Fatal(err) + } + executable, err := os.ReadFile(os.Args[0]) + if err != nil { + t.Fatal(err) + } + if err = os.WriteFile(binary, executable, 0700); err != nil { + t.Fatal(err) + } + t.Setenv("CODEFLY_TEST_STOP_IDENTITY_CHILD_BINARY", binary) + pidFile := filepath.Join(t.TempDir(), "pid") + owner := exec.Command(os.Args[0], "-test.run=^TestStopIdentityFallbackOwner$") + owner.Env = append(os.Environ(), "CODEFLY_TEST_STOP_IDENTITY_CHILD_PID="+pidFile) + if out, err := owner.CombinedOutput(); err != nil { + t.Fatalf("owner: %v %s", err, out) + } + raw, err := os.ReadFile(pidFile) + if err != nil { + t.Fatal(err) + } + pid, err := strconv.Atoi(string(raw)) + if err != nil { + t.Fatal(err) + } + t.Cleanup(func() { _ = syscall.Kill(pid, syscall.SIGKILL) }) + path := filepath.Join(os.Getenv("HOME"), ".codefly", "runs", "authenticated-v1", fmt.Sprintf("%d.pgid", pid)) + data, err := os.ReadFile(path) + if err != nil { + t.Fatal(err) + } + var record map[string]any + if err = json.Unmarshal(data, &record); err != nil { + t.Fatal(err) + } + for _, k := range []string{"leader", "owner"} { + record[k].(map[string]any)["boot_id"] = "incompatible-recorded-boot" + } + data, err = json.Marshal(record) + if err != nil { + t.Fatal(err) + } + if err = os.WriteFile(path, data, 0600); err != nil { + t.Fatal(err) + } + deadline := time.Now().Add(5 * time.Second) + for { + rows, scanErr := scanRunProcesses(context.Background(), processgroup.InWorkspace(workspace)) + if scanErr != nil { + t.Fatal(scanErr) + } + found := false + for _, row := range rows { + found = found || (row.PID == pid && row.Kind == "native" && row.Status == "orphaned") + } + if found { + break + } + if time.Now().After(deadline) { + t.Fatal("child never became eligible for orphan fallback") + } + time.Sleep(10 * time.Millisecond) + } + err = clearCommand(context.Background(), nil, stopClearOptions()) + if err == nil || !strings.Contains(err.Error(), "incompatible boot identity") { + t.Fatalf("expected boot refusal: %v", err) + } + if syscall.Kill(pid, 0) != nil { + t.Fatalf("stop killed incompatible registered group %d through fallback after refusal", pid) + } + if retained, readErr := os.ReadFile(path); readErr != nil || !bytes.Equal(retained, data) { + t.Fatalf("stop changed incompatible record: %v", readErr) + } +} diff --git a/cmd/stop_identity_test.go b/cmd/stop_identity_test.go index dea9f8fb..eece708e 100644 --- a/cmd/stop_identity_test.go +++ b/cmd/stop_identity_test.go @@ -22,6 +22,7 @@ func TestStopReportsIncompatibleBootIdentityWithoutDiscardingEvidence(t *testing t.Setenv("HOME", home) workspace := psWorkspace(t) t.Chdir(workspace) + t.Setenv(processgroup.LaunchWorkspaceEnv, workspace) child := exec.Command(os.Args[0], "-test.run=^TestPsHelper$") child.Env = append(os.Environ(), psHelperEnv+"=1", processgroup.LaunchWorkspaceEnv+"="+workspace) child.Dir = workspace diff --git a/docs/orchestration.md b/docs/orchestration.md index 5ca3172f..af09773b 100644 --- a/docs/orchestration.md +++ b/docs/orchestration.md @@ -1135,7 +1135,8 @@ identity. Older Darwin agents wrote a boot timestamp where current agents write a boot-session UUID. This mismatch does not prove PID reuse: cleanup neither signals the group nor discards its recovery evidence. Recover the owning run and update its agent before restarting. A successful stop must not hide a -process it could not authenticate. +process it could not authenticate. If registry cleanup fails, stop also skips +the signature-based orphan fallback so it cannot bypass that refusal. `Stop()` and `Shutdown()` do not fan every manager out at once. Launch order is not completion order, so stopping a database concurrently with the API draining diff --git a/docs/supported-matrix.md b/docs/supported-matrix.md index 6f81c7c7..93646fbe 100644 --- a/docs/supported-matrix.md +++ b/docs/supported-matrix.md @@ -27,33 +27,13 @@ When `go.mod` moves to a new core release line, update the `core` field in `go test ./pkg/conformance/...` before the full coverage and race gates. `TestMatrixCoreMatchesGoMod` refuses a stale declared version. -The current pin is a `v0.19.7` pseudo-version of core: `257e6b23`, branch -`chore/envelope-on-0.19.6`, which merges `v0.19.6` (with the git source -declaration, codefly-dev/core#832) into `5522c4da`, the commit carrying the -module envelope that `pkg/delivery/signing` uses. `v0.19.6` carries -`b81d92e6`, the merge of codefly-dev/core#812 — the module manifest -(`solutionhost.ModuleManifest`, one signed document per module, whose -workload is `(service, container)` and which names no build) and -`workloadidentity`, which a native run attests its workloads with. Its -companion is `github.com/codefly-dev/sdk-go/workcontext` at `70dc7a54` -(codefly-dev/sdk-go#80, a credential carries no build seal). Releasing a CLI -on it requires core `v0.19.6` and a `workcontext` release past `v0.1.0` to -be tagged first. The previous pin, a `v0.19.5` pseudo-version, carried the -`runtime-started-artifact/v1` contract (codefly-dev/core#796, -codefly-dev/cli#989), which this CLI no longer reads. On top of `v0.19.2`, `v0.19.4` adds -the issuer's non-consuming `Authority.ReadBack` and moves kustomize back to -`v0.21.1`, whose kube-openapi does not pull `structured-merge-diff/v7` (the -`v0.21.3` that `v0.19.2` and `v0.19.3` carried broke every build compiling -client-go). `v0.19.2` added a -solution manifest's scoped `api.consumes` and its consumption contract (#784) -and an issuer-assigned Work Context session (#783). `v0.19.1` -verifies delivered carriers -against pinned platform keys (#773), projects a service account entirely on a -caller-supplied filesystem (#774, #777), and carries ToolProjection and slot-only authority (#755), ciguard (#750), caller tokens, -image-lock/publish evidence (#759), and configuration-origin evidence (#746), -in addition to #752's composition-provenance contract. This source matrix does -not qualify released agents for the new -`deployment-composition-provenance/v1` capability. +The current pin targets Core's `v0.19.8` release line. The exact source +revision is declared in `go.mod`; the CLI consumes Core's container parent +attribution and release acquisition APIs. These source gates qualify the CLI +against that selected revision. They do not qualify separately compiled agents +or imply that the target Core release has been published. Agents creating +sidecar containers must adopt the attribution API in their own source and +qualify their lifecycle before consumers select the resulting release. A missing receipt can be a consequence of an earlier failure: coverage and race run `go test -failfast`, so a failed package can stop the npm tests in diff --git a/go.mod b/go.mod index b726f3e9..b952df94 100644 --- a/go.mod +++ b/go.mod @@ -16,7 +16,7 @@ require ( github.com/codefly-dev/authority/libraries/delivery/go v0.0.0-20261026023425-000d44495d93 github.com/codefly-dev/authority/libraries/fixture-composition/go v0.0.0-20261027171336-5178f01696e3 github.com/codefly-dev/authority/libraries/hop/go v0.0.0-20261026175541-3afad7224855 - github.com/codefly-dev/core v0.19.8-0.20261027204224-b14f7188c548 + github.com/codefly-dev/core v0.19.8-0.20261027204840-3d31544addf5 github.com/codefly-dev/golor v0.1.3 github.com/codefly-dev/llm v0.1.7 github.com/codefly-dev/sdk-go/workcontext v0.1.1-0.20261018052022-70dc7a546458 diff --git a/go.sum b/go.sum index ce5ab798..a34d99e1 100644 --- a/go.sum +++ b/go.sum @@ -121,6 +121,8 @@ github.com/codefly-dev/core v0.19.8-0.20261027203111-6eadd92dd156 h1:OLQ/LG4+KGR github.com/codefly-dev/core v0.19.8-0.20261027203111-6eadd92dd156/go.mod h1:NB7x1UZ01rYTfEZEl4vUFD1A+fgOXhsWbgA3M8x4ThA= github.com/codefly-dev/core v0.19.8-0.20261027204224-b14f7188c548 h1:5wqbjiooPNttorByfEApw4Sxjp1VSVyDU6ciHFfiMkg= github.com/codefly-dev/core v0.19.8-0.20261027204224-b14f7188c548/go.mod h1:NB7x1UZ01rYTfEZEl4vUFD1A+fgOXhsWbgA3M8x4ThA= +github.com/codefly-dev/core v0.19.8-0.20261027204840-3d31544addf5 h1:08ZV27FM2N4V6NtGrp8T5k+Fc73Ten7BCX6FRkFIba4= +github.com/codefly-dev/core v0.19.8-0.20261027204840-3d31544addf5/go.mod h1:NB7x1UZ01rYTfEZEl4vUFD1A+fgOXhsWbgA3M8x4ThA= github.com/codefly-dev/golor v0.1.3 h1:xmo+ceyJFRYZdvpWE2fNd0jeaadp/Ibm1BnganiGKOc= github.com/codefly-dev/golor v0.1.3/go.mod h1:sl/u/K1l7J0Pr3xyVZp8fOJYQItKKst1No9JqgzLLoY= github.com/codefly-dev/gortk v0.2.0 h1:7bOlS5valYz2zil+fZctQNcPCYBcPj86abcw9N8h1hQ= diff --git a/pkg/conformance/matrix.json b/pkg/conformance/matrix.json index e17544d6..7ad1a155 100644 --- a/pkg/conformance/matrix.json +++ b/pkg/conformance/matrix.json @@ -1,7 +1,7 @@ { "schema_version": 1, "cli": "source", - "core": "v0.19.7", + "core": "v0.19.8", "rows": [ { "id": "linux-amd64-source", diff --git a/pkg/processgroup/composed_test.go b/pkg/processgroup/composed_test.go index dd517ff4..8e325ae8 100644 --- a/pkg/processgroup/composed_test.go +++ b/pkg/processgroup/composed_test.go @@ -43,6 +43,9 @@ func newDirectory(t *testing.T, name string) string { // looks like and must still be attributable from its working directory. func startRunFrom(t *testing.T, workspace, dir string) int { t.Helper() + // Core transports the current parent's marker even when cmd.Env was + // materialized elsewhere. Model the launching run, not cached child input. + t.Setenv(LaunchWorkspaceEnv, workspace) port := availablePort(t) readyPath := filepath.Join(dir, "ready-"+strconv.Itoa(port)) command := exec.Command(os.Args[0], "-test.run=^TestProcessGroupHelper$") @@ -50,9 +53,6 @@ func startRunFrom(t *testing.T, workspace, dir string) int { helperRoleEnv+"=listener", helperPortEnv+"="+strconv.Itoa(port), helperReadyEnv+"="+readyPath) - if workspace != "" { - command.Env = append(command.Env, LaunchWorkspaceEnv+"="+workspace) - } command.Dir = dir command.Stdout = io.Discard command.Stderr = io.Discard diff --git a/pkg/processgroup/launch.go b/pkg/processgroup/launch.go index 57ceb87a..74ce344d 100644 --- a/pkg/processgroup/launch.go +++ b/pkg/processgroup/launch.go @@ -4,14 +4,16 @@ import ( "bytes" "os" "path/filepath" + + runners "github.com/codefly-dev/core/runners/base" ) // LaunchWorkspaceEnv carries the workspace whose run started a process. The CLI -// records it in its OWN environment before it starts anything, which is what -// makes every descendant carry it: the agents it spawns, the runners inside -// them, the service binaries those exec, and the stateful stores they bring up. -// Nothing downstream has to cooperate, which matters because agents are -// released independently of this repository and cannot be asked to. +// records it in its own environment before starting agents. Core's tracked and +// owned process launchers carry the current marker into service processes even +// when a runner materializes a separate environment, as Nix does. Independently +// released agents must adopt that Core transport; ordinary environment +// inheritance alone does not preserve attribution through those runners. // // It exists because a working directory cannot answer "whose run is this?". A // composed module is checked out OUTSIDE the workspace that composes it — a @@ -21,7 +23,7 @@ import ( // by working directory therefore misses precisely the processes a composed run // adds. The stores are the same gap from the other end: a postgres cluster runs // from a data directory under ~/.codefly/data, which is in no workspace at all. -const LaunchWorkspaceEnv = "CODEFLY_RUN_WORKSPACE" +const LaunchWorkspaceEnv = runners.LaunchWorkspaceEnv // MarkLaunchWorkspace records dir as the workspace this process's run belongs // to, so everything this process goes on to start is attributable to it. From 979661938acf6668ef2e4d93e874df10bfb5be5f Mon Sep 17 00:00:00 2001 From: Antoine Toussaint <312536060+antoinetoussaint-byte@users.noreply.github.com> Date: Tue, 27 Oct 2026 17:27:16 -0400 Subject: [PATCH 6/7] fix(stop): consume authenticated workspace provenance from Core --- cmd/clear.go | 142 ++----------------------- cmd/clear_test.go | 96 +---------------- cmd/ps.go | 35 ++---- cmd/stop_identity_fallback_test.go | 46 ++++---- cmd/stop_identity_test.go | 2 +- docs/cli-reference.md | 17 +-- docs/commands.md | 42 ++++---- docs/orchestration.md | 17 +++ go.mod | 2 +- go.sum | 2 + pkg/processgroup/boot_identity_test.go | 2 +- pkg/processgroup/orphans.go | 139 ++++++++---------------- pkg/processgroup/orphans_test.go | 52 +++++---- pkg/processgroup/reaper.go | 77 ++++++-------- pkg/processgroup/reaper_test.go | 104 +++++++++++++----- pkg/processgroup/registered.go | 106 ++++++++++++++++++ pkg/processgroup/registered_test.go | 94 ++++++++++++++++ pkg/processgroup/scope.go | 13 +-- pkg/processgroup/scope_test.go | 17 +-- 19 files changed, 495 insertions(+), 510 deletions(-) create mode 100644 pkg/processgroup/registered.go create mode 100644 pkg/processgroup/registered_test.go diff --git a/cmd/clear.go b/cmd/clear.go index d7a63a77..ea792815 100644 --- a/cmd/clear.go +++ b/cmd/clear.go @@ -5,12 +5,7 @@ import ( "errors" "fmt" "os" - "os/exec" - "path/filepath" - "strconv" "strings" - "syscall" - "time" "github.com/codefly-dev/cli/cmd/common" "github.com/codefly-dev/cli/pkg/processgroup" @@ -21,85 +16,6 @@ import ( "github.com/spf13/cobra" ) -// codeflyOwnedPIDs returns the PIDs of running codefly-owned processes: the -// `codefly` CLI binary itself and any agent binary under ~/.codefly/agents/. -// It matches on the EXECUTABLE path, never on a substring of the full command -// line, so unrelated processes that merely mention the repo path are spared. -// The caller's own PID (self) is always excluded. -func codeflyOwnedPIDs(ctx context.Context, self int, scope processgroup.Scope) ([]int, error) { - // `command=` prints the full argv; the first token is the executable. - out, err := exec.CommandContext(ctx, "ps", "-axo", "pid=,command=").Output() - if err != nil { - return nil, fmt.Errorf("list processes: %w", err) - } - pids := parseCodeflyOwnedPIDs(out, self) - if scope.All() { - return pids, nil - } - // A scoped kill only signals codefly processes belonging to this workspace's - // run. An agent or CLI whose workspace cannot be resolved belongs to nobody - // we can name, so a scoped kill leaves it alone. Attribution follows the run - // that launched the process, not its working directory: an agent serving a - // composed module runs from that module's checkout, outside the workspace - // that composed it. - kept := make([]int, 0, len(pids)) - for _, pid := range pids { - workspace, ok := processgroup.WorkspaceOfProcess(pid) - if ok && scope.Includes(workspace) { - kept = append(kept, pid) - } - } - return kept, nil -} - -func parseCodeflyOwnedPIDs(out []byte, self int) []int { - var pids []int - for _, line := range strings.Split(strings.TrimSpace(string(out)), "\n") { - fields := strings.Fields(line) - if len(fields) < 2 { - continue - } - pid, convErr := strconv.Atoi(fields[0]) - if convErr != nil || pid == self { - continue - } - exePath := fields[1] - owned := filepath.Base(exePath) == "codefly" || strings.Contains(exePath, "/.codefly/agents/") - if owned { - pids = append(pids, pid) - } - } - return pids -} - -// killedProcessSettle bounds how long clear waits for the codefly processes it -// SIGKILLed to actually exit before it sweeps for orphaned native services. Agents -// die promptly under SIGKILL; the cap only guards against a wedged process holding -// up the command indefinitely. -const killedProcessSettle = 3 * time.Second - -// waitProcessesExited blocks until every pid has been fully reaped (kill(0) fails -// with ESRCH) or the deadline passes, whichever comes first. A pid we cannot signal -// (still present) counts as alive; only a genuinely gone process lets the loop -// advance. The killed agents share this process's user, so a signal never fails -// with EPERM here. -func waitProcessesExited(ctx context.Context, pids []int, timeout time.Duration) { - deadline := time.Now().Add(timeout) - for { - anyAlive := false - for _, pid := range pids { - if syscall.Kill(pid, 0) == nil { - anyAlive = true - break - } - } - if !anyAlive || ctx.Err() != nil || !time.Now().Before(deadline) { - return - } - time.Sleep(20 * time.Millisecond) - } -} - var ( clearKeepProcesses bool clearKeepContainers bool @@ -215,8 +131,6 @@ func clearCommand(ctx context.Context, args []string, options clearOptions) (ret if options.keepProcesses { w.Info("keeping processes (--keep-processes)") - } else { - failures = append(failures, killCodeflyProcesses(ctx, w, options)...) } managedFailures := sweepStaleState(ctx, w, options) @@ -269,60 +183,18 @@ func clearCommand(ctx context.Context, args []string, options clearOptions) (ret return errors.Join(failures...) } -// killCodeflyProcesses kills every codefly-owned process in scope, or lists -// them on a dry run, and waits for the killed ones to exit. -func killCodeflyProcesses(ctx context.Context, w *wool.Wool, options clearOptions) []error { - var failures []error - // Match codefly-owned processes by their EXECUTABLE, not by a substring - // over the whole command line. The old `ps aux | grep codefly.dev` - // matched any process whose argv merely mentioned the repo path — an - // editor, a `tail -f`, a build — and kill -9'd it. Agent binaries always - // live under ~/.codefly/agents/, and the CLI itself is the `codefly` - // executable; nothing else qualifies. - self := os.Getpid() - pids, err := codeflyOwnedPIDs(ctx, self, options.scope) - if err != nil { - w.Warn("cannot enumerate codefly processes", wool.ErrField(err)) - failures = append(failures, err) - } - switch { - case options.dryRun: - w.Info("would kill codefly processes", wool.Field("count", len(pids)), wool.Field("pids", pids)) - case len(pids) == 0: - w.Info("no codefly processes running") - default: - killed := 0 - for _, pid := range pids { - p, findErr := os.FindProcess(pid) - if findErr != nil { - failures = append(failures, fmt.Errorf("find process %d: %w", pid, findErr)) - continue - } - if killErr := p.Kill(); killErr != nil { - failures = append(failures, fmt.Errorf("kill process %d: %w", pid, killErr)) - continue - } - killed++ - } - w.Info("killed codefly processes", wool.Field("killed", killed), wool.Field("total", len(pids))) - // SIGKILL is asynchronous: the agents we just signalled are not yet - // reaped, so the native binaries they supervised have not yet - // reparented to init. The orphan sweeps below decide a group is - // reapable only once its supervisor is gone (groupIsStale), so racing - // them against a still-dying agent would leave the just-orphaned - // binary looking supervised and skip it. Wait for the agents to exit - // before sweeping so a `clear` that killed a live run still reaps the - // natives it just orphaned. - waitProcessesExited(ctx, pids, killedProcessSettle) - } - return failures -} - // sweepStaleState removes the state crashed or exited CLIs left behind — // per-spawn UDS sockets under /tmp/codefly-uds and process-group tracking // files under ~/.codefly/runs — or counts it on a dry run. func sweepStaleState(ctx context.Context, w *wool.Wool, options clearOptions) []error { if options.dryRun { + if !options.keepProcesses { + groups, err := processgroup.PreviewManagedProcessGroups(ctx, options.scope) + if err != nil { + return []error{err} + } + w.Info("would stop registered process groups", wool.Field("count", len(groups)), wool.Field("pgids", groups)) + } if n := manager.CountStaleAgentSockets(); n > 0 { w.Info("would remove stale agent sockets", wool.Field("count", n)) } else { diff --git a/cmd/clear_test.go b/cmd/clear_test.go index 8e7afb96..1266c12f 100644 --- a/cmd/clear_test.go +++ b/cmd/clear_test.go @@ -1,14 +1,6 @@ package cmd -import ( - "context" - "os/exec" - "reflect" - "testing" - "time" - - "github.com/codefly-dev/cli/pkg/processgroup" -) +import "testing" func TestClearAndStopCommandsReturnErrors(t *testing.T) { for name, command := range map[string]struct { @@ -39,89 +31,3 @@ func TestStopOptionsDoNotInheritClearFlags(t *testing.T) { t.Fatalf("stop options inherited clear state: %+v", got) } } - -func TestParseCodeflyOwnedPIDs(t *testing.T) { - out := []byte(` 10 /usr/local/bin/codefly run service api - 11 /Users/me/.codefly/agents/go/bin/agent serve - 12 /usr/bin/editor /repo/codefly.dev/file.go - 13 /usr/local/bin/codefly stop -bad /usr/local/bin/codefly -`) - if got, want := parseCodeflyOwnedPIDs(out, 13), []int{10, 11}; !reflect.DeepEqual(got, want) { - t.Fatalf("owned PIDs = %v, want %v", got, want) - } -} - -func TestCodeflyOwnedPIDsReturnsProcessErrors(t *testing.T) { - ctx, cancel := context.WithCancel(context.Background()) - cancel() - if _, err := codeflyOwnedPIDs(ctx, -1, processgroup.AllWorkspaces()); err == nil { - t.Fatal("expected cancelled process enumeration to return an error") - } -} - -// TestWaitProcessesExitedBlocksUntilGone guards the reparent race: clear must not -// sweep for orphaned natives until the agents it SIGKILLed have actually exited. -// A no-op wait (the original bug) would return immediately while the process is -// still alive, so the first half asserts the wait genuinely blocks; the second -// asserts it returns promptly once the process is gone. -func TestWaitProcessesExitedBlocksUntilGone(t *testing.T) { - cmd := exec.Command("sleep", "30") - if err := cmd.Start(); err != nil { - t.Fatal(err) - } - pid := cmd.Process.Pid - reaped := make(chan struct{}) - go func() { - _, _ = cmd.Process.Wait() - close(reaped) - }() - t.Cleanup(func() { - _ = cmd.Process.Kill() - <-reaped - }) - - start := time.Now() - waitProcessesExited(context.Background(), []int{pid}, 300*time.Millisecond) - if elapsed := time.Since(start); elapsed < 250*time.Millisecond { - t.Fatalf("waitProcessesExited returned after %s while the process was still alive; expected to wait to the timeout", elapsed) - } - - if err := cmd.Process.Kill(); err != nil { - t.Fatal(err) - } - <-reaped - - start = time.Now() - waitProcessesExited(context.Background(), []int{pid}, 5*time.Second) - if elapsed := time.Since(start); elapsed > time.Second { - t.Fatalf("waitProcessesExited blocked %s after the process exited; expected a prompt return", elapsed) - } -} - -// TestWaitProcessesExitedHonorsCancelledContext ensures a wedged process cannot -// hang clear once the command's context is cancelled (e.g. a second Ctrl-C). -func TestWaitProcessesExitedHonorsCancelledContext(t *testing.T) { - cmd := exec.Command("sleep", "30") - if err := cmd.Start(); err != nil { - t.Fatal(err) - } - pid := cmd.Process.Pid - reaped := make(chan struct{}) - go func() { - _, _ = cmd.Process.Wait() - close(reaped) - }() - t.Cleanup(func() { - _ = cmd.Process.Kill() - <-reaped - }) - - ctx, cancel := context.WithCancel(context.Background()) - cancel() - start := time.Now() - waitProcessesExited(ctx, []int{pid}, time.Minute) - if elapsed := time.Since(start); elapsed > time.Second { - t.Fatalf("waitProcessesExited ignored the cancelled context and blocked %s", elapsed) - } -} diff --git a/cmd/ps.go b/cmd/ps.go index 9b3d2b5c..4478bf35 100644 --- a/cmd/ps.go +++ b/cmd/ps.go @@ -39,37 +39,24 @@ type psProcess struct { Started time.Time `json:"started"` } -// PsCmd lists what a codefly run is still holding: its frontend dev servers and -// its native-mode service processes — the compiled service binaries and the -// stateful stores it started. -// -// It lists a run COMPLETELY, which it did not. Two gaps, one cause: it scanned -// only dev servers, so a run's compiled services and stores never appeared at -// all; and a dev server was only found when its working directory sat inside a -// workspace, which is never true of a composed module's frontend — that runs -// from the module's own checkout, outside the workspace that composed it. A -// composed run therefore showed a fraction of itself, and `stop` then left -// exactly the processes `ps` could not name still holding their ports. -// -// It is scoped to the current workspace, so it answers "what is my run still -// holding" — the same set `codefly stop` acts on. `--all` widens it to every -// workspace on the machine, which is how to find a leak belonging to a checkout -// you are not standing in. +// PsCmd lists signature-matched frontend and native processes. It is a +// diagnostic view, not the complete registered population used by stop. var PsCmd = &cobra.Command{ Use: "ps", Short: "List the processes this workspace's run is still holding", Long: `List the processes a codefly run is keeping alive: frontend dev servers (next dev / npm run dev / vite) and native-mode services (compiled service -binaries and the stateful stores a run started), including those belonging to -composed modules checked out outside the workspace. +binaries and PostgreSQL), including composed modules when their launch +environment or working directory identifies the workspace. This signature scan +can omit registered processes whose command or environment has changed; stop +uses authenticated registry discovery independently. -Scoped to the current workspace — the same set 'codefly stop' acts on. Use ---all for every workspace on this machine. Outside a workspace the listing is -machine-wide, since there is nothing to scope to. +Scoped to the current workspace. Use --all for every workspace on this machine. +Outside a workspace the listing is machine-wide. -STATUS is one of: orphaned (codefly's, escaped its supervisor — reaped by -'codefly clear'), tracked (codefly's, still supervised), or external (not -codefly's — shown for visibility, never reaped).`, +STATUS is diagnostic: orphaned (marked process without its supervisor), tracked +(marked process still supervised), or external (no marker). A marker or status +never authorizes cleanup: clear must authenticate a registered identity.`, Args: cobra.NoArgs, RunE: func(cmd *cobra.Command, _ []string) error { ctx, done := common.NewContext() diff --git a/cmd/stop_identity_fallback_test.go b/cmd/stop_identity_fallback_test.go index 4feba4a5..23855942 100644 --- a/cmd/stop_identity_fallback_test.go +++ b/cmd/stop_identity_fallback_test.go @@ -35,12 +35,19 @@ func TestStopIdentityFallbackOwner(t *testing.T) { t.Fatal(err) } } -func TestStopDoesNotFallbackAfterBootIdentityRefusal(t *testing.T) { +func TestStopDoesNotFallbackAfterBootIdentityRefusal(t *testing.T) { testStopIdentityRefusal(t, false) } +func TestStopDoesNotKillAgentBeforeBootIdentityRefusal(t *testing.T) { + testStopIdentityRefusal(t, true) +} +func testStopIdentityRefusal(t *testing.T, agentShape bool) { t.Setenv("HOME", t.TempDir()) workspace := psWorkspace(t) t.Chdir(workspace) t.Setenv(processgroup.LaunchWorkspaceEnv, workspace) binary := filepath.Join(t.TempDir(), ".cache", "native", "stop-child") + if agentShape { + binary = filepath.Join(t.TempDir(), ".codefly", "agents", "stop-child") + } if err := os.MkdirAll(filepath.Dir(binary), 0700); err != nil { t.Fatal(err) } @@ -67,7 +74,7 @@ func TestStopDoesNotFallbackAfterBootIdentityRefusal(t *testing.T) { t.Fatal(err) } t.Cleanup(func() { _ = syscall.Kill(pid, syscall.SIGKILL) }) - path := filepath.Join(os.Getenv("HOME"), ".codefly", "runs", "authenticated-v1", fmt.Sprintf("%d.pgid", pid)) + path := filepath.Join(os.Getenv("HOME"), ".codefly", "runs", "authenticated-v2", fmt.Sprintf("%d.pgid", pid)) data, err := os.ReadFile(path) if err != nil { t.Fatal(err) @@ -86,24 +93,27 @@ func TestStopDoesNotFallbackAfterBootIdentityRefusal(t *testing.T) { if err = os.WriteFile(path, data, 0600); err != nil { t.Fatal(err) } - deadline := time.Now().Add(5 * time.Second) - for { - rows, scanErr := scanRunProcesses(context.Background(), processgroup.InWorkspace(workspace)) - if scanErr != nil { - t.Fatal(scanErr) - } - found := false - for _, row := range rows { - found = found || (row.PID == pid && row.Kind == "native" && row.Status == "orphaned") + if !agentShape { + deadline := time.Now().Add(5 * time.Second) + for { + rows, scanErr := scanRunProcesses(context.Background(), processgroup.InWorkspace(workspace)) + if scanErr != nil { + t.Fatal(scanErr) + } + found := false + for _, row := range rows { + found = found || (row.PID == pid && row.Kind == "native" && row.Status == "orphaned") + } + if found { + break + } + if time.Now().After(deadline) { + t.Fatal("child never became eligible for orphan fallback") + } + time.Sleep(10 * time.Millisecond) } - if found { - break - } - if time.Now().After(deadline) { - t.Fatal("child never became eligible for orphan fallback") - } - time.Sleep(10 * time.Millisecond) } + err = clearCommand(context.Background(), nil, stopClearOptions()) if err == nil || !strings.Contains(err.Error(), "incompatible boot identity") { t.Fatalf("expected boot refusal: %v", err) diff --git a/cmd/stop_identity_test.go b/cmd/stop_identity_test.go index eece708e..bedbfa67 100644 --- a/cmd/stop_identity_test.go +++ b/cmd/stop_identity_test.go @@ -41,7 +41,7 @@ func TestStopReportsIncompatibleBootIdentityWithoutDiscardingEvidence(t *testing } time.Sleep(time.Millisecond) } - path := filepath.Join(home, ".codefly", "runs", "authenticated-v1", fmt.Sprintf("%d.pgid", group.PGID())) + path := filepath.Join(home, ".codefly", "runs", "authenticated-v2", fmt.Sprintf("%d.pgid", group.PGID())) data, err := os.ReadFile(path) if err != nil { t.Fatal(err) diff --git a/docs/cli-reference.md b/docs/cli-reference.md index a87a9886..8ab32321 100644 --- a/docs/cli-reference.md +++ b/docs/cli-reference.md @@ -3832,16 +3832,17 @@ List the processes this workspace's run is still holding ``` List the processes a codefly run is keeping alive: frontend dev servers (next dev / npm run dev / vite) and native-mode services (compiled service -binaries and the stateful stores a run started), including those belonging to -composed modules checked out outside the workspace. +binaries and PostgreSQL), including composed modules when their launch +environment or working directory identifies the workspace. This signature scan +can omit registered processes whose command or environment has changed; stop +uses authenticated registry discovery independently. -Scoped to the current workspace — the same set 'codefly stop' acts on. Use ---all for every workspace on this machine. Outside a workspace the listing is -machine-wide, since there is nothing to scope to. +Scoped to the current workspace. Use --all for every workspace on this machine. +Outside a workspace the listing is machine-wide. -STATUS is one of: orphaned (codefly's, escaped its supervisor — reaped by -'codefly clear'), tracked (codefly's, still supervised), or external (not -codefly's — shown for visibility, never reaped). +STATUS is diagnostic: orphaned (marked process without its supervisor), tracked +(marked process still supervised), or external (no marker). A marker or status +never authorizes cleanup: clear must authenticate a registered identity. ``` ``` diff --git a/docs/commands.md b/docs/commands.md index e3cf95c1..e196cd51 100644 --- a/docs/commands.md +++ b/docs/commands.md @@ -4353,33 +4353,27 @@ Replay recorded operations. ### `codefly clear` -Clear cached state and temporary files. Also reaps leaked frontend dev servers -(`next dev` / `npm run dev` / `vite`) that codefly spawned — proven by their -process-group authentication — and that lost their supervisor: the leaks -`codefly ps` reports as `orphaned`. Dev servers the user launched by hand (shown -as `external`) are never touched. +Clear cached state and temporary files. Frontend/native signature scans identify +additional cleanup candidates. Recovery requires Core's authenticated process-group +registration and owner-death evidence; a name or environment marker alone never +permits signaling. Missing or ambiguous proof leaves the process running and +returns an error. Registry failures prevent fallback cleanup from bypassing refusal. ### `codefly ps` -List the processes this workspace's run is still holding: frontend dev servers -(`next dev` / `npm run dev` / `vite`) and native-mode services — the compiled -service binaries and the stateful stores a run started. STATUS is `orphaned` -(codefly's, escaped its supervisor — reaped by `codefly clear`), `tracked` -(codefly's, still supervised), or `external` (not codefly's — shown for -visibility, never reaped). Add `--json` for machine-readable output. - -A process belongs to the run that launched it, not to the directory it runs in. -That is the difference that makes a composed run listable at all: a composed -module is checked out outside the workspace that composes it, so its services -run from that checkout, and a store runs from a data directory under -`~/.codefly/data` that is in no workspace at all. The run is recorded in the -environment of everything a `codefly` invocation starts, so `ps` lists a -composed run whole — which matters because it is the set `codefly stop` acts on. - -Scoped to the current workspace. `--all` lists every workspace's processes on -this machine, which is how to find a leak belonging to a checkout you are not -standing in; outside a workspace the listing is machine-wide, since there is -nothing to scope to. +List signature-matched frontend dev servers (`next dev` / `npm run dev` / `vite`) +and native service binaries or PostgreSQL. Add `--json` for machine-readable +output. STATUS is diagnostic: `orphaned` means a marked process lost its +supervisor, `tracked` means it remains supervised, and `external` means no marker +was observed. These labels do not establish permission to signal a process. + +The view uses launch environment and working directory for workspace attribution. +It can omit registered children that rewrite their command/environment, including +Redis. `codefly stop` independently enumerates Core registrations and selects +persisted authenticated launch provenance; its coverage is broader than this view. + +Scoped to the current workspace; `--all` widens the diagnostic listing. Outside a +workspace the listing is machine-wide. --- diff --git a/docs/orchestration.md b/docs/orchestration.md index af09773b..3241b99f 100644 --- a/docs/orchestration.md +++ b/docs/orchestration.md @@ -1129,6 +1129,23 @@ HTTP readiness deliberately stops at TCP reachability. Probing `/` would invent health contract and can itself keep a development server unready. ## Teardown +`codefly stop` discovers supported process-group registrations through Core, +which authenticates their recorded identities and owns signaling and removal. +The command does not kill CLI/agent processes by executable name or path. +The CLI selects the current workspace using the authenticated launch workspace +persisted at spawn. This survives an exited controller and children that rewrite +their environment. Independently built agents must adopt this Core producer; +older registrations without that metadata retain environment/cwd attribution. +Historical root records remain a separate compatibility path. A root record cannot override +a surviving supported registration's refusal or workspace selection. + +Signature scans can find additional frontend/native candidates, but a process +name or environment marker grants no signal authority. Missing or unproven +registered ownership leaves the process running and reports an error. Registry +inspection failure also prevents the legacy and signature fallback paths from +overriding the refusal. An older CLI that cannot read the new namespace cannot +recover its groups; upgrade the cleanup consumer along with the producer. + `codefly stop` retains an authenticated process-group record and returns an error when a live leader has the recorded start identity but a different boot identity. Older Darwin agents wrote a boot timestamp where current agents write diff --git a/go.mod b/go.mod index b952df94..a72047cc 100644 --- a/go.mod +++ b/go.mod @@ -16,7 +16,7 @@ require ( github.com/codefly-dev/authority/libraries/delivery/go v0.0.0-20261026023425-000d44495d93 github.com/codefly-dev/authority/libraries/fixture-composition/go v0.0.0-20261027171336-5178f01696e3 github.com/codefly-dev/authority/libraries/hop/go v0.0.0-20261026175541-3afad7224855 - github.com/codefly-dev/core v0.19.8-0.20261027204840-3d31544addf5 + github.com/codefly-dev/core v0.19.8-0.20261027211051-bc4e15cab5b0 github.com/codefly-dev/golor v0.1.3 github.com/codefly-dev/llm v0.1.7 github.com/codefly-dev/sdk-go/workcontext v0.1.1-0.20261018052022-70dc7a546458 diff --git a/go.sum b/go.sum index a34d99e1..fe301896 100644 --- a/go.sum +++ b/go.sum @@ -123,6 +123,8 @@ github.com/codefly-dev/core v0.19.8-0.20261027204224-b14f7188c548 h1:5wqbjiooPNt github.com/codefly-dev/core v0.19.8-0.20261027204224-b14f7188c548/go.mod h1:NB7x1UZ01rYTfEZEl4vUFD1A+fgOXhsWbgA3M8x4ThA= github.com/codefly-dev/core v0.19.8-0.20261027204840-3d31544addf5 h1:08ZV27FM2N4V6NtGrp8T5k+Fc73Ten7BCX6FRkFIba4= github.com/codefly-dev/core v0.19.8-0.20261027204840-3d31544addf5/go.mod h1:NB7x1UZ01rYTfEZEl4vUFD1A+fgOXhsWbgA3M8x4ThA= +github.com/codefly-dev/core v0.19.8-0.20261027211051-bc4e15cab5b0 h1:r5xxRusbZ54Wpk2T6aqqqNWvHmVVJ8HMJNdofE7mqrc= +github.com/codefly-dev/core v0.19.8-0.20261027211051-bc4e15cab5b0/go.mod h1:NB7x1UZ01rYTfEZEl4vUFD1A+fgOXhsWbgA3M8x4ThA= github.com/codefly-dev/golor v0.1.3 h1:xmo+ceyJFRYZdvpWE2fNd0jeaadp/Ibm1BnganiGKOc= github.com/codefly-dev/golor v0.1.3/go.mod h1:sl/u/K1l7J0Pr3xyVZp8fOJYQItKKst1No9JqgzLLoY= github.com/codefly-dev/gortk v0.2.0 h1:7bOlS5valYz2zil+fZctQNcPCYBcPj86abcw9N8h1hQ= diff --git a/pkg/processgroup/boot_identity_test.go b/pkg/processgroup/boot_identity_test.go index 2860867f..9b2c901a 100644 --- a/pkg/processgroup/boot_identity_test.go +++ b/pkg/processgroup/boot_identity_test.go @@ -38,7 +38,7 @@ func TestOldDarwinBootIdentityRetainsLiveRecord(t *testing.T) { waitForFile(t, readyPath) pid := readPID(t, pidPath) t.Cleanup(func() { _ = syscall.Kill(-pid, syscall.SIGKILL) }) - path := filepath.Join(home, ".codefly", stateDirName, authenticatedDirName, fmt.Sprintf("%d.pgid", pid)) + path := filepath.Join(home, ".codefly", stateDirName, "authenticated-v2", fmt.Sprintf("%d.pgid", pid)) original, err := os.ReadFile(path) if err != nil { t.Fatal(err) diff --git a/pkg/processgroup/orphans.go b/pkg/processgroup/orphans.go index c0367e66..8ca4165a 100644 --- a/pkg/processgroup/orphans.go +++ b/pkg/processgroup/orphans.go @@ -11,6 +11,7 @@ import ( "time" "github.com/codefly-dev/core/resources" + runners "github.com/codefly-dev/core/runners/base" "github.com/shirou/gopsutil/v3/process" ) @@ -49,9 +50,8 @@ type DevServerOrphan struct { // the registry-based reaper can no longer identify. Orphaned bool // Owned is true when the process carries codefly's process-group - // authentication in its environment, proving codefly spawned it. Only owned - // groups are ever signalled; a dev server the user launched by hand inside a - // workspace is surfaced by a scan but never reaped. + // marker in its environment. This is a discovery hint; the reaper separately + // requires Core to authenticate its registered identity before any signal. Owned bool } @@ -148,13 +148,10 @@ func snapshotProcess(ctx context.Context, pid int, proc *process.Process) (proce return snap, true } -// ReapDevServerOrphans terminates the process groups of leaked dev servers. A -// group is reaped only when it is (1) codefly-owned — a member carries codefly's -// process-group authentication, so codefly spawned it — and (2) stale — its -// group leader is gone or has itself been reparented to init. Dev servers the -// user launched by hand (no authentication) and servers still supervised by a -// live codefly (leader with a live parent) are left untouched. With dryRun set, -// nothing is signalled and the return value is what would be reaped. +// ReapDevServerOrphans inspects signature-matched candidates in scope. Core +// must prove registered ownership and owner death before an identity-bound +// signal. A missing registration is a refusal, even with a marker present. +// Dry runs perform the same admission checks without signaling. func ReapDevServerOrphans(ctx context.Context, dryRun bool, scope Scope) ([]DevServerOrphan, error) { orphans, err := ScanDevServerOrphans(ctx) if err != nil { @@ -172,19 +169,20 @@ func ReapDevServerOrphans(ctx context.Context, dryRun bool, scope Scope) ([]DevS } stale, err := groupIsStale(orphan.PGID) if err != nil { - failures = append(failures, fmt.Errorf("inspect dev-server process group %d (%s): %w", orphan.PGID, orphan.Cwd, err)) + failures = append(failures, err) continue } if !stale { continue } reapedGroups[orphan.PGID] = struct{}{} - reaped = append(reaped, orphan) - if dryRun { + selected, err := reapOrphanGroup(ctx, orphan.PGID, scope, dryRun) + if err != nil { + failures = append(failures, fmt.Errorf("reap dev-server process group %d (%s): %w", orphan.PGID, orphan.Cwd, err)) continue } - if err := killAuthenticatedProcessGroup(ctx, orphan.PGID); err != nil { - failures = append(failures, fmt.Errorf("reap dev-server process group %d (%s): %w", orphan.PGID, orphan.Cwd, err)) + if selected { + reaped = append(reaped, orphan) } } return reaped, errors.Join(failures...) @@ -202,7 +200,7 @@ func ReapDevServerOrphans(ctx context.Context, dryRun bool, scope Scope) ([]DevS // // Unlike a dev server, a native service is not required to sit inside a workspace // directory — a postgres data dir lives under ~/.codefly/data, outside the repo — -// so ownership rests entirely on the process-group authentication. +// so recovery requires authenticated registered ownership and workspace scope. type NativeServiceOrphan struct { PID int PGID int @@ -226,8 +224,7 @@ type NativeServiceOrphan struct { // ScanNativeServiceOrphans finds native-mode service processes (a compiled user // binary under a codefly build cache, or a postgres cluster) running anywhere on // the machine, without relying on the process-group registry. Ownership is decided -// by the process-group authentication alone, so a user's own or system postgres is -// surfaced with Owned=false and never reaped. +// by its marker for discovery only; signals require Core registration proof. func ScanNativeServiceOrphans(ctx context.Context) ([]NativeServiceOrphan, error) { pids, err := process.PidsWithContext(ctx) if err != nil { @@ -279,11 +276,9 @@ func ScanNativeServiceOrphans(ctx context.Context) ([]NativeServiceOrphan, error return orphans, nil } -// ReapNativeServiceOrphans terminates the process groups of leaked native-mode -// services. As with dev servers, a group is reaped only when it is (1) codefly-owned -// — a member carries the process-group authentication — and (2) stale — its leader -// is gone or reparented to init. With dryRun set, nothing is signalled and the -// return value is what would be reaped. +// ReapNativeServiceOrphans applies the same registered-identity and scope +// checks as dev-server recovery to native signature candidates. Missing or +// ambiguous registrations return an error and leave the process running. func ReapNativeServiceOrphans(ctx context.Context, dryRun bool, scope Scope) ([]NativeServiceOrphan, error) { orphans, err := ScanNativeServiceOrphans(ctx) if err != nil { @@ -301,19 +296,20 @@ func ReapNativeServiceOrphans(ctx context.Context, dryRun bool, scope Scope) ([] } stale, err := groupIsStale(orphan.PGID) if err != nil { - failures = append(failures, fmt.Errorf("inspect native-service process group %d (%s): %w", orphan.PGID, orphan.Command, err)) + failures = append(failures, err) continue } if !stale { continue } reapedGroups[orphan.PGID] = struct{}{} - reaped = append(reaped, orphan) - if dryRun { + selected, err := reapOrphanGroup(ctx, orphan.PGID, scope, dryRun) + if err != nil { + failures = append(failures, fmt.Errorf("reap native-service process group %d (%s): %w", orphan.PGID, orphan.Command, err)) continue } - if err := killAuthenticatedProcessGroup(ctx, orphan.PGID); err != nil { - failures = append(failures, fmt.Errorf("reap native-service process group %d (%s): %w", orphan.PGID, orphan.Command, err)) + if selected { + reaped = append(reaped, orphan) } } return reaped, errors.Join(failures...) @@ -384,9 +380,8 @@ func isRegularFile(path string) bool { } // processIsCodeflyOwned reports whether pid carries codefly's process-group -// authentication in its environment — proof that codefly spawned it (and, via -// inheritance, its whole tree). This survives the loss of the registry record, -// so it identifies leaked codefly servers that `clear` may safely reap. +// marker in its environment. This identifies candidates for inspection, not +// authority to signal: recovery requires Core to authenticate a registration. func processIsCodeflyOwned(pid int) (bool, error) { authentication, err := readProcessGroupAuthentication(pid) if errors.Is(err, errProcessNotFound) { @@ -416,77 +411,31 @@ func groupIsStale(pgid int) (bool, error) { return true, nil } -// groupHasCodeflyMember reports whether the live process group still contains a -// codefly-owned member. It is re-checked immediately before each destructive -// signal so that a process group whose number was recycled between the scan and -// the kill is never signalled unless it is, right now, codefly's. -func groupHasCodeflyMember(ctx context.Context, pgid int) (bool, error) { - members, err := inspectProcessGroup(ctx, pgid) +// reapOrphanGroup treats a signature and environment marker as discovery only. +// Core must prove the registered birth, owner death and scope before any signal. +func reapOrphanGroup(ctx context.Context, pgid int, scope Scope, dryRun bool) (bool, error) { + group, err := runners.LookupProcessGroup(pgid) if err != nil { - return false, err - } - for _, member := range members { - owned, err := processIsCodeflyOwned(member.pid) - if err != nil { - return false, err - } - if owned { - return true, nil - } - } - return false, nil -} - -// killAuthenticatedProcessGroup escalates SIGTERM then SIGKILL to the group, -// but re-confirms the group still holds a codefly-owned member immediately -// before each signal. That closes the window between scan and kill in which the -// process-group number could have been recycled by an unrelated group: if the -// group is no longer codefly's (or already gone), it aborts without signalling. -func killAuthenticatedProcessGroup(ctx context.Context, pgid int) error { - if pgid <= 1 { - return fmt.Errorf("refusing to signal process group %d", pgid) - } - if pgid == syscall.Getpgrp() { - return errors.New("refusing to signal own process group") + return false, fmt.Errorf("cannot authenticate orphan; process left running: %w", err) } - owned, err := groupHasCodeflyMember(ctx, pgid) + ownership, err := group.InspectOwnership(ctx) if err != nil { - return err - } - if !owned { - return nil - } - if err = signalGroup(pgid, syscall.SIGTERM); err != nil { - return err - } - if waitForGroupDeath(ctx, pgid, sigtermGrace) { - return nil - } - if err = ctx.Err(); err != nil { - return err - } - owned, err = groupHasCodeflyMember(ctx, pgid) - if err != nil { - return err + return false, err } - if !owned { - return nil + if !ownership.Authenticated { + return false, errors.New("orphan ownership is unproven; process left running") } - if err = signalGroup(pgid, syscall.SIGKILL); err != nil { - return err + if ownership.OwnerAlive || !registeredGroupInScope(group, ownership, scope) { + return false, nil } - if waitForGroupDeath(ctx, pgid, sigkillGrace) { - return nil + if dryRun { + return true, nil } - if err = ctx.Err(); err != nil { - return err + if err := group.Terminate(ctx); err != nil { + return false, err } - return fmt.Errorf("process group %d remained alive after SIGKILL", pgid) -} - -func signalGroup(pgid int, signal syscall.Signal) error { - if err := syscall.Kill(-pgid, signal); err != nil && !errors.Is(err, syscall.ESRCH) { - return fmt.Errorf("signal process group %d: %w", pgid, err) + if err := group.RemoveIfDeadContext(ctx); err != nil { + return false, err } - return nil + return true, nil } diff --git a/pkg/processgroup/orphans_test.go b/pkg/processgroup/orphans_test.go index 0f5c1a81..4203466f 100644 --- a/pkg/processgroup/orphans_test.go +++ b/pkg/processgroup/orphans_test.go @@ -254,6 +254,7 @@ func TestScanDevServerOrphansMarksUnauthenticatedExternal(t *testing.T) { } func TestReapDevServerOrphansDryRunSkipsUnownedAndSupervised(t *testing.T) { + t.Setenv("HOME", t.TempDir()) workspace := tempWorkspace(t) external := startDevServerHelper(t, workspace, "", 0) // no auth => not codefly's supervised := startDevServerHelper(t, workspace, testGroupAuth, 0) // owned, but leader has a live parent @@ -262,7 +263,7 @@ func TestReapDevServerOrphansDryRunSkipsUnownedAndSupervised(t *testing.T) { ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second) defer cancel() - reaped, err := ReapDevServerOrphans(ctx, true, AllWorkspaces()) + reaped, err := ReapDevServerOrphans(ctx, true, InWorkspace(workspace)) if err != nil { t.Fatal(err) } @@ -276,7 +277,8 @@ func TestReapDevServerOrphansDryRunSkipsUnownedAndSupervised(t *testing.T) { } } -func TestReapDevServerOrphansDryRunSelectsOwnedStaleGroup(t *testing.T) { +func TestReapDevServerOrphansDryRunRefusesUnregisteredStaleGroup(t *testing.T) { + t.Setenv("HOME", t.TempDir()) workspace := tempWorkspace(t) leader := startDevServerHelper(t, workspace, testGroupAuth, 0) leaderPID := leader.Process.Pid @@ -298,9 +300,9 @@ func TestReapDevServerOrphansDryRunSelectsOwnedStaleGroup(t *testing.T) { ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second) defer cancel() - reaped, err := ReapDevServerOrphans(ctx, true, AllWorkspaces()) - if err != nil { - t.Fatal(err) + reaped, err := ReapDevServerOrphans(ctx, true, InWorkspace(workspace)) + if err == nil { + t.Fatal("unregistered group was accepted") } selected := false for _, orphan := range reaped { @@ -308,12 +310,13 @@ func TestReapDevServerOrphansDryRunSelectsOwnedStaleGroup(t *testing.T) { selected = true } } - if !selected { - t.Fatalf("owned stale group %d was not selected for reaping", leaderPID) + if selected { + t.Fatalf("owned stale group %d was incorrectly selected for reaping", leaderPID) } } -func TestKillAuthenticatedProcessGroupReapsOwnedGroup(t *testing.T) { +func TestOrphanReaperRefusesMarkerWithoutRegistration(t *testing.T) { + t.Setenv("HOME", t.TempDir()) workspace := tempWorkspace(t) command := startDevServerHelper(t, workspace, testGroupAuth, 0) pid := command.Process.Pid @@ -322,15 +325,16 @@ func TestKillAuthenticatedProcessGroupReapsOwnedGroup(t *testing.T) { ctx, cancel := context.WithTimeout(context.Background(), 20*time.Second) defer cancel() - if err := killAuthenticatedProcessGroup(ctx, pid); err != nil { - t.Fatalf("killAuthenticatedProcessGroup(%d) = %v", pid, err) + if selected, err := reapOrphanGroup(ctx, pid, AllWorkspaces(), false); err == nil || selected { + t.Fatalf("unregistered marker accepted: selected=%v error=%v", selected, err) } - if isAlive(pid) { - t.Fatalf("owned dev-server group %d survived reaping", pid) + if !isAlive(pid) { + t.Fatalf("unregistered process %d was killed", pid) } } func TestKillAuthenticatedProcessGroupSparesUnownedGroup(t *testing.T) { + t.Setenv("HOME", t.TempDir()) workspace := tempWorkspace(t) command := startDevServerHelper(t, workspace, "", 0) // no auth => not codefly's pid := command.Process.Pid @@ -338,8 +342,8 @@ func TestKillAuthenticatedProcessGroupSparesUnownedGroup(t *testing.T) { ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second) defer cancel() - if err := killAuthenticatedProcessGroup(ctx, pid); err != nil { - t.Fatalf("killAuthenticatedProcessGroup(%d) = %v, want nil (unowned group left untouched)", pid, err) + if selected, err := reapOrphanGroup(ctx, pid, AllWorkspaces(), false); err == nil || selected { + t.Fatalf("unregistered process accepted: selected=%v error=%v", selected, err) } if !isAlive(pid) { t.Fatalf("unowned dev server %d was killed despite lacking codefly authentication", pid) @@ -426,6 +430,9 @@ func TestScanNativeServiceOrphansMarksUnauthenticatedExternal(t *testing.T) { } func TestReapNativeServiceOrphansDryRunSkipsUnownedAndSupervised(t *testing.T) { + t.Setenv("HOME", t.TempDir()) + workspace := tempWorkspace(t) + t.Setenv(LaunchWorkspaceEnv, workspace) external := startNativeServiceHelper(t, "postgres", "", 0) // no auth => not codefly's supervised := startNativeServiceHelper(t, nativeCacheBinary(t), testGroupAuth, 0) // owned, but leader has a live parent waitForScannedNativeOrphan(t, external.Process.Pid) @@ -433,7 +440,7 @@ func TestReapNativeServiceOrphansDryRunSkipsUnownedAndSupervised(t *testing.T) { ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second) defer cancel() - reaped, err := ReapNativeServiceOrphans(ctx, true, AllWorkspaces()) + reaped, err := ReapNativeServiceOrphans(ctx, true, InWorkspace(workspace)) if err != nil { t.Fatal(err) } @@ -447,7 +454,10 @@ func TestReapNativeServiceOrphansDryRunSkipsUnownedAndSupervised(t *testing.T) { } } -func TestReapNativeServiceOrphansDryRunSelectsOwnedStaleGroup(t *testing.T) { +func TestReapNativeServiceOrphansDryRunRefusesUnregisteredStaleGroup(t *testing.T) { + t.Setenv("HOME", t.TempDir()) + workspace := tempWorkspace(t) + t.Setenv(LaunchWorkspaceEnv, workspace) leader := startNativeServiceHelper(t, nativeCacheBinary(t), testGroupAuth, 0) leaderPID := leader.Process.Pid waitForGroupLeader(t, leaderPID) @@ -468,9 +478,9 @@ func TestReapNativeServiceOrphansDryRunSelectsOwnedStaleGroup(t *testing.T) { ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second) defer cancel() - reaped, err := ReapNativeServiceOrphans(ctx, true, AllWorkspaces()) - if err != nil { - t.Fatal(err) + reaped, err := ReapNativeServiceOrphans(ctx, true, InWorkspace(workspace)) + if err == nil { + t.Fatal("unregistered group was accepted") } selected := false for _, orphan := range reaped { @@ -478,7 +488,7 @@ func TestReapNativeServiceOrphansDryRunSelectsOwnedStaleGroup(t *testing.T) { selected = true } } - if !selected { - t.Fatalf("owned stale native-service group %d was not selected for reaping", leaderPID) + if selected { + t.Fatalf("owned stale native-service group %d was incorrectly selected for reaping", leaderPID) } } diff --git a/pkg/processgroup/reaper.go b/pkg/processgroup/reaper.go index 23f40323..ac143d65 100644 --- a/pkg/processgroup/reaper.go +++ b/pkg/processgroup/reaper.go @@ -15,6 +15,7 @@ import ( "syscall" "time" + runners "github.com/codefly-dev/core/runners/base" "github.com/codefly-dev/core/wool" "github.com/gofrs/flock" "github.com/shirou/gopsutil/v3/process" @@ -22,7 +23,6 @@ import ( const ( stateDirName = "runs" - authenticatedDirName = "authenticated-v1" registryLockName = ".reaper.lock" maxSweepPasses = 4 maxRecordSize = 16 << 10 @@ -127,6 +127,8 @@ const ( ) // GroupCleanup identifies the outcome retained for one registry record. +// Record is a logical registered-group: key for Core-owned registrations +// and a filesystem path for historical root records. type GroupCleanup struct { PGID int Record string @@ -245,13 +247,22 @@ func reconcileProcessGroupRegistries(ctx context.Context, policy reapPolicy) (Cl evidence.Outcome = CleanupFailed return evidence, err } - currentEvidence, currentErr := reapProcessGroups(ctx, filepath.Join(dir, authenticatedDirName), policy) - evidence.merge(currentEvidence) - legacyEvidence, legacyErr := reapProcessGroups(ctx, dir, policy) - evidence.merge(legacyEvidence) - if err := errors.Join(currentErr, legacyErr); err != nil { - evidence.Outcome = CleanupFailed - return evidence, err + // Reaping a parent may orphan a group already inspected earlier in the + // pass, including a child written by a different producer generation. + for range maxSweepPasses { + currentEvidence, currentErr := reapRegisteredProcessGroups(ctx, policy) + evidence.merge(currentEvidence) + if currentErr != nil { + return evidence, currentErr + } + legacyEvidence, legacyErr := reapProcessGroups(ctx, dir, policy) + evidence.merge(legacyEvidence) + if legacyErr != nil { + return evidence, legacyErr + } + if len(currentEvidence.RecoveredPGIDs()) == 0 && len(legacyEvidence.RecoveredPGIDs()) == 0 { + break + } } return evidence, nil } @@ -385,20 +396,7 @@ func reconcile(ctx context.Context, path string, policy reapPolicy) (string, err if errors.Is(err, os.ErrNotExist) { return dispositionRemoved, nil } - if filepath.Base(filepath.Dir(path)) == authenticatedDirName && snapshot.info != nil { - quarantinePath, quarantineErr := quarantineInvalidRecord(path, snapshot) - if quarantineErr != nil { - return dispositionFailed, errors.Join( - fmt.Errorf("read process-group record %s: %w", path, err), - fmt.Errorf("quarantine invalid process-group record %s: %w", path, quarantineErr), - ) - } - w.Warn("quarantined invalid process-group record without signaling", - wool.Field("record", path), - wool.Field("quarantine", quarantinePath), - wool.ErrField(err)) - return dispositionRemoved, nil - } + w.Warn("could not reconcile process-group record", wool.Field("record", path), wool.Field("disposition", "retained-invalid-record"), @@ -410,6 +408,13 @@ func reconcile(ctx context.Context, path string, policy reapPolicy) (string, err wool.Field("pgid", rec.pgid), wool.Field("parent", rec.parent), } + // A historical root record must not bypass a supported registration's + // authentication or persisted workspace selection. + if _, lookupErr := runners.LookupProcessGroup(rec.pgid); lookupErr == nil { + return dispositionFailed, fmt.Errorf("group %d has both a legacy root record and a supported registration; records retained", rec.pgid) + } else if !errors.Is(lookupErr, runners.ErrProcessGroupNotRegistered) { + return dispositionFailed, lookupErr + } if !groupAlive(rec.pgid) { if removeErr := removeRecord(path, snapshot); removeErr != nil { @@ -419,9 +424,9 @@ func reconcile(ctx context.Context, path string, policy reapPolicy) (string, err wool.Field("disposition", "removed-dead-group"))...) return dispositionRemoved, nil } - // A record carries no workspace of its own — independently released agents - // write these and this package cannot change their format — so the group is - // attributed the same way an orphan is: from its leader's working directory. + // Historical root records carry no workspace metadata. Supported owner + // registrations were handled above through Core; this compatibility path + // retains environment/cwd attribution for historical producers. // Out of scope means left alone entirely: not signalled, and its record kept, // because the workspace that owns it is still using it. if !policy.scope.includesProcessGroup(rec.pgid) { @@ -724,24 +729,6 @@ func removeRecord(path string, snapshot recordSnapshot) error { return nil } -func quarantineInvalidRecord(path string, snapshot recordSnapshot) (string, error) { - if err := recordIsUnchanged(path, snapshot); err != nil { - return "", err - } - quarantinePath := path + ".invalid" - if err := os.Rename(path, quarantinePath); err != nil { - return "", err - } - directory, err := os.Open(filepath.Dir(path)) - if err != nil { - return quarantinePath, err - } - if err := errors.Join(directory.Sync(), directory.Close()); err != nil { - return quarantinePath, err - } - return quarantinePath, nil -} - func recordedCommand(summary string) string { separator := strings.LastIndex(summary, " <") if separator <= 0 || !strings.HasSuffix(summary, " args>") { @@ -854,10 +841,8 @@ func inspectProcessGroup(ctx context.Context, pgid int) ([]processIdentity, erro return identities, nil } -var inspectAuthenticatedProcessGroup = inspectProcessGroup - func authenticateAuthenticatedGroup(ctx context.Context, rec *authenticatedRecord) ([]processIdentity, bool, error) { - members, err := inspectAuthenticatedProcessGroup(ctx, rec.PGID) + members, err := inspectProcessGroup(ctx, rec.PGID) if err != nil { return nil, false, err } diff --git a/pkg/processgroup/reaper_test.go b/pkg/processgroup/reaper_test.go index 4be87f31..955f7e9e 100644 --- a/pkg/processgroup/reaper_test.go +++ b/pkg/processgroup/reaper_test.go @@ -22,15 +22,16 @@ import ( ) const ( - helperRoleEnv = "CODEFLY_TEST_PROCESS_GROUP_ROLE" - helperPortEnv = "CODEFLY_TEST_PROCESS_GROUP_PORT" - helperPIDEnv = "CODEFLY_TEST_PROCESS_GROUP_PID_FILE" - helperChildPIDEnv = "CODEFLY_TEST_PROCESS_GROUP_CHILD_PID_FILE" - helperReadyEnv = "CODEFLY_TEST_PROCESS_GROUP_READY_FILE" - helperReleaseEnv = "CODEFLY_TEST_PROCESS_GROUP_RELEASE_FILE" - helperTermEnv = "CODEFLY_TEST_PROCESS_GROUP_TERM_FILE" - helperPortsEnv = "CODEFLY_TEST_PROCESS_GROUP_PORTS" - helperStateDirEnv = "CODEFLY_TEST_PROCESS_GROUP_STATE_DIR" + authenticatedDirName = "authenticated-v1" // Explicit old-producer compatibility fixture. + helperRoleEnv = "CODEFLY_TEST_PROCESS_GROUP_ROLE" + helperPortEnv = "CODEFLY_TEST_PROCESS_GROUP_PORT" + helperPIDEnv = "CODEFLY_TEST_PROCESS_GROUP_PID_FILE" + helperChildPIDEnv = "CODEFLY_TEST_PROCESS_GROUP_CHILD_PID_FILE" + helperReadyEnv = "CODEFLY_TEST_PROCESS_GROUP_READY_FILE" + helperReleaseEnv = "CODEFLY_TEST_PROCESS_GROUP_RELEASE_FILE" + helperTermEnv = "CODEFLY_TEST_PROCESS_GROUP_TERM_FILE" + helperPortsEnv = "CODEFLY_TEST_PROCESS_GROUP_PORTS" + helperStateDirEnv = "CODEFLY_TEST_PROCESS_GROUP_STATE_DIR" ) func TestProcessGroupHelper(t *testing.T) { @@ -47,6 +48,32 @@ func TestProcessGroupHelper(t *testing.T) { if _, err := runnersbase.StartTrackedProcessGroup(command); err != nil { t.Fatal(err) } + if os.Getenv("CODEFLY_TEST_MIXED_REGISTRIES") == "1" { + current := filepath.Join(os.Getenv("HOME"), ".codefly", "runs", "authenticated-v2", fmt.Sprintf("%d.pgid", command.Process.Pid)) + data, err := os.ReadFile(current) + if err != nil { + t.Fatal(err) + } + var historical authenticatedRecord + if err := json.Unmarshal(data, &historical); err != nil { + t.Fatal(err) + } + data, err = json.Marshal(historical) + if err != nil { + t.Fatal(err) + } + previous := filepath.Join(os.Getenv("HOME"), ".codefly", "runs", "authenticated-v1") + if err := os.MkdirAll(previous, 0700); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(filepath.Join(previous, fmt.Sprintf("%d.pgid", command.Process.Pid)), data, 0600); err != nil { + t.Fatal(err) + } + if err := os.Remove(current); err != nil { + t.Fatal(err) + } + + } if err := os.WriteFile(os.Getenv(helperPIDEnv), []byte(strconv.Itoa(command.Process.Pid)), 0o600); err != nil { t.Fatal(err) } @@ -77,14 +104,19 @@ func TestProcessGroupHelper(t *testing.T) { command.SysProcAttr = &syscall.SysProcAttr{Setpgid: true} command.Stdout = io.Discard command.Stderr = io.Discard - if err := command.Start(); err != nil { - t.Fatal(err) + if os.Getenv("CODEFLY_TEST_MIXED_REGISTRIES") == "1" { + if _, err := runnersbase.StartTrackedProcessGroup(command); err != nil { + t.Fatal(err) + } + } else { + if err := command.Start(); err != nil { + t.Fatal(err) + } + if err := writeLegacyPgidFile(command.Process.Pid, os.TempDir(), []string{os.Args[0]}); err != nil { + t.Fatal(err) + } } pid := command.Process.Pid - if err := writeLegacyPgidFile(pid, os.TempDir(), []string{os.Args[0]}); err != nil { - _ = syscall.Kill(-pid, syscall.SIGKILL) - t.Fatal(err) - } if err := os.WriteFile(os.Getenv(helperChildPIDEnv), []byte(strconv.Itoa(pid)), 0o600); err != nil { _ = syscall.Kill(-pid, syscall.SIGKILL) t.Fatal(err) @@ -279,6 +311,13 @@ func TestProcessGroupHelper(t *testing.T) { } func TestReaperCleansLegacyAgentProcessesAfterCurrentOwnerIsKilled(t *testing.T) { + testReaperOwnerChain(t) +} +func TestReaperConvergesAcrossProducerNamespaces(t *testing.T) { + t.Setenv("CODEFLY_TEST_MIXED_REGISTRIES", "1") + testReaperOwnerChain(t) +} +func testReaperOwnerChain(t *testing.T) { t.Setenv("HOME", t.TempDir()) dir := t.TempDir() port := availablePort(t) @@ -350,9 +389,26 @@ func TestReaperCleansAuthenticatedRootRecordAfterOwnerIsKilled(t *testing.T) { waitForFile(t, pidPath) waitForFile(t, readyPath) pid := readPID(t, pidPath) - currentRecord := filepath.Join(home, ".codefly", stateDirName, "authenticated-v1", fmt.Sprintf("%d.pgid", pid)) + currentRecord := filepath.Join(home, ".codefly", stateDirName, "authenticated-v2", fmt.Sprintf("%d.pgid", pid)) rootRecord := registryPath(t, pid) - if err := os.Rename(currentRecord, rootRecord); err != nil { + // Historical root JSON predates persisted workspace attribution. Preserve + // that old wire fixture rather than move a new namespace envelope to root. + data, err := os.ReadFile(currentRecord) + if err != nil { + t.Fatal(err) + } + var historical authenticatedRecord + if err := json.Unmarshal(data, &historical); err != nil { + t.Fatal(err) + } + data, err = json.Marshal(historical) + if err != nil { + t.Fatal(err) + } + if err := os.WriteFile(rootRecord, data, 0600); err != nil { + t.Fatal(err) + } + if err := os.Remove(currentRecord); err != nil { t.Fatal(err) } t.Cleanup(func() { @@ -465,7 +521,7 @@ func TestStopManagedProcessGroupsReapsGraphAndPreservesStateForRestart(t *testin } } -func TestReaperTreatsEmptyAuthenticatedRecordAsClean(t *testing.T) { +func TestReaperUsesOwnerInspectionForSupportedRecords(t *testing.T) { home := t.TempDir() t.Setenv("HOME", home) port := availablePort(t) @@ -508,20 +564,15 @@ func TestReaperTreatsEmptyAuthenticatedRecordAsClean(t *testing.T) { if err := os.WriteFile(recordPath, recordData, 0o600); err != nil { t.Fatal(err) } - originalInspector := inspectAuthenticatedProcessGroup - inspectAuthenticatedProcessGroup = func(context.Context, int) ([]processIdentity, error) { return nil, nil } - t.Cleanup(func() { inspectAuthenticatedProcessGroup = originalInspector }) - evidence, err := ReapStaleProcessGroupsWithEvidence(context.Background(), AllWorkspaces()) - inspectAuthenticatedProcessGroup = originalInspector if err != nil { t.Fatal(err) } if evidence.Outcome != CleanupClean { t.Fatalf("cleanup outcome = %q, want %q", evidence.Outcome, CleanupClean) } - if _, err := os.Stat(recordPath); !errors.Is(err, os.ErrNotExist) { - t.Fatalf("empty authenticated record still exists: %v", err) + if _, err := os.Stat(recordPath); err != nil { + t.Fatalf("live v1 registration was not preserved: %v", err) } assertPortHeld(t, port) if err := syscall.Kill(-pid, 0); err != nil { @@ -895,6 +946,9 @@ func TestReaperContinuesConvergenceAfterRecordFailure(t *testing.T) { if err := os.WriteFile(lockPath, nil, 0o600); err != nil { t.Fatal(err) } + if _, err := runnersbase.ListProcessGroups(context.Background()); err != nil { + t.Fatal(err) + } if err := os.Chmod(registryDir, 0o555); err != nil { t.Fatal(err) } diff --git a/pkg/processgroup/registered.go b/pkg/processgroup/registered.go new file mode 100644 index 00000000..714be68e --- /dev/null +++ b/pkg/processgroup/registered.go @@ -0,0 +1,106 @@ +package processgroup + +import ( + "context" + "errors" + "fmt" + + runners "github.com/codefly-dev/core/runners/base" +) + +// PreviewManagedProcessGroups lists live, authenticated, in-scope registrations +// without signaling or removing records. Refusals match managed stop admission. +func PreviewManagedProcessGroups(ctx context.Context, scope Scope) ([]int, error) { + groups, err := runners.ListProcessGroups(ctx) + if err != nil { + return nil, err + } + var selected []int + for _, group := range groups { + if !group.Alive() { + continue + } + ownership, err := group.InspectOwnership(ctx) + if err != nil { + return nil, err + } + if !ownership.Authenticated { + return nil, fmt.Errorf("group %d ownership is unproven", group.PGID()) + } + if registeredGroupInScope(group, ownership, scope) { + selected = append(selected, group.PGID()) + } + } + return selected, nil +} + +// Core owns supported registry discovery, authentication and identity-bound +// signals. CLI supplies cleanup policy and evidence; the separate root scanner +// remains solely for historical line/JSON records outside Core's namespaces. +func reapRegisteredProcessGroups(ctx context.Context, policy reapPolicy) (CleanupEvidence, error) { + evidence := CleanupEvidence{Outcome: CleanupClean} + groups, err := runners.ListProcessGroups(ctx) + if err != nil { + evidence.Outcome = CleanupFailed + return evidence, err + } + var failures []error + for _, group := range groups { + outcome, outside, err := reconcileRegisteredGroup(ctx, group, policy) + if outside { + evidence.OutOfScope = append(evidence.OutOfScope, group.PGID()) + continue + } + if err != nil { + outcome = CleanupFailed + failures = append(failures, fmt.Errorf("reconcile registered group %d: %w", group.PGID(), err)) + } + // An owner handle has an identity, not a consumer-owned record path. + evidence.record(fmt.Sprintf("registered-group:%d", group.PGID()), group.PGID(), outcome) + } + return evidence, errors.Join(failures...) +} + +func reconcileRegisteredGroup(ctx context.Context, group *runners.TrackedProcessGroup, policy reapPolicy) (CleanupOutcome, bool, error) { + if err := ctx.Err(); err != nil { + return CleanupFailed, false, err + } + if !group.Alive() { + return CleanupClean, false, group.RemoveIfDeadContext(ctx) + } + ownership, err := group.InspectOwnership(ctx) + if err != nil { + return CleanupFailed, false, err + } + if !ownership.Authenticated { + return CleanupFailed, false, fmt.Errorf("live group ownership is unproven; registration retained without signaling") + } + if !registeredGroupInScope(group, ownership, policy.scope) { + return CleanupClean, true, nil + } + if ownership.OwnerAlive && !policy.stopManaged { + return CleanupClean, false, nil + } + if err := group.Terminate(ctx); err != nil { + return CleanupFailed, false, err + } + if err := group.RemoveIfDeadContext(ctx); err != nil { + return CleanupFailed, false, err + } + if !ownership.OwnerAlive { + return CleanupRecoveredOrphan, false, nil + } + return CleanupClean, false, nil +} + +func registeredGroupInScope(group *runners.TrackedProcessGroup, ownership *runners.ProcessGroupOwnership, scope Scope) bool { + if scope.All() { + return true + } + if ownership.LaunchWorkspace != "" { + return scope.Includes(ownership.LaunchWorkspace) + } + // Older registrations have no persisted workspace. Preserve their existing + // environment/cwd attribution only after Core has authenticated the group. + return scope.includesProcessGroup(group.PGID()) +} diff --git a/pkg/processgroup/registered_test.go b/pkg/processgroup/registered_test.go new file mode 100644 index 00000000..e5606ff4 --- /dev/null +++ b/pkg/processgroup/registered_test.go @@ -0,0 +1,94 @@ +package processgroup + +import ( + "context" + "io" + "os" + "os/exec" + "path/filepath" + "strconv" + "syscall" + "testing" + "time" + + runners "github.com/codefly-dev/core/runners/base" +) + +func TestRegisteredWorkspaceOwner(t *testing.T) { + pidFile := os.Getenv("CODEFLY_TEST_REGISTERED_PID_FILE") + if pidFile == "" { + return + } + child := exec.Command("env", "-i", "sleep", "60") + child.Stdout, child.Stderr = io.Discard, io.Discard + group, err := runners.StartTrackedProcessGroup(child) + if err != nil { + t.Fatal(err) + } + if err := os.WriteFile(pidFile, []byte(strconv.Itoa(group.PGID())), 0600); err != nil { + t.Fatal(err) + } +} + +func startUnobservableRun(t *testing.T, workspace, cwd string) int { + t.Helper() + pidFile := filepath.Join(t.TempDir(), "pid") + owner := exec.Command(os.Args[0], "-test.run=^TestRegisteredWorkspaceOwner$") + owner.Dir = cwd + owner.Env = append(withoutLaunchWorkspace(os.Environ()), LaunchWorkspaceEnv+"="+workspace, "CODEFLY_TEST_REGISTERED_PID_FILE="+pidFile) + if out, err := owner.CombinedOutput(); err != nil { + t.Fatalf("owner: %v: %s", err, out) + } + pid := readPID(t, pidFile) + t.Cleanup(func() { _ = syscall.Kill(-pid, syscall.SIGKILL) }) + deadline := time.Now().Add(5 * time.Second) + for { + identity, err := runners.InspectProcess(pid) + if err == nil && identity.Executable == "sleep" { + if _, visible := LaunchWorkspaceOfProcess(pid); !visible { + break + } + } + if time.Now().After(deadline) { + t.Fatal("child did not erase its environment") + } + time.Sleep(10 * time.Millisecond) + } + return pid +} + +func TestRegisteredStopUsesPersistedWorkspaceAfterOwnerAndEnvironmentDisappear(t *testing.T) { + t.Setenv("HOME", t.TempDir()) + first, second := newWorkspace(t, "first"), newWorkspace(t, "second") + // Both children run from the sibling checkout. Its cwd is deliberately + // wrong evidence for the first child's launch ownership. + firstPID := startUnobservableRun(t, first, second) + secondPID := startUnobservableRun(t, second, second) + preview, err := PreviewManagedProcessGroups(context.Background(), InWorkspace(first)) + if err != nil || len(preview) != 1 || preview[0] != firstPID { + t.Fatalf("preview=%v err=%v", preview, err) + } + if !groupAlive(firstPID) || !groupAlive(secondPID) { + t.Fatal("preview signaled a group") + } + evidence, err := StopManagedProcessGroups(context.Background(), InWorkspace(first)) + if err != nil { + t.Fatal(err) + } + if groupAlive(firstPID) { + t.Fatal("target survived scoped stop") + } + if !groupAlive(secondPID) { + t.Fatal("scoped stop killed sibling") + } + if _, err := runners.LookupProcessGroup(secondPID); err != nil { + t.Fatalf("sibling registration lost: %v", err) + } + found := false + for _, pgid := range evidence.RecoveredPGIDs() { + found = found || pgid == firstPID + } + if !found { + t.Fatalf("target missing from recovery evidence: %+v", evidence) + } +} diff --git a/pkg/processgroup/scope.go b/pkg/processgroup/scope.go index 900a89dd..33ee74e5 100644 --- a/pkg/processgroup/scope.go +++ b/pkg/processgroup/scope.go @@ -65,16 +65,9 @@ func (s Scope) Includes(workspace string) bool { return sameDirectory(filepath.Clean(workspace), s.Workspace) } -// includesProcessGroup resolves a group's workspace from its leader and reports -// whether it is in scope. -// -// A registry record carries no workspace of its own — independently released -// agents write these and this package cannot change their format — so the -// leader is asked instead: the run recorded in its environment when it was -// launched, and only failing that the workspace enclosing its working -// directory. Asking the working directory first was the whole defect: a -// composed module's service runs from a checkout outside the workspace that -// composed it, so a scoped stop left it running and reported success. +// includesProcessGroup provides environment/cwd compatibility attribution for +// historical records without a persisted launch workspace. Current registered +// groups select Core's authenticated LaunchWorkspace before using this fallback. func (s Scope) includesProcessGroup(pgid int) bool { if s.All() { return true diff --git a/pkg/processgroup/scope_test.go b/pkg/processgroup/scope_test.go index 5ff56d44..1dc6d2a1 100644 --- a/pkg/processgroup/scope_test.go +++ b/pkg/processgroup/scope_test.go @@ -2,6 +2,7 @@ package processgroup import ( "context" + "errors" "io" "os" "os/exec" @@ -58,9 +59,11 @@ func startRunIn(t *testing.T, workspace string) int { t.Fatal(err) } pgid := tracked.PGID() + reaped := make(chan struct{}) + go func() { _, _ = command.Process.Wait(); close(reaped) }() t.Cleanup(func() { _ = syscall.Kill(-pgid, syscall.SIGKILL) - _, _ = command.Process.Wait() + <-reaped }) waitForFile(t, readyPath) return pgid @@ -73,15 +76,17 @@ func startRunIn(t *testing.T, workspace string) int { // find its run. func recordExists(t *testing.T, pgid int) bool { t.Helper() + if _, err := runnersbase.LookupProcessGroup(pgid); err == nil { + return true + } else if !errors.Is(err, runnersbase.ErrProcessGroupNotRegistered) { + t.Fatal(err) + } dir, err := stateDir() if err != nil { t.Fatal(err) } - name := strconv.Itoa(pgid) + ".pgid" - for _, candidate := range []string{filepath.Join(dir, authenticatedDirName, name), filepath.Join(dir, name)} { - if _, err := os.Stat(candidate); err == nil { - return true - } + if _, err := os.Stat(filepath.Join(dir, strconv.Itoa(pgid)+".pgid")); err == nil { + return true } return false } From 8add946171a59a6082e62740f036c8b770640833 Mon Sep 17 00:00:00 2001 From: Antoine Toussaint <312536060+antoinetoussaint-byte@users.noreply.github.com> Date: Tue, 27 Oct 2026 17:27:58 -0400 Subject: [PATCH 7/7] fix(stop): report each preserved group once across recovery passes --- pkg/processgroup/reaper.go | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/pkg/processgroup/reaper.go b/pkg/processgroup/reaper.go index ac143d65..79c9d07f 100644 --- a/pkg/processgroup/reaper.go +++ b/pkg/processgroup/reaper.go @@ -10,6 +10,7 @@ import ( "io" "os" "path/filepath" + "slices" "strconv" "strings" "syscall" @@ -160,7 +161,11 @@ func (e CleanupEvidence) RecoveredPGIDs() []int { } func (e *CleanupEvidence) merge(other CleanupEvidence) { - e.OutOfScope = append(e.OutOfScope, other.OutOfScope...) + for _, pgid := range other.OutOfScope { + if !slices.Contains(e.OutOfScope, pgid) { + e.OutOfScope = append(e.OutOfScope, pgid) + } + } for _, group := range other.Groups { e.record(group.Record, group.PGID, group.Outcome) }