fix(conformance,lint,release): public-ports class, lint continuation, worktree bundles, rehearse args, prerelease compare (D-0626 D-0615 D-1169 D-1107 D-1106)

.github/wiki/Plugin-Conformance.md

@@ -27,7 +27,7 @@ Exit 0 means every selected rule passed. A rule that cannot run (no Docker, no t
 | `healthcheck` | A compose plugin has a real health check: a compose `healthcheck:` that is not disabled (`disable: true`, `test: NONE`), or a Dockerfile `HEALTHCHECK` whose last instruction is not `NONE`. A disabled compose healthcheck fails even when the Dockerfile has one. |
 | `replace` | No `go.mod` or `go.work` `replace` or `use` points outside the plugin directory, quoted targets included. |
 | `docker-socket` | No `docker.sock`, `/var/run/docker` or `/var/run` bind in the compose fragment named by `service.compose` (whatever it is called), in any `*.yml` or `*.yaml` of the plugin, or in a Dockerfile (ADR 0027), unless a dated exception names the plugin. |
-| `port-bind` | Every published port of every compose service binds `127.0.0.1`: in the `service.compose` fragment (which must parse) and in any other `*.yml` or `*.yaml` of the plugin (outside `vendor/`, `node_modules/`, `third_party/` and `testdata/`) that has a top-level `services:` map. The parser is `scripts/conformance/portbind` (Go). A port with no host IP (`"9007:9007"`, `"9007"`), `0.0.0.0`, an IPv6 address, a host IP taken from a variable, a long-syntax entry without `host_ip: 127.0.0.1` and `network_mode: host` all fail. A deliberate public port needs a dated `port-bind` exception (today: `traefik`, 80 and 443). |
+| `port-bind` | Every published port of every compose service binds `127.0.0.1`: in the `service.compose` fragment (which must parse) and in any other `*.yml` or `*.yaml` of the plugin (outside `vendor/`, `node_modules/`, `third_party/` and `testdata/`) that has a top-level `services:` map. The parser is `scripts/conformance/portbind` (Go). A port with no host IP (`"9007:9007"`, `"9007"`), `0.0.0.0`, an IPv6 address, a host IP taken from a variable, a long-syntax entry without `host_ip: 127.0.0.1` and `network_mode: host` all fail. A deliberate public port is listed in `scripts/conformance/public-ports.yaml` (plugin, one container port, reason; no expiry; checked per port, so every other port of that plugin still has to bind `127.0.0.1`; `network_mode: host` is never excused). Today: `traefik` 80, 443 and 8444. The `exceptions` rule validates the file. A dated `port-bind` exception in `exceptions.yaml` still excuses a whole plugin. |
 | `migrations` | A plugin with `migrations/*.sql` declares `migrations.apply: boot`, and a Go plugin calls `migrate.Apply` at boot in non-test source: a call in a comment or only in a `_test.go` file does not count. |
 | `boot-schema` | A manifest with `migrations.apply: boot` declares `schema: np_<name with - replaced by _>`, the schema that holds its boot ledger. A `Schema: "np_..."` literal in non-test Go source must name the same schema. |
 | `migration-immutable` | A boot migration file that the last release shipped is frozen: editing, deleting or renaming it fails, because the boot ledger stores each applied file's sha256 and a changed file is fatal at boot. Only plugins that the baseline release already applied at boot (manifest v2 with `migrations.apply: boot` at that tag) are checked; earlier releases had no ledger. Fix: add a new file. |
@@ -41,7 +41,7 @@ Exit 0 means every selected rule passed. A rule that cannot run (no Docker, no t
 
 ## Exceptions
 
-`scripts/conformance/exceptions.yaml` lists the only allowed rule breaks. Each entry has `rule`, `plugin`, `reason`, `owner_ref` and `expires`. An expired entry fails the gate, and `expires` may not be later than the v1.5.0 cap date. The cap is a constant in `scripts/conformance/lib.sh` (`release_cap`), not read from the data file, so a file cannot raise its own cap. Remove an entry in the same change that lands its owner Ticket. Today: `docker-socket` for `forgejo` and `monitoring`, `port-bind` for `traefik` (the edge proxy publishes 80 and 443 by design), and `cli-targets` entries for plugins whose release builds no binary yet (see the file).
+`scripts/conformance/exceptions.yaml` lists the only allowed rule breaks. Each entry has `rule`, `plugin`, `reason`, `owner_ref` and `expires`. An expired entry fails the gate, and `expires` may not be later than the v1.5.0 cap date. The cap is a constant in `scripts/conformance/lib.sh` (`release_cap`), not read from the data file, so a file cannot raise its own cap. Remove an entry in the same change that lands its owner Ticket. Today: `docker-socket` for `forgejo` and `monitoring`, and `cli-targets` entries for plugins whose release builds no binary yet (see the file).
 
 ## Pinned schema
 

.github/workflows/hygiene.yml

@@ -183,6 +183,16 @@ jobs:
         if: always()
         run: bash scripts/ci/tests/test-lint-compose-password-urls.sh
 
+      - name: "plugin-counts: bundles checkout autodetect from a worktree (D-1169)"
+        id: plugin_counts_bundles_dir_test
+        if: always()
+        run: bash scripts/ci/tests/test-plugin-counts-bundles-dir.sh
+
+      - name: "rehearse: a --tag or --out with no value is refused, not looped on (D-1107)"
+        id: rehearse_args_test
+        if: always()
+        run: bash scripts/release/rehearse-args-test.sh
+
       # docker compose ships on ubuntu-latest; the smoke renders, starts nothing.
       - name: "fragment-urlenc-smoke: reserved-character passwords render and round-trip (D-0235)"
         id: fragment_urlenc_smoke
@@ -504,6 +514,8 @@ jobs:
             [nself_first]="${{ steps.nself_first.outcome }}"
             [compose_password_urls]="${{ steps.compose_password_urls.outcome }}"
             [compose_password_urls_test]="${{ steps.compose_password_urls_test.outcome }}"
+            [plugin_counts_bundles_dir_test]="${{ steps.plugin_counts_bundles_dir_test.outcome }}"
+            [rehearse_args_test]="${{ steps.rehearse_args_test.outcome }}"
             [fragment_urlenc_smoke]="${{ steps.fragment_urlenc_smoke.outcome }}"
             [compose_network_literals]="${{ steps.compose_network_literals.outcome }}"
             [tracked_binaries]="${{ steps.tracked_binaries.outcome }}"

free/infra/cmd/help_json.go

@@ -28,6 +28,7 @@ import (
 	"bytes"
 	"fmt"
 	"io"
+	"strconv"
 	"strings"
 
 	"github.com/nself-org/cli/sdk/go/v2/output"
@@ -57,7 +58,7 @@ func helpJSON(root *cobra.Command, args []string, stdout, stderr io.Writer) (han
 	}
 	rest := make([]string, 0, len(args))
 	for _, a := range args {
-		if a == "--json" || strings.HasPrefix(a, "--json=") {
+		if isJSONToken(a) {
 			continue
 		}
 		rest = append(rest, a)
@@ -98,6 +99,22 @@ func helpJSON(root *cobra.Command, args []string, stdout, stderr io.Writer) (han
 	return true, 0
 }
 
+// isJSONToken reports whether a is a spelling of --json that wantsJSON accepts:
+// the bare flag or --json=<value> with a value strconv.ParseBool takes. Anything
+// else (--json=maybe) stays in the probe's arguments, so the flag parse fails
+// the way the normal run's parse does and the request is left to that path.
+func isJSONToken(a string) bool {
+	if a == "--json" {
+		return true
+	}
+	v, ok := strings.CutPrefix(a, "--json=")
+	if !ok {
+		return false
+	}
+	_, err := strconv.ParseBool(v)
+	return err == nil
+}
+
 // resetFlags puts every flag of c back to its default, so a parse done only to
 // look for --help leaves no trace.
 func resetFlags(c *cobra.Command) {

free/infra/cmd/help_json_test.go

@@ -170,3 +170,22 @@ func TestHelpJSONLeavesFlagsClean(t *testing.T) {
 		t.Errorf("name leaked: %q changed=%v", v, f.Lookup("name").Changed)
 	}
 }
+
+// TestHelpJSONOnlyStripsValidJSONForms: the probe drops --json spellings pflag accepts for a
+// bool and keeps anything else, so `--json=maybe` makes the flag parse fail and the normal path
+// (which refuses it) runs, instead of the probe answering help for an invalid request.
+func TestHelpJSONOnlyStripsValidJSONForms(t *testing.T) {
+	rootOnce.Do(configureRoot)
+	for _, tok := range []string{"--json", "--json=true", "--json=1", "--json=T"} {
+		var out bytes.Buffer
+		if handled, code := helpJSON(infraCmd, []string{"plan", "--help", tok}, &out, &bytes.Buffer{}); !handled || code != 0 {
+			t.Errorf("plan --help %s: handled=%v code=%d, want handled", tok, handled, code)
+		}
+	}
+	for _, tok := range []string{"--json=maybe", "--json=", "--json=yes"} {
+		var out bytes.Buffer
+		if handled, _ := helpJSON(infraCmd, []string{"plan", "--help", "--json", tok}, &out, &bytes.Buffer{}); handled || out.Len() != 0 {
+			t.Errorf("plan --help --json %s: handled=%v stdout=%q, want unhandled and silent", tok, handled, out.String())
+		}
+	}
+}

free/infra/scripts/parity-envelope.py

@@ -5,8 +5,8 @@
   prints its legacy JSON (or nothing on stdout when it fails). A byte diff of stdout can no longer be the check.
   This script checks the head's contract and the documented SURF-29 mapping instead.
 Usage:   parity-envelope.py <sub> <core.out> <head.out> <core.rc> <head.rc>      (sub = provision|list|resize|destroy)
-Checks:  1. head stdout is exactly one JSON document: a v1 envelope (schema_version "1", a command string, exactly one
-            of data or error). Nothing before or after it.
+Checks:  1. head stdout is exactly one JSON document: a v1 envelope (schema_version "1", a command equal to "infra server <sub>",
+            exactly one of data or error). Nothing before or after it.
          2. status: core exit 0 <=> envelope ok and head exit 0. Core exit 1 <=> error envelope whose exit_code equals
             the head exit, the exit_code matches its class (user 1, infra 2, auth 3, destructive_blocked 4, see
             cmd/errcodes.go) and is not 0.
@@ -17,7 +17,9 @@
               resize     core object              == data.action        (data also holds server_id, target_type)
               destroy    SnapshotID/RetainedIPs/ReleasedIPs == snapshot_id/retained_ips/released_ips (null == [];
                          data also holds server_id)
+         Only None (core's null list) is read as an empty list; false, 0, "" and {} are differences.
          The error text is not compared: core prints "Error: ..." on stderr, the head's envelope message is its own.
+         Its type is: error.message must be a string.
 Output:  one line per problem; exit 0 when none, 1 otherwise.
 """
 import json
@@ -26,6 +28,11 @@
 CLASS_EXIT = {"user": 1, "infra": 2, "auth": 3, "destructive_blocked": 4}
 
 
+def none_as_empty(v):
+    """core prints a nil slice as null; that is the only value read as an empty list."""
+    return [] if v is None else v
+
+
 def canon(v):
     return json.dumps(v, sort_keys=True, separators=(",", ":"))
 
@@ -40,22 +47,29 @@ def load_one(path):
     return doc
 
 
-def check_envelope(doc):
+def check_envelope(doc, sub):
     errs = []
     if not isinstance(doc, dict):
         return ["not a JSON object"]
     if doc.get("schema_version") != "1":
         errs.append("schema_version is %r, want '1'" % (doc.get("schema_version"),))
+    want_cmd = "infra server " + sub
     if not isinstance(doc.get("command"), str) or not doc.get("command"):
         errs.append("command missing")
+    elif doc["command"] != want_cmd:
+        errs.append("command is %r, want %r" % (doc["command"], want_cmd))
     if ("data" in doc) == ("error" in doc):
         errs.append("want exactly one of data or error")
     err = doc.get("error")
     if err is not None:
+        if not isinstance(err, dict):
+            return errs + ["error is not an object"]
         if err.get("class") not in CLASS_EXIT or CLASS_EXIT[err.get("class")] != err.get("exit_code"):
             errs.append("exit_code %r does not match class %r" % (err.get("exit_code"), err.get("class")))
         if not isinstance(err.get("code"), str):
             errs.append("error.code missing")
+        if not isinstance(err.get("message"), str):
+            errs.append("error.message is %r, want a string" % (err.get("message"),))
     return errs
 
 
@@ -86,9 +100,9 @@ def compare_data(sub, core, data):
                 errs.append("core action != data.action")
     elif sub == "destroy":
         if need_keys(data, ["server_id", "snapshot_id", "retained_ips", "released_ips"], errs):
-            want = {"snapshot_id": core.get("SnapshotID"), "retained_ips": core.get("RetainedIPs") or [],
-                    "released_ips": core.get("ReleasedIPs") or []}
-            got = {k: (data[k] or [] if k.endswith("_ips") else data[k]) for k in want}
+            want = {"snapshot_id": core.get("SnapshotID"), "retained_ips": none_as_empty(core.get("RetainedIPs")),
+                    "released_ips": none_as_empty(core.get("ReleasedIPs"))}
+            got = {k: (none_as_empty(data[k]) if k.endswith("_ips") else data[k]) for k in want}
             if canon(want) != canon(got):
                 errs.append("core destroy result != data: core=%s data=%s" % (canon(want), canon(got)))
     else:
@@ -103,7 +117,7 @@ def main(argv):
     except ValueError as e:
         print("  head stdout: %s" % e)
         return 1
-    errs = check_envelope(doc)
+    errs = check_envelope(doc, sub)
     if core_rc not in (0, 1):
         errs.append("core exit %d, want 0 or 1" % core_rc)
     ok = "error" not in doc

free/infra/scripts/test_parity_envelope.py

@@ -0,0 +1,64 @@
+#!/usr/bin/env python3
+"""Tests for free/infra/scripts/parity-envelope.py (D-0615).
+
+Run: python3 free/infra/scripts/test_parity_envelope.py  (or pytest). No Docker, no network.
+"""
+import importlib.util
+import os
+import unittest
+
+HERE = os.path.dirname(os.path.abspath(__file__))
+_spec = importlib.util.spec_from_file_location("pe", os.path.join(HERE, "parity-envelope.py"))
+pe = importlib.util.module_from_spec(_spec)
+_spec.loader.exec_module(pe)
+
+
+def err_env(command="infra server destroy", message="refused", **over):
+    e = {"class": "user", "exit_code": 1, "code": "E401", "message": message}
+    e.update(over)
+    return {"schema_version": "1", "command": command, "error": e}
+
+
+class CheckEnvelope(unittest.TestCase):
+    def test_good_error_envelope(self):
+        self.assertEqual(pe.check_envelope(err_env(), "destroy"), [])
+
+    def test_command_must_be_infra_server_sub(self):
+        for cmd in ("infra", "infra plan", "infra server list", "server destroy"):
+            self.assertTrue(pe.check_envelope(err_env(command=cmd), "destroy"), cmd)
+
+    def test_message_must_be_a_string(self):
+        for bad in (None, 7, ["x"], {"a": 1}):
+            errs = pe.check_envelope(err_env(message=bad), "destroy")
+            self.assertTrue(any("error.message" in e for e in errs), bad)
+        e = err_env()
+        del e["error"]["message"]
+        self.assertTrue(any("error.message" in x for x in pe.check_envelope(e, "destroy")))
+
+    def test_error_that_is_not_an_object(self):
+        doc = {"schema_version": "1", "command": "infra server destroy", "error": "boom"}
+        self.assertTrue(pe.check_envelope(doc, "destroy"))
+
+
+class DestroyData(unittest.TestCase):
+    DATA = {"server_id": 1, "snapshot_id": None, "retained_ips": [], "released_ips": []}
+
+    def compare(self, core, **over):
+        data = dict(self.DATA)
+        data.update(over)
+        return pe.compare_data("destroy", core, data)
+
+    def test_null_lists_equal_empty_lists(self):
+        self.assertEqual(self.compare({"SnapshotID": None, "RetainedIPs": None, "ReleasedIPs": None}), [])
+        self.assertEqual(self.compare({"SnapshotID": None, "RetainedIPs": [], "ReleasedIPs": []}, retained_ips=None), [])
+
+    def test_other_empty_values_are_not_an_empty_list(self):
+        for falsy in (False, 0, "", {}):
+            core = {"SnapshotID": None, "RetainedIPs": falsy, "ReleasedIPs": []}
+            self.assertTrue(self.compare(core), "core %r" % (falsy,))
+            core = {"SnapshotID": None, "RetainedIPs": [], "ReleasedIPs": []}
+            self.assertTrue(self.compare(core, released_ips=falsy), "head %r" % (falsy,))
+
+
+if __name__ == "__main__":
+    unittest.main()

free/k8s/cmd/help_json.go

@@ -28,6 +28,7 @@ import (
 	"bytes"
 	"fmt"
 	"io"
+	"strconv"
 	"strings"
 
 	"github.com/nself-org/cli/sdk/go/v2/output"
@@ -57,7 +58,7 @@ func helpJSON(root *cobra.Command, args []string, stdout, stderr io.Writer) (han
 	}
 	rest := make([]string, 0, len(args))
 	for _, a := range args {
-		if a == "--json" || strings.HasPrefix(a, "--json=") {
+		if isJSONToken(a) {
 			continue
 		}
 		rest = append(rest, a)
@@ -101,6 +102,22 @@ func helpJSON(root *cobra.Command, args []string, stdout, stderr io.Writer) (han
 	return true, 0
 }
 
+// isJSONToken reports whether a is a spelling of --json that wantsJSON accepts:
+// the bare flag or --json=<value> with a value strconv.ParseBool takes. Anything
+// else (--json=maybe) stays in the probe's arguments, so the flag parse fails
+// the way the normal run's parse does and the request is left to that path.
+func isJSONToken(a string) bool {
+	if a == "--json" {
+		return true
+	}
+	v, ok := strings.CutPrefix(a, "--json=")
+	if !ok {
+		return false
+	}
+	_, err := strconv.ParseBool(v)
+	return err == nil
+}
+
 // resetFlags puts every flag of c back to its default, so a parse done only to
 // look for --help leaves no trace.
 func resetFlags(c *cobra.Command) {

free/k8s/cmd/help_json_test.go

@@ -111,3 +111,21 @@ func TestHelpFlagValueIsNotHelp(t *testing.T) {
 		t.Fatalf("control: handled=%v code=%d RunE calls=%d", handled, code, *n)
 	}
 }
+
+// TestHelpJSONOnlyStripsValidJSONForms: the probe drops --json spellings pflag accepts for a
+// bool and keeps anything else, so `--json=maybe` makes the flag parse fail and the normal path
+// (which refuses it) runs, instead of the probe answering help for an invalid request.
+func TestHelpJSONOnlyStripsValidJSONForms(t *testing.T) {
+	for _, tok := range []string{"--json", "--json=true", "--json=1", "--json=T"} {
+		var out bytes.Buffer
+		if handled, code := helpJSON(rootCmd, []string{"install", "--help", tok}, &out, &bytes.Buffer{}); !handled || code != 0 {
+			t.Errorf("install --help %s: handled=%v code=%d, want handled", tok, handled, code)
+		}
+	}
+	for _, tok := range []string{"--json=maybe", "--json=", "--json=yes"} {
+		var out bytes.Buffer
+		if handled, _ := helpJSON(rootCmd, []string{"install", "--help", "--json", tok}, &out, &bytes.Buffer{}); handled || out.Len() != 0 {
+			t.Errorf("install --help --json %s: handled=%v stdout=%q, want unhandled and silent", tok, handled, out.String())
+		}
+	}
+}