Skip to content

fix(check): build a nested child's deploy key from its root - #128

Merged
github-actions[bot] merged 1 commit into
mainfrom
fix/bed-deploy-key-local-root
Oct 22, 2026
Merged

github-actions[bot] merged 1 commit into
mainfrom
fix/bed-deploy-key-local-root

Conversation

@aitrawog

Copy link
Copy Markdown
Contributor

Summary

Fixes #127: a bed whose root is not namespaced could not deploy a nested child member. bedAdd handed the whole <root>.<child> roster address to deployKeyForBed, which splits at the FIRST dot. That dot 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.

The mechanism fix converts the ROOT with deployKeyForBed and then appends the child, via one new bedAddChild(root, child, tag) helper used by all five nested-child deploy/redeploy call sites (VM LocalChildKeys, kubevirt ChildKeys, the pod arm's ChildKeys, and both fresh-rebuild redeploy loops). The namespaced-root arm is unchanged: openclaw.check-openclaw-pod + child still yields openclaw/check-openclaw-pod.child.

The missing control named in the issue is closed: the new test asserts the whole argv for a non-namespaced root with a nested child, and the namespaced arm is asserted alongside it.

How tested

Throwaway minimal fixture (not committed), mirroring the failing bed shape: a non-namespaced bed with a nested local child.

repro-bed:
  local:
    from: repro-app
    host: local
    disposable: true
    plan:
      - check: the repro bed plan is reached
        command: "true"
        context: [runtime]
    repro-bed-member:
      local:
        from: repro-app

Pre-fix, the mangled key fails verbatim (real opencharly/layer-containerd manifest, whose only VM bed is check-containerd-vm):

$ charly -C /tmp/w14-repro deploy add 'check-containerd-vm/check-containerd-vm-member'
charly: error: command "deploy": charly deploy add: no <ref> and charly.yml has no entry for "check-containerd-vm/check-containerd-vm-member"
rc=1

Same mechanism on the minimal fixture:

$ charly -C /tmp/w14-repro-min deploy add 'repro-bed/repro-bed-member'
charly: error: command "deploy": charly deploy add: no <ref> and charly.yml has no entry for "repro-bed/repro-bed-member"
rc=1

The corrected dotted key resolves live at the deploy resolver:

$ charly -C /tmp/w14-repro-min deploy add 'repro-bed.repro-bed-member' --dry-run
charly: resolving project (first run — may take a moment)...
...
rc=0

The new test fails against the pre-fix mechanism, and the namespaced arm stays green (mutant = bedAddChild reverted to bedAdd([]string{root + "." + child}, tag); the mutant line was present in the file before this run):

=== RUN   TestBedAddChildConvertsTheRootBeforeAppendingTheChild
=== RUN   TestBedAddChildConvertsTheRootBeforeAppendingTheChild/a_non-namespaced_root_keeps_the_dotted_child_path
    bed_container_ref_test.go:133: bedAddChild(non-namespaced root) = ["deploy" "add" "check-containerd-vm/check-containerd-vm-member" "--dev-local-pkg" "--tag" "2026.295.1200"], want ["deploy" "add" "check-containerd-vm.check-containerd-vm-member" "--dev-local-pkg" "--tag" "2026.295.1200"] — `check-containerd-vm/check-containerd-vm-member` is a deploy key nothing creates
=== RUN   TestBedAddChildConvertsTheRootBeforeAppendingTheChild/a_namespaced_root_still_converts_at_the_namespace_boundary
--- FAIL: TestBedAddChildConvertsTheRootBeforeAppendingTheChild (0.00s)
    --- FAIL: TestBedAddChildConvertsTheRootBeforeAppendingTheChild/a_non-namespaced_root_keeps_the_dotted_child_path (0.00s)
    --- PASS: TestBedAddChildConvertsTheRootBeforeAppendingTheChild/a_namespaced_root_still_converts_at_the_namespace_boundary (0.00s)
FAIL

Post-fix, both arms pass:

$ go test . -run 'TestBedAddChildConvertsTheRootBeforeAppendingTheChild|TestDeployKeyForBed' -count=1 -v
--- PASS: TestBedAddChildConvertsTheRootBeforeAppendingTheChild (0.00s)
    --- PASS: TestBedAddChildConvertsTheRootBeforeAppendingTheChild/a_non-namespaced_root_keeps_the_dotted_child_path (0.00s)
    --- PASS: TestBedAddChildConvertsTheRootBeforeAppendingTheChild/a_namespaced_root_still_converts_at_the_namespace_boundary (0.00s)
--- PASS: TestDeployKeyForBed (0.00s)
    --- PASS: TestDeployKeyForBed/nested_child (0.00s)
    --- PASS: TestDeployKeyForBed/leading_dot (0.00s)
    --- PASS: TestDeployKeyForBed/trailing_dot (0.00s)
    --- PASS: TestDeployKeyForBed/empty (0.00s)
--- PASS: TestDeployKeyForBed_ContractWithSpec (0.00s)
PASS
ok  	github.com/opencharly/plugin-check/candy/plugin-check	0.039s

Full gate on the committed tree (git status --short empty before the run):

# head 1953fb2427b4537db11c3db16c75e4cacb01d1c8 (git rev-parse HEAD)
# tree e426187b8189df5b6ec5560eb556197b1ad1174a (git rev-parse HEAD^{tree})
# module .../w14-plugin-check-bedkey/plugin-check/candy/plugin-check
# git status --short (repo root): (empty)
## gofmt -l .
gofmt rc=0
## go build ./...
build rc=0
## go vet ./...
vet rc=0
## go test ./... -count=1
ok  	github.com/opencharly/plugin-check/candy/plugin-check	16.426s
?   	github.com/opencharly/plugin-check/candy/plugin-check/cmd/serve	[no test files]
test rc=0

Repo structural gate and the repo's own disposable bed (run on the same tree content that became this commit):

$ charly -C <worktree> box validate
charly box validate: OK — checked 1 candy, 4 deploys, 7 distros, 7 builders; 0 warnings, 0 errors
rc=0

$ charly -C <worktree> check run check-dotted-local-resolve
charly check run check-dotted-local-resolve: PASS (steps=6)
summary.yml: warnings: 0 (the one allowlisted finding per step is the dev-worktree binary notice)

Scope of the live proof, stated honestly: the host charly binary used for the bed exercises its baked command:check plugin, not the changed source. So the bed is the repo's structural smoke, not proof of this diff. The diff's proof is the mutant-killed argv test plus the live deploy-resolver run above (mangled key rejected, corrected key accepted). A fresh-rebuild R10 bed that links this plugin source is the check-local witness in opencharly/charly, which this repo's AGENTS.md names and which is not part of this bounded PR.

Namespaced arm: green post-fix, and green against the mutant too (which is exactly why the pre-fix test suite missed the defect).

Branch diff (origin/main...HEAD): 3 files, +78/−17.

Rulebook compliance

  • R0 — loaded git-workflow, go, strict-policy, root-cause-analyzer, check, plugin, and agents before any code/git action; read the repo's AGENTS.md.
  • R1 — the failure was reproduced BEFORE remediation (verbatim above). Every finding here is fixed in this change; none is filed or deferred. The missed control named in the issue is added in TestBedAddChildConvertsTheRootBeforeAppendingTheChild, and the test is proven able to fail with a mutation that only reverts the mechanism.
  • R2 — one conversion path: bedAddChild builds the child ref by converting the root and appending, and all five nested-child call sites use it; bedAdd and bedAddChild share one argv assembler (bedAddResolved). No sibling implementation is left behind.
  • R3 — no sleep, retry, suppression, or alternate path; the resolver error is surfaced as before.
  • R4 — no doc change was needed; the code and its comments now state the root-only contract.
  • R5 — no legacy name/path removed. The retired call shape is proven gone: git grep -nF 'bedAdd([]string{name + "." + childKey}' HEAD → no output, rc=1; the five replacements are present (git grep -n 'bedAddChild(name, childKey' HEAD | wc -l → 5). The remaining name+"."+childKey is deploy del, whose dotted-path resolver is the correct address for it.
  • R6 — work in .worktrees/w14-plugin-check-bedkey/plugin-check off fresh origin/main; git status read before staging; no force-push, no hook bypass, no submodule edit, no push to main.
  • R7 — the new test fails without the change (mutant output above) and passes with it; the changed path's real boundary (the deploy resolver) was executed live with both the wrong and the right key.
  • R8 — no external live service is mocked; not applicable beyond the live resolver run.
  • R9 — no charly binary rebuild was required or claimed; no runtime OS dependency added.
  • R10 — gate re-run on committed tree 1953fb2 / tree e426187; gofmt clean, build/vet/test green, box validate 0 warnings, repo bed PASS with 0 warnings. One cutover, one commit.

Change classification

Plugin-library Go source + tests (command:check bed runner). Not a schema, pin, gitlink, docs, or runtime OS-dependency change. Class gate: module go build/go vet/gofmt/go test, repo charly box validate, and the repo's host-local disposable bed.

Filed issues

None — no finding was separable from this change. (The brief's sibling plugin-check#100 remains a separate, already-filed defect at a different call site and is not touched here.)

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)

bedAdd converted the whole `<root>.<child>` 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)*
@github-actions

