fix(companions): proto 0.0.18 for the streaming facade; refuse an unbumped companion change

.github/workflows/companion-version.yml

@@ -0,0 +1,34 @@
+name: companion-version
+
+# A change to a companion's image inputs must bump its version: publishing
+# skips a tag the registry already has, and a forced republish swaps the image
+# under every machine that cached the tag. companions/proto 0.0.17 shipped the
+# streaming Python facade that way (core#837).
+#
+# It runs on every pull request, with no path filter, so it can be a required
+# check of main's ruleset: a path-filtered workflow that does not run leaves a
+# required check pending forever. internal/companionbump reads which files are
+# a companion's inputs from the Dockerfiles themselves; a change that touches
+# none passes at once.
+on:
+  pull_request:
+
+permissions:
+  contents: read
+
+jobs:
+  companion-version:
+    name: Companion version
+    runs-on: ubuntu-latest
+    timeout-minutes: 10
+    steps:
+      - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1
+        with:
+          fetch-depth: 0
+      - uses: actions/setup-go@b7ad1dad31e06c5925ef5d2fc7ad053ef454303e # v7.0.0
+        with:
+          go-version-file: go.mod
+      - name: every changed companion carries a new version
+        env:
+          BASE_REF: origin/${{ github.base_ref }}
+        run: go run ./internal/companionbump "$BASE_REF"

companions/buildspec.go

@@ -2,7 +2,11 @@ package companions
 
 import (
 	"fmt"
+	"os"
+	"path"
+	"path/filepath"
 	"slices"
+	"strings"
 )
 
 // BaseImageArg is the Docker build argument a companion Dockerfile declares
@@ -106,3 +110,142 @@ func BuildSpecs() ([]BuildSpec, error) {
 	}
 	return out, nil
 }
+
+// Instruction is one logical Dockerfile instruction: its physical lines
+// joined where a line ends in the escape character, comment lines inside it
+// dropped, and Line the physical line it starts on.
+type Instruction struct {
+	Line int
+	Text string
+}
+
+// Instructions splits a Dockerfile into its logical instructions. A COPY
+// written over several lines is one instruction, so its sources on the later
+// lines are read with it rather than missed.
+func Instructions(dockerfile string) []Instruction {
+	var out []Instruction
+	var current []string
+	start := 0
+	for number, raw := range strings.Split(dockerfile, "\n") {
+		line := strings.TrimRight(raw, " \t\r")
+		trimmed := strings.TrimSpace(line)
+		if len(current) == 0 && (trimmed == "" || strings.HasPrefix(trimmed, "#")) {
+			continue
+		}
+		if len(current) > 0 && strings.HasPrefix(trimmed, "#") {
+			continue // a comment inside a continued instruction is not part of it
+		}
+		if len(current) == 0 {
+			start = number + 1
+		}
+		if strings.HasSuffix(line, "\\") {
+			current = append(current, strings.TrimSuffix(line, "\\"))
+			continue
+		}
+		current = append(current, line)
+		out = append(out, Instruction{Line: start, Text: strings.Join(current, " ")})
+		current = nil
+	}
+	if len(current) > 0 {
+		out = append(out, Instruction{Line: start, Text: strings.Join(current, " ")})
+	}
+	return out
+}
+
+// CopyKind classifies an instruction for CopySources.
+type CopyKind int
+
+const (
+	// NotCopy is any instruction that is neither COPY nor ADD.
+	NotCopy CopyKind = iota
+	// FromStage is COPY --from=<stage|image>, which reads from another image
+	// rather than from the build context.
+	FromStage
+	// FromContext is a COPY or ADD that reads paths out of the build context.
+	// An ADD source that is a URL or a git remote is not one, and is left out.
+	FromContext
+	// Unparsed is a COPY or ADD whose shape this parser does not understand —
+	// the JSON form, or one with no source and destination. It is reported
+	// rather than skipped: a copy nobody reads is an image input nothing
+	// checks.
+	Unparsed
+)
+
+// CopySources returns the build-context sources of a COPY or ADD instruction,
+// in any case (Dockerfile instructions are case-insensitive). Pass it a
+// logical instruction (Instructions), not a physical line.
+//
+// Leading flags are stripped rather than abandoning the line. Skipping any
+// flagged COPY would leave `COPY --chown=…` and `COPY --link` — both ordinary
+// context reads — unread, so their sources would escape every check.
+func CopySources(instruction string) ([]string, CopyKind) {
+	fields := strings.Fields(instruction)
+	if len(fields) == 0 {
+		return nil, NotCopy
+	}
+	command := strings.ToUpper(fields[0])
+	if command != "COPY" && command != "ADD" {
+		return nil, NotCopy
+	}
+	operands := fields[1:]
+	for len(operands) > 0 && strings.HasPrefix(operands[0], "--") {
+		if strings.HasPrefix(operands[0], "--from=") {
+			return nil, FromStage
+		}
+		operands = operands[1:]
+	}
+	if len(operands) < 2 || strings.HasPrefix(operands[0], "[") {
+		return nil, Unparsed
+	}
+	var sources []string
+	for _, source := range operands[:len(operands)-1] {
+		if strings.HasPrefix(source, "<<") {
+			continue // a heredoc: its content is in the Dockerfile itself
+		}
+		if command == "ADD" && remoteSource(source) {
+			continue
+		}
+		sources = append(sources, source)
+	}
+	return sources, FromContext
+}
+
+// remoteSource reports whether an ADD source is fetched rather than read from
+// the build context.
+func remoteSource(source string) bool {
+	for _, prefix := range []string{"http://", "https://", "git@", "git://", "ssh://"} {
+		if strings.HasPrefix(source, prefix) {
+			return true
+		}
+	}
+	return false
+}
+
+// Inputs returns the repository-root-relative POSIX paths, read from the
+// checkout at root, that this companion's image is built from in this
+// repository: its Dockerfile and every build-context COPY or ADD source, a file or a
+// directory. The staged CLI binary is not one: it is built from codefly-dev/cli
+// at publish time. A change under any input changes the image, so it needs a
+// new Version: publishing skips a tag the registry already has.
+func (s BuildSpec) Inputs(root string) ([]string, error) {
+	content, err := os.ReadFile(filepath.Join(root, filepath.FromSlash(s.Dockerfile))) //nolint:gosec // the spec's own Dockerfile under the checkout root
+	if err != nil {
+		return nil, fmt.Errorf("cannot read the <%s> Dockerfile: %w", s.Name, err)
+	}
+	inputs := []string{s.Dockerfile}
+	for _, instruction := range Instructions(string(content)) {
+		sources, kind := CopySources(instruction.Text)
+		switch kind {
+		case Unparsed:
+			return nil, fmt.Errorf("%s:%d is a COPY or ADD whose sources cannot be read: %s", s.Dockerfile, instruction.Line, strings.TrimSpace(instruction.Text))
+		case FromContext:
+			for _, source := range sources {
+				if s.CLIBinary != "" && source == s.CLIBinary {
+					continue
+				}
+				inputs = append(inputs, path.Clean(path.Join(s.Context, source)))
+			}
+		}
+	}
+	return inputs, nil
+}

