From 1953fb2427b4537db11c3db16c75e4cacb01d1c8 Mon Sep 17 00:00:00 2001 From: Andreas Trawoeger Date: Thu, 22 Oct 2026 13:55:21 +0200 Subject: [PATCH] fix(check): build a nested child's deploy key from its root MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit bedAdd converted the whole `.` address through deployKeyForBed, which splits at the FIRST dot. That is the namespace boundary only for a namespaced root; for a non-namespaced root it is the root/child boundary, so `check-containerd-vm.check-containerd-vm-member` became the nonexistent key `check-containerd-vm/check-containerd-vm-member` and the bed failed at `deploy add` before any check ran (#127). Convert the root with deployKeyForBed and then append the child, and use that path at all five nested-child deploy/redeploy call sites. Add the missing local-root-with-nested-child test, asserting the whole argv; the namespaced-root arm is asserted unchanged. Closes #127 *Agent: `containerd-crabbox-program/w14` · session `8b1c01cf-7f79-4a75-bff8-240415a06208`* *Assisted-by: Claude Code deepseek-v4.1-flash:cloud (analysed on a live system)* --- candy/plugin-check/bed_container_ref_test.go | 40 +++++++++++++-- candy/plugin-check/bed_deploy_tag_test.go | 4 +- candy/plugin-check/bed_run.go | 51 +++++++++++++++----- 3 files changed, 78 insertions(+), 17 deletions(-) diff --git a/candy/plugin-check/bed_container_ref_test.go b/candy/plugin-check/bed_container_ref_test.go index fb29815..3965a62 100644 --- a/candy/plugin-check/bed_container_ref_test.go +++ b/candy/plugin-check/bed_container_ref_test.go @@ -2,6 +2,7 @@ package check import ( "context" + "slices" "testing" "github.com/opencharly/sdk/kit" @@ -84,10 +85,10 @@ func TestBedAddConvertsTheRosterAddress(t *testing.T) { } }) - t.Run("a nested child keeps its tail", func(t *testing.T) { - got := bedAdd([]string{"openclaw.check-openclaw-pod.child"}, "") + t.Run("a namespaced nested child keeps its tail", func(t *testing.T) { + got := bedAddChild("openclaw.check-openclaw-pod", "child", "") if got[2] != "openclaw/check-openclaw-pod.child" { - t.Fatalf("bedAdd child ref = %q, want openclaw/check-openclaw-pod.child", got[2]) + t.Fatalf("bedAddChild child ref = %q, want openclaw/check-openclaw-pod.child", got[2]) } }) @@ -111,3 +112,36 @@ func TestBedAddConvertsTheRosterAddress(t *testing.T) { } }) } + +// TestBedAddChildConvertsTheRootBeforeAppendingTheChild pins the nested-child ref shape. +// +// A child reference is `.`. deployKeyForBed splits at the FIRST dot, which is the +// namespace boundary only for a NAMESPACED root. Passing the whole dotted address through bedAdd +// rewrites a non-namespaced root's child as `/` — a deploy key nothing creates — so +// the bed fails at `deploy add` before any check runs (plugin-check#127). The conversion must +// happen on the ROOT alone, and the child is appended to the resulting key. +func TestBedAddChildConvertsTheRootBeforeAppendingTheChild(t *testing.T) { + const tag = "2026.295.1200" + + t.Run("a non-namespaced root keeps the dotted child path", func(t *testing.T) { + got := bedAddChild("check-containerd-vm", "check-containerd-vm-member", tag) + want := []string{ + "deploy", "add", "check-containerd-vm.check-containerd-vm-member", + "--dev-local-pkg", "--tag", tag, + } + if !slices.Equal(got, want) { + t.Fatalf("bedAddChild(non-namespaced root) = %q, want %q — `check-containerd-vm/check-containerd-vm-member` is a deploy key nothing creates", got, want) + } + }) + + t.Run("a namespaced root still converts at the namespace boundary", func(t *testing.T) { + got := bedAddChild("openclaw.check-openclaw-pod", "child", "") + want := []string{ + "deploy", "add", "openclaw/check-openclaw-pod.child", + "--dev-local-pkg", + } + if !slices.Equal(got, want) { + t.Fatalf("bedAddChild(namespaced root) = %q, want %q", got, want) + } + }) +} diff --git a/candy/plugin-check/bed_deploy_tag_test.go b/candy/plugin-check/bed_deploy_tag_test.go index 1f83308..d42b06f 100644 --- a/candy/plugin-check/bed_deploy_tag_test.go +++ b/candy/plugin-check/bed_deploy_tag_test.go @@ -21,8 +21,8 @@ func TestBedDeployCarriesTheRunTag(t *testing.T) { // arm, and the pod arm's positional form. argvs := map[string][]string{ "vm root": bedAdd([]string{"check-arch-vm", "arch"}, tag), - "vm child": bedAdd([]string{"check-arch-vm.arch-host"}, tag), - "local child": bedAdd([]string{"check-crabbox-e2e-pod.child"}, tag), + "vm child": bedAddChild("check-arch-vm", "arch-host", tag), + "local child": bedAddChild("check-crabbox-e2e-pod", "child", tag), "pod arm": append(bedAdd([]string{"check-crabbox-pod", "crabbox"}, tag), "--node-only"), } for name, argv := range argvs { diff --git a/candy/plugin-check/bed_run.go b/candy/plugin-check/bed_run.go index 2e9f99c..cac79aa 100644 --- a/candy/plugin-check/bed_run.go +++ b/candy/plugin-check/bed_run.go @@ -200,13 +200,17 @@ func withRunTag(args []string, tag string) []string { // A bed exists to prove the in-development package builds and installs, so on a bed that condition // must be loud. func bedAdd(refs []string, tag string) []string { - // The ref is a ROSTER address ("openclaw.check-openclaw-pod", or a child "ns.box.child") and the deploy - // side needs the DEPLOY KEY, for the same reason configStartArgs converts before config/start: a roster - // address does not resolve from the project root, and the deploy resolver then derives a bare NAMESPACE - // as its requested box -- `unknown box "openclaw"` from sdk/deploykit/graph.go's FilterBox. Measured on + // The ref is a ROSTER address ("openclaw.check-openclaw-pod") and the deploy side needs the DEPLOY KEY, + // for the same reason configStartArgs converts before config/start: a roster address does not resolve + // from the project root, and the deploy resolver then derives a bare NAMESPACE as its requested box -- + // `unknown box "openclaw"` from sdk/deploykit/graph.go's FilterBox. Measured on // openclaw.check-openclaw-pod's deploy-add (opencharly/plugin-build#28). Converting HERE rather than at - // each call site is R2's answer: eight callers hand this function a roster address, and one conversion - // applied in one place is what stops the ninth caller from missing it. + // each call site is R2's answer: every caller holding only a roster address hands it here, and one + // conversion applied in one place is what stops the next caller from missing it. + // + // A NESTED CHILD is the one ref this function must not receive whole: deployKeyForBed splits at the + // first dot, which is the namespace boundary only for a namespaced root. Use bedAddChild, which + // converts the ROOT and then appends the child. // // Remote refs are left alone: spec.ResolveBoxName resolves those by parsing, not by namespace splitting, // so the roster conversion would mangle them. @@ -214,7 +218,26 @@ func bedAdd(refs []string, tag string) []string { if len(args) > 0 && !isRemoteImageRef(args[0]) { args = append([]string{deployKeyForBed(args[0])}, args[1:]...) } - return withRunTag(append([]string{"deploy", "add"}, append(args, "--dev-local-pkg")...), tag) + return bedAddResolved(args, tag) +} + +// bedAddResolved builds the `charly deploy add` argv from refs whose first element is ALREADY a +// DEPLOY KEY. bedAdd is the roster-address entry point; this is the one the child path uses +// after converting the ROOT itself. +func bedAddResolved(refs []string, tag string) []string { + return withRunTag(append([]string{"deploy", "add"}, append(refs, "--dev-local-pkg")...), tag) +} + +// bedAddChild builds the `charly deploy add` argv for a NESTED child of a bed. +// +// The conversion must happen on the ROOT alone, BEFORE the child is appended. A child reference +// is `.`, and deployKeyForBed splits at the FIRST dot: for a namespaced root +// (`openclaw.check-openclaw-pod`) that dot is the namespace boundary, but for a non-namespaced +// root (`check-containerd-vm`) it is the root/child boundary. Handing the whole dotted address +// to bedAdd therefore turns a non-namespaced child into `/` — a deploy key nothing +// creates — and `charly deploy add` fails before any check runs (plugin-check#127). +func bedAddChild(root, child, tag string) []string { + return bedAddResolved([]string{deployKeyForBed(root) + "." + child}, tag) } // deployKeyForBed maps a bed's ROSTER address to the DEPLOY KEY the deploy phase creates. @@ -227,6 +250,10 @@ func bedAdd(refs []string, tag string) []string { // earlier for `charly box build`: a name that does not resolve from the project root. The // conversion is `spec.DeployKey`'s own, reused rather than re-spelled (R3), and a local bed // with no namespace is returned unchanged. +// +// This function converts a ROOT address only. Never hand it an already-dotted `.` +// whose root is non-namespaced: it would treat the root/child boundary as a namespace boundary. +// Nested child refs are built by bedAddChild. func deployKeyForBed(name string) string { if ns, rest, ok := strings.Cut(name, "."); ok && ns != "" && rest != "" { return spec.DeployKey(ns, rest) @@ -1100,7 +1127,7 @@ func runCheckBed(ctx context.Context, ex *sdk.Executor, name string, opts bedRun // host-resolved deployNestedLocalChildren subset). A VM's nested CONTAINER children are // deployed IN-GUEST by plugin-deploy-vm's PostApply, so a host-side re-deploy would be wrong. for _, childKey := range d.LocalChildKeys { - if err := step("deploy-"+childKey, bedAdd([]string{name + "." + childKey}, d.ImageTag)...); err != nil { + if err := step("deploy-"+childKey, bedAddChild(name, childKey, d.ImageTag)...); err != nil { return fail("deploy nested local child %s.%s: %w", name, childKey, err) } } @@ -1125,7 +1152,7 @@ func runCheckBed(ctx context.Context, ex *sdk.Executor, name string, opts bedRun // Nested HOST-ROOTED (kind:local) children deploy host-side after the root; // the kubevirt plugin's own PostApply handles in-guest children. for _, childKey := range d.ChildKeys { - if err := step("deploy-"+childKey, bedAdd([]string{name + "." + childKey}, d.ImageTag)...); err != nil { + if err := step("deploy-"+childKey, bedAddChild(name, childKey, d.ImageTag)...); err != nil { return fail("deploy nested child %s.%s: %w", name, childKey, err) } } @@ -1169,7 +1196,7 @@ func runCheckBed(ctx context.Context, ex *sdk.Executor, name string, opts bedRun } // Deploy any nested children onto the started substrate, pre-order. for _, childKey := range d.ChildKeys { - if err := step("deploy-"+childKey, bedAdd([]string{name + "." + childKey}, d.ImageTag)...); err != nil { + if err := step("deploy-"+childKey, bedAddChild(name, childKey, d.ImageTag)...); err != nil { return fail("deploy nested child %s.%s: %w", name, childKey, err) } } @@ -1370,13 +1397,13 @@ func runCheckBed(ctx context.Context, ex *sdk.Executor, name string, opts bedRun } if d.IsVM { for _, childKey := range d.LocalChildKeys { - if err := step("redeploy-"+childKey, bedAdd([]string{name + "." + childKey}, d.ImageTag)...); err != nil { + if err := step("redeploy-"+childKey, bedAddChild(name, childKey, d.ImageTag)...); err != nil { return fail("re-deploy nested local child %s.%s (fresh rebuild): %w", name, childKey, err) } } } else { for _, childKey := range d.ChildKeys { - if err := step("redeploy-"+childKey, bedAdd([]string{name + "." + childKey}, d.ImageTag)...); err != nil { + if err := step("redeploy-"+childKey, bedAddChild(name, childKey, d.ImageTag)...); err != nil { return fail("re-deploy nested child %s.%s (fresh rebuild): %w", name, childKey, err) } }