test(occdev): pin the k3d import retry at both call sites and selector value quoting

[freeqaz]

Problem

A mutation sweep of #1572 (retry a direct k3d image import that lost its stream) and #1263 (quote the bootstrap preparation namespace) left real gaps. The retry loop in importArchiveDirect is well pinned on its own (16 of 25 mutants killed), but nothing ran it through its two callers:

• Reverting importDevelopmentImage or importOpenShellImage to the old one-shot k3d image import survived.
• Dropping either archive cleanup survived, and so did returning from the development import before the ctr images list digest lookup or importing by the requested name instead of the engine-recorded one. importDevelopmentImage's staged path had no test at all.
• Creating the output buffer once instead of per attempt survived. Then a second attempt that fails for another reason is retried, because the first attempt's closed-stream text is still in the buffer.
• Dropping k3d's stdout survived.

For #1263, the namespace and key quoting are pinned, but an unquoted node selector value (for example yes or 010) survived.

Change

Test-only.

• internal/occdev/k3d_import_test.go:
  • A closed stream followed by a ctr failure is retried once and then fails, with one retry notice.
  • A new closed-stream retry test for importDevelopmentImage (staged digest image, Podman). Before this, its staged path had no test at all.
• internal/occdev/openshell_k3d_profile_test.go: in the existing TestOpenShellImageImportRegistersThePodmanRecordedName, the first import now loses its stream.
• Both call-site tests use flakyK3dImportCase, whose fake k3d checks that the archive still exists on every attempt. They check:
  • two imports of the same archive;
  • the image lookup only after the successful import;
  • the digest reference returned;
  • the retry notice and k3d's stdout forwarded;
  • the archive removed afterwards.
• tests/integration/bootstrap-volume.test.mjs: the YAML-scalar node selector test now uses each key as its own value, so an unquoted value fails as well as an unquoted key.

Verification

• Mutation sweep (go test ./internal/occdev/ -count=1 per mutant): fix(occdev): retry a direct k3d image import that lost its stream #1572 went from 16/25 to 24/25 killed. The survivor drops strings.ToLower from the closed-stream matcher. It is equivalent for k3d's real output, which carries every phrase in lower case on its second line.
• fix(deploy): quote bootstrap preparation namespace #1263 script mutants went from 2/4 to 3/4. The survivor single-quotes the namespace, which is equivalent because --namespace must be a DNS label.
• test(queue): complete unordered expected work in any claim order #1571's completeExpected was driven against fake queues: it fails when a key is never claimed (queue drained, queue empty, 200 unrelated claims, duplicate claim). Four of five helper mutants are caught. The survivor returns early once every key has been claimed, which is equivalent.
• A review found nits, applied in the second commit. The mutation results are unchanged.
• Pinned 20/20 on the final head at 2 CPUs (taskset -c 0-1, GOMAXPROCS=2), each round gated on a 1-minute load under 40: go test ./internal/occdev/ -count=1 -p 1 and node --test --test-concurrency=2 tests/integration/bootstrap-volume.test.mjs (11/11).
• go vet ./internal/occdev/, gofmt -l, and prettier --check on the touched files are clean.

No new conflicts with open PRs that touch internal/occdev/ or the bootstrap volume files (git merge-tree --write-tree --name-only <pr-head> <base>):

PR	head	vs origin/main	vs this branch
#1557	2f2d8b41f	clean	clean
#1543	a6bfbc985	clean	clean
#1537	68d745246	clean	clean
#1501	187cba5e4	clean	clean
#1273	99f6a4602	clean	clean
#1138	f40438227	5 conflicted files	same 5 files, no new file
#1137	a3a64a1d7	4 conflicted files	same 4 files, no new file
#519	6ee4c77e1	19 conflicted files	same 19 files, no new file
#302	e6837de6c	22 conflicted files	same 22 files, no new file

Risks

Test-only; the new Go tests add well under a second to internal/occdev. The new fakes match exact command lines, so a future change to the import's arguments has to update them. That is the intent.

[clawsweeper]

🦞👀
ClawSweeper picked this up.

Pull request received. I will update this pull request when review starts.

ClawSweeper review complete

ClawSweeper finished reviewing this revision. The review result is being finalized.

View the workflow run.

[clawsweeper]