Copy link
Copy Markdown

Review — PASS

What the change does

The diff converts the nested-child deploy ref correctly: bedAddChild(root, child, tag) runs deployKeyForBed(root) on the root alone and then appends "." + child, instead of handing the whole <root>.<child> address to bedAdd (which splits at the first dot and mangles a non-namespaced root into <root>/<child>).

I traced both arms against deployKeyForBed:

  • Non-namespaced: deployKeyForBed("check-containerd-vm") → no dot → unchanged → check-containerd-vm.check-containerd-vm-member. Correct.
  • Namespaced: deployKeyForBed("openclaw.check-openclaw-pod") → spec.DeployKey("openclaw","check-openclaw-pod") → openclaw/check-openclaw-pod → openclaw/check-openclaw-pod.child. Correct, and identical to the old bedAdd result for namespaced roots (so no behavior regression on that arm — which is exactly why the pre-fix suite missed the defect).

A child containing its own dot is also handled correctly in both arms (<converted-root>.<child>), matching the prior namespaced output and fixing the non-namespaced case.

Tests

  • TestBedAddChildConvertsTheRootBeforeAppendingTheChild asserts the whole argv for both arms, including tag ordering (--dev-local-pkg then --tag), matching bedAddResolved → withRunTag. This is a genuine regression guard: reverting bedAddChild to bedAdd([]string{root+"."+child}, tag) produces the buggy check-containerd-vm/check-containerd-vm-member and fails the test, as the body's mutant output shows (and the want/got are consistent with the test code).
  • TestBedAddConvertsTheRosterAddress is updated to exercise the new path while preserving its assertion.
  • TestBedDeployCarriesTheRunTag "vm child"/"local child" rows now use bedAddChild, which is the corrected construction; the old rows were built through the buggy bedAdd path. Consistent.