companions/buildspec_test.go

@@ -216,64 +216,40 @@ func TestBuildSpecsMatchWhatTheCurrentPublisherApplies(t *testing.T) {
 	}
 }
 
-// copyKind classifies a Dockerfile line for copySources.
-type copyKind int
-
-const (
-	// notCopy is any line that is not a COPY instruction.
-	notCopy copyKind = iota
-	// fromStage is COPY --from=<stage|image>, which reads from another image
-	// rather than from the build context.
-	fromStage
-	// fromContext is a COPY that reads paths out of the build context.
-	fromContext
-	// unparsed is a COPY whose shape this parser does not understand — the
-	// JSON form, or one with no source and destination. It is reported rather
-	// than skipped: a COPY nobody checks is exactly the drift these tests
-	// exist to catch.
-	unparsed
-)
-
-// copySources returns the build-context sources of a COPY instruction.
-//
-// Leading flags are stripped rather than abandoning the line. Skipping any
-// flagged COPY would leave `COPY --chown=…` and `COPY --link` — both ordinary
-// context reads — unguarded, so a bad path in one would reach the builder.
-func copySources(line string) ([]string, copyKind) {
-	fields := strings.Fields(line)
-	if len(fields) == 0 || fields[0] != "COPY" {
-		return nil, notCopy
-	}
-	operands := fields[1:]
-	for len(operands) > 0 && strings.HasPrefix(operands[0], "--") {
-		if strings.HasPrefix(operands[0], "--from=") {
-			return nil, fromStage
-		}
-		operands = operands[1:]
-	}
-	if len(operands) < 2 || strings.HasPrefix(operands[0], "[") {
-		return nil, unparsed
-	}
-	return operands[:len(operands)-1], fromContext
+// A COPY spread over continued lines is one instruction: its later sources
+// are inputs too, and a comment inside it is not part of it.
+func TestInstructionsJoinContinuedLines(t *testing.T) {
+	dockerfile := "FROM alpine\n# a comment\nCOPY a \\\n  # inside\n  b /dst/\nRUN true\n"
+	instructions := companions.Instructions(dockerfile)
+	require.Len(t, instructions, 3)
+	require.Equal(t, 3, instructions[1].Line)
+	sources, kind := companions.CopySources(instructions[1].Text)
+	require.Equal(t, companions.FromContext, kind)
+	require.Equal(t, []string{"a", "b"}, sources)
 }
 
 func TestCopySourcesReadsEveryContextCopyShape(t *testing.T) {
 	for line, expected := range map[string]struct {
 		sources []string
-		kind    copyKind
+		kind    companions.CopyKind
 	}{
-		"COPY facades/go /src":                          {[]string{"facades/go"}, fromContext},
-		"COPY a b /dst/":                                {[]string{"a", "b"}, fromContext},
-		"COPY --chown=1000:1000 facades/ts /opt":        {[]string{"facades/ts"}, fromContext},
-		"COPY --link --chown=root:root bin/x /bin/x":    {[]string{"bin/x"}, fromContext},
-		"  COPY bin/linux/${TARGETARCH}/codefly /bin/c": {[]string{"bin/linux/${TARGETARCH}/codefly"}, fromContext},
-		"COPY --from=builder /go/bin/buf /usr/local/":   {nil, fromStage},
-		"RUN apk add --no-cache bash":                   {nil, notCopy},
-		"":                                              {nil, notCopy},
-		`COPY ["a", "b"]`:                               {nil, unparsed},
-		"COPY onlyone":                                  {nil, unparsed},
+		"COPY facades/go /src":                          {[]string{"facades/go"}, companions.FromContext},
+		"COPY a b /dst/":                                {[]string{"a", "b"}, companions.FromContext},
+		"COPY --chown=1000:1000 facades/ts /opt":        {[]string{"facades/ts"}, companions.FromContext},
+		"COPY --link --chown=root:root bin/x /bin/x":    {[]string{"bin/x"}, companions.FromContext},
+		"  COPY bin/linux/${TARGETARCH}/codefly /bin/c": {[]string{"bin/linux/${TARGETARCH}/codefly"}, companions.FromContext},
+		"COPY --from=builder /go/bin/buf /usr/local/":   {nil, companions.FromStage},
+		"RUN apk add --no-cache bash":                   {nil, companions.NotCopy},
+		"":                                              {nil, companions.NotCopy},
+		"copy facades/py /opt":                          {[]string{"facades/py"}, companions.FromContext},
+		"ADD entrypoint.sh /usr/local/bin/":             {[]string{"entrypoint.sh"}, companions.FromContext},
+		"add --chown=1 a https://example.com/x.tgz /d/": {[]string{"a"}, companions.FromContext},
+		"ADD git@github.com:o/r.git /src":               {nil, companions.FromContext},
+		"COPY <<EOF /etc/conf":                          {nil, companions.FromContext},
+		`COPY ["a", "b"]`:                               {nil, companions.Unparsed},
+		"COPY onlyone":                                  {nil, companions.Unparsed},
 	} {
-		sources, kind := copySources(line)
+		sources, kind := companions.CopySources(line)
 		require.Equalf(t, expected.kind, kind, "kind for %q", line)
 		require.Equalf(t, expected.sources, sources, "sources for %q", line)
 	}
@@ -286,12 +262,12 @@ func TestDockerfileCopySourcesResolveInTheDeclaredContext(t *testing.T) {
 	root := repositoryRoot(t)
 	for _, spec := range buildSpecs(t) {
 		context := filepath.Join(root, filepath.FromSlash(spec.Context))
-		for number, line := range strings.Split(dockerfile(t, spec), "\n") {
-			sources, kind := copySources(line)
-			require.NotEqualf(t, unparsed, kind,
+		for _, instruction := range companions.Instructions(dockerfile(t, spec)) {
+			sources, kind := companions.CopySources(instruction.Text)
+			require.NotEqualf(t, companions.Unparsed, kind,
 				"%s:%d is a COPY this test cannot read, so nothing checks its sources: %s",
-				spec.Dockerfile, number+1, strings.TrimSpace(line))
-			if kind != fromContext {
+				spec.Dockerfile, instruction.Line, strings.TrimSpace(instruction.Text))
+			if kind != companions.FromContext {
 				continue
 			}
 			for _, source := range sources {
@@ -304,7 +280,7 @@ func TestDockerfileCopySourcesResolveInTheDeclaredContext(t *testing.T) {
 				}
 				_, err := os.Stat(filepath.Join(context, filepath.FromSlash(source)))
 				require.NoErrorf(t, err, "%s:%d copies %s, which does not exist in the %s context",
-					spec.Dockerfile, number+1, source, spec.Context)
+					spec.Dockerfile, instruction.Line, source, spec.Context)
 			}
 		}
 	}
@@ -339,8 +315,8 @@ func TestDockerIgnoreExcludesNothingTheBuildsNeed(t *testing.T) {
 		if spec.CLIBinary != "" {
 			required = append(required, spec.CLIBinary)
 		}
-		for _, line := range strings.Split(dockerfile(t, spec), "\n") {
-			if sources, kind := copySources(line); kind == fromContext {
+		for _, instruction := range companions.Instructions(dockerfile(t, spec)) {
+			if sources, kind := companions.CopySources(instruction.Text); kind == companions.FromContext {
 				required = append(required, sources...)
 			}
 		}

companions/proto/README.md

@@ -125,6 +125,18 @@ local plugins: it compiles the `*_pb2` messages and `*_pb2_grpc` stubs with the
 own, and their gencode outran the protobuf runtime a generated library
 declares. It ships in companion version 0.0.17.
 
+The Python facade plugin (`facades/python/protoc_gen_codefly_facade_python.py`)
+calls the gateway seam named for each RPC's shape: `unary`, `unary_stream`,
+`stream_unary` or `stream_stream`. It ships in companion version 0.0.18. An
+earlier `0.0.17` image renders every RPC as `unary`; 0.0.17 was republished
+with the streaming plugin, and a machine that had pulled it before keeps the
+old image.
+
+A change to any of a companion's image inputs (its Dockerfile and every
+build-context `COPY` source, `BuildSpec.Inputs`) must bump its
+`info.codefly.yaml`: `internal/companionbump`, the required `Companion version`
+check on every pull request, refuses one that does not.
+
 Rust generation stages output before publishing it. `.codefly-rust-output.json`
 records the generated files and their hashes; only unchanged, tracked files may
 be replaced or removed. Unrelated files are preserved. A collision with an

companions/proto/info.codefly.yaml

@@ -1 +1 @@
-version: 0.0.17
+version: 0.0.18

docs/runbooks/branch-protection.md

@@ -36,9 +36,9 @@ Queried, not inferred. Ruleset 24397257, `target: branch`,
 
 - **`required_status_checks`** — `Build`, `Proto`, `Registry cache cold (go)`,
   `Registry cache cold (next)`, `Registry cache clean runner (go)`,
-  `Registry cache clean runner (next)`. Six, not the seven below:
+  `Registry cache clean runner (next)`. Six, not the eight below:
   `pnpm source evidence` is derived as safe to require and is **not** in the
-  live ruleset.
+  live ruleset; `Companion version` joins it once #838 merges.
 - **`strict_required_status_checks_policy: false`** — a pull request that went
   green against an older `main` can land without being retested. The `"strict":
   true` recommended below is not what is applied.
@@ -79,6 +79,7 @@ if you require them.
 
 ```
 Build
+Companion version
 Proto
 pnpm source evidence
 Registry cache cold (go)
@@ -92,10 +93,12 @@ coverage, `govulncheck`, and the version/tag and CGO-free guards. It is the
 check #447 was red on. `Proto` is the same workflow's `buf breaking` gate on
 `proto/`, split out so a BSR hiccup and a schema break answer separately.
 `pnpm source evidence` runs the real lockfile audit and inventory tests with the
-pinned pnpm toolchain. The other four are `build-cache.yml`'s registry
+pinned pnpm toolchain. `Companion version` (`companion-version.yml`) refuses a
+change to a companion's image inputs that keeps its version, so a published tag
+never names two images. The other four are `build-cache.yml`'s registry
 cache-conformance matrix.
 
-All seven qualify on the same three grounds, which are what make a required check
+All eight qualify on the same three grounds, which are what make a required check
 safe rather than merely desirable:
 
 - **Their workflow runs on every pull request to `main`** — `on.pull_request`
@@ -143,6 +146,7 @@ gh api -X PUT repos/codefly-dev/core/rulesets/24397257 --input - <<'JSON'
       "do_not_enforce_on_create": false,
       "required_status_checks": [
         {"context": "Build"},
+        {"context": "Companion version"},
         {"context": "Proto"},
         {"context": "pnpm source evidence"},
         {"context": "Registry cache cold (go)"},

internal/ciguard/branchprotection_test.go

@@ -34,6 +34,7 @@ import (
 // docs/runbooks/branch-protection.md tells an operator to apply.
 var requiredChecks = []string{
 	"Build",
+	"Companion version",
 	"Proto",
 	"pnpm source evidence",
 	"Registry cache clean runner (go)",