Codex review: needs maintainer review before merge. Reviewed October 6, 2026, 11:23 PM ET / October 7, 2026, 03:23 UTC (Revision 2).

ClawSweeper review

What this changes

Adds tests for both local Kubernetes image-import callers, retry error isolation, archive cleanup and digest verification, and bootstrap node selector values that resemble YAML scalars.

Merge readiness

✅ Ready for maintainer review

Keep open: these focused regression tests remain absent from current main and add useful coverage beyond the merged fixes. No actionable correctness or security findings remain.

Priority: P3
Reviewed head: 9f360992eaa30053730c02c27616936a8ed1bc9c

Review scores

Measure	Result	What it means
Overall readiness	🐚 platinum hermit (4/6)	Focused, useful regression coverage with no actionable findings and clearly bounded fixture evidence.
Proof confidence	🌊 off-meta tidepool	Not applicable: Verified maintainer permissions exempt this test-only PR from the external-contributor proof gate. The actual import owners and bootstrap script are exercised with command fixtures; the reported mutation and repeated-run results do not establish live k3d transport or Kubernetes execution. No stored-data contract changes.
Patch quality	🐚 platinum hermit (4/6)	No actionable review findings were identified.

Verification

Check	Result	Evidence
Real behavior	Not applicable	Not applicable: Verified maintainer permissions exempt this test-only PR from the external-contributor proof gate. The actual import owners and bootstrap script are exercised with command fixtures; the reported mutation and repeated-run results do not establish live k3d transport or Kubernetes execution. No stored-data contract changes.
Evidence reviewed	9 items	Pinned introduced change: The merge-base-to-head delta changes only three test files: 127 added and 10 removed lines. No production code, dependency, workflow, configuration or persisted format changes. Observable import regressions: The added cases invoke the actual import owners with command fixtures. They distinguish a later non-stream failure from the initial closed stream, require the archive to exist during each attempt, and assert retry output, digest lookup ordering, returned reference and final archive removal. Existing tests already cover the attempt bound, cancellation and other initial failures. Independent manifest assertion: The selector case runs the production bootstrap script and parses its emitted manifest through the existing Kubernetes client YAML loader; the fake kubectl only captures that manifest. Using six scalar-like strings as both keys and values strengthens the existing assertion without inventing renderer behavior or claiming live scheduling proof.
Findings	None	None.
Security	None	None.

How this fits together

The development launcher exports local container images, imports them into k3d, and verifies their immutable references before deployment. Separately, the production bootstrap helper renders a preparation Pod from operator-supplied namespace and node selector settings.

flowchart TD
  A[Local container images] --> B[Development image export]
  B --> C[Bounded k3d import retry]
  C --> D[Digest verification and archive cleanup]
  D --> E[Deployment image reference]
  F[Operator bootstrap settings] --> G[Preparation Pod manifest]
  G --> H[Kubernetes volume preparation]

Loading

Before merge

None.

Agent review details

Security

None.

Review metrics

Metric	Value	Why it matters
Production versus test LOC	production +0/-0; tests +127/-10	All growth strengthens existing test boundaries without expanding the production surface.

Technical review

Best possible solution:

Retain these focused regressions alongside the existing retry and bootstrap serialization owners.

Do we have a high-confidence way to reproduce the issue?

Not applicable: this PR strengthens regression coverage rather than reporting a remaining product defect; tests were inspected but not executed in this read-only review.

Is this the best way to solve the issue?

Yes: exercising the real import callers and extending the existing parsed-manifest assertion is a focused way to protect the uncovered behavior.

AGENTS.md: found and applied where relevant.

Codex review notes: model internal, reasoning medium; reviewed against 45b6d2421f1f.

Labels

Label changes:

No label changes.

Label justifications:

• P3: This is bounded test hardening for already implemented behavior, with no production behavior change.
• rating: 🐚 platinum hermit: Overall readiness is 🐚 platinum hermit; proof is 🌊 off-meta tidepool and patch quality is 🐚 platinum hermit.
• status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Not applicable: Verified maintainer permissions exempt this test-only PR from the external-contributor proof gate. The actual import owners and bootstrap script are exercised with command fixtures; the reported mutation and repeated-run results do not establish live k3d transport or Kubernetes execution. No stored-data contract changes.