Scope & cleanliness

  • All five nested-child call sites (LocalChildKeys deploy, kubevirt ChildKeys, pod-arm ChildKeys, and both fresh-rebuild redeploy loops) are converted to bedAddChild; no leftover bedAdd([]string{name+"."+childKey}, ...) remains in the diff.
  • bedAdd and bedAddChild share one assembler (bedAddResolved) — no duplicated argv logic, no dead code. The append(args, "--dev-local-pkg") aliasing shape is unchanged from the pre-existing code (not worsened).
  • Body claims are internally consistent: 3 files, +78/−17 matches the per-file counts (37+2+39 / 3+2+12). The body is honest about the limits of the live bed proof (host binary uses its baked plugin, not this source) and relies on the mutant-killed argv test plus the live deploy-resolver run for the actual diff proof. No contradiction.
  • The untouched deploy del dotted-path call is explicitly acknowledged and is a different resolver; out of scope here.

Comments / sign-off

There are zero comment threads, so no maintainer sign-off exists and none is claimed. The change stands on its own evidence: a test that fails without the mechanism change and passes with it, plus a coherent, self-consistent refactor. No secrets, injection, new dependencies, or privilege changes.

No blocking findings.

Verdict: PASS


ollama-cloud/deepseek-v4.1-flash — action-review.

View action run

@github-actions
github-actions Bot enabled auto-merge (squash) October 22, 2026 11:58
@github-actions
github-actions Bot merged commit ff06d8e into main Oct 22, 2026
1 check passed
charly-auto-merge Bot added a commit that referenced this pull request Oct 22, 2026
@aitrawog

Copy link
Copy Markdown
Contributor Author

Post-merge finding filed: opencharly/.github#223 — this PR carried a plain Closes #127, but #127 did not auto-close on merge (its timeline has no closed event before the manual one). Remediation was the explicit RESOLVED comment + close on #127. The PR body is frozen after merge, so the filed issue is recorded here.

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)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

a VM bed's nested kind:local child cannot deploy: bedAdd rewrites <root>.<child> to <root>/<child>

2 participants