Evidence

What I checked:

• Pinned introduced change: The merge-base-to-head delta changes only three test files: 127 added and 10 removed lines. No production code, dependency, workflow, configuration or persisted format changes. (9f360992eaa3)
• Observable import regressions: The added cases invoke the actual import owners with command fixtures. They distinguish a later non-stream failure from the initial closed stream, require the archive to exist during each attempt, and assert retry output, digest lookup ordering, returned reference and final archive removal. Existing tests already cover the attempt bound, cancellation and other initial failures. (internal/occdev/k3d_import_test.go:222, 9f360992eaa3)
• Independent manifest assertion: The selector case runs the production bootstrap script and parses its emitted manifest through the existing Kubernetes client YAML loader; the fake kubectl only captures that manifest. Using six scalar-like strings as both keys and values strengthens the existing assertion without inventing renderer behavior or claiming live scheduling proof. (tests/integration/bootstrap-volume.test.mjs:401, 9f360992eaa3)
• Still necessary on current main: Inspection of the fetched main test blobs shows only the earlier helper-level retry tests and selector-key assertion. The changed test paths have no delta between the pinned merge base and fetched main, so this PR's additional coverage remains unique. GitHub reports no published releases; no shipped-release claim is made. (internal/occdev/k3d_import_test.go:1, 45b6d2421f1f)
• Merged behavior and routing history: GitHub verifies fix(occdev): retry a direct k3d image import that lost its stream #1572 merged the retry owner into main. Current-main path logs associate freeqaz with the retry implementation and Eric Cai with bootstrap scalar quoting. Some historical blobs needed by local follow/blame inspection were unavailable; GitHub commit patches supplied the relevant history context, without establishing source-line introduction attribution. (internal/occdev/k3d_import.go:26, a2e160b99a64)
• Bootstrap history: The GitHub commit patch records Eric Cai's selector-key quoting change, also included in merged fix(deploy): quote bootstrap preparation namespace #1263. This PR strengthens coverage of the already quoted values rather than replacing that fix. (scripts/prepare-bootstrap-volume:162, b9d57a348084)

Likely related people:

• freeqaz: Suggested for follow-up; no historical authorship or introduction is verified. (role: unverified routing candidate; confidence: low)
• ericcaiwx-star: Suggested for follow-up; no historical authorship or introduction is verified. (role: unverified routing candidate; confidence: low)

Rating scale

Score	Internal tier	Crab rank	Meaning
6/6	S	🦀 challenger crab	Exceptional readiness
5/6	A	🦞 diamond lobster	Very strong readiness
4/6	B	🐚 platinum hermit	Good normal PR; ordinary maintainer review
3/6	C	🦐 gold shrimp	Useful, but confidence is limited
2/6	D	🦪 silver shellfish	Proof or implementation needs work
1/6	F	🧂 unranked krab	Not merge-ready
N/A	NA	🌊 off-meta tidepool	Rating does not apply

Overall follows the weaker of proof and patch quality.
Shiny media proof means a screenshot, video, or linked artifact directly shows the changed behavior. Runtime, network, CSP, and security claims still need visible diagnostics.

Workflow

• ClawSweeper keeps one durable marker-backed review comment per issue or PR.
• Re-runs edit this comment so the latest verdict, findings, and automation markers stay together instead of adding duplicate bot comments.
• A fresh review can be triggered by eligible @clawsweeper re-review comments, exact-item GitHub events, scheduled/background review runs, or manual workflow dispatch.
• PR/issue authors and users with repository write access can comment @clawsweeper re-review or @clawsweeper re-run on an open PR or issue to request a fresh review only.
• Maintainers can also comment @clawsweeper review to request a fresh review only.
• Fresh-review commands do not start repair, autofix, rebase, CI repair, or automerge.
• Maintainer-only repair and merge flows require explicit commands such as @clawsweeper autofix, @clawsweeper automerge, @clawsweeper fix ci, or @clawsweeper address review.
• Maintainers can comment @clawsweeper explain to ask for more context, or @clawsweeper stop to stop active automation.

History

Review history (1 earlier review cycle)

• reviewed 2026-10-07T03:17:46.638Z sha e6b557a :: needs maintainer review before merge. :: none