[freeqaz]

Problem

Image references were checked by five different rules (profile preflight, Compute, the chart's images.*/repositoryCredentials.image, the chart's logging.collector.image, and the bash tools). None matched Kubernetes:

• All of them accepted references Kubernetes refuses as InvalidImageName at pull time: .., a leading hyphen, a trailing or triple underscore or an empty component in the path; a host with a trailing dot; a tag over 128 characters or starting with .; a repository path over 255 characters; a tag followed by more path.
• runtime.image, the Collector and Compute also accepted non-ASCII letters (including non-ASCII uppercase such as É and the Kelvin sign), zero-width characters and NBSP; Compute alone accepted two digests, a leading space, @ in the path, an IPv6 zone ID and a host starting with a hyphen.
• IPv6 registry hosts ([::1]:5000/img@sha256:..., which Kubernetes accepts) were refused by images.*, controllerImage, repository.image and both bash tools, but accepted by the Collector, runtime.image and Compute.

Change

• packages/occ/src/image-reference.ts: imageReferenceProblem(image), a port of distribution/reference v0.6.0 ParseNormalizedNamed (Docker Hub normalization, the lowercase repository check, the reference grammar, the 255-character path limit, the bare image ID refusal) plus go-digest v1.0.0 digest validation. It returns uppercase-repository, invalid, or nothing.
• Profile preflight uses it for controlPlane.controllerImage, runtime.image and repository.image; each still requires a lowercase @sha256: digest.
• Compute uses it for the gateway and Agent images, with or without requireImmutableDigest.
• prepare-bootstrap-volume and upgrade-production-images call it through scripts/lib/image-reference-problem.mjs. prepare-bootstrap-volume now quotes the image in its Pod manifest, so a bracketed IPv6 host stays a string.
• The chart cannot share JS, so openclaw.validateImageReference in _helpers.tpl applies the same steps with Helm's Go regexp and lower, for images.*, repositoryCredentials.image and logging.collector.image. IPv6 registry hosts are now accepted everywhere.
• Error messages are unchanged: ... repository path must be lowercase for an uppercase repository, the field's existing immutable-image message otherwise; Compute adds ... image must be a container image reference Kubernetes can pull., which API and worker startup report as RUNTIME_IMAGE_INVALID (covered in configuration-startup.test.mjs).
• upgrade-production-images fingerprints the rule and its helper in parameters.json.
• tests/fixtures/image-reference-cases.mjs: 66 cases with Kubernetes' verdict, used by new table tests in the preflight/chart parity test (preflight, Compute in both digest modes, and all three chart fields), bootstrap-volume.test.mjs (the helper runs end to end with its fake kubectl and writes accepted references into the Pod as written) and production-image-upgrade.test.mjs (all three image flags).
• Docs: the 2026-10-13 breaking-change entry gains a clause; the profile guide and rendering flow describe the shared rule.

Verification

• The port against the real parser: a Go program linking github.com/distribution/reference v0.6.0 agrees with imageReferenceProblem on all 66 table cases and on accept/refuse for 141,685 unique fuzzed references (16,085 accepted). The Helm define, rendered by Helm 3.19.2 in a minimal chart, agrees with the same Go program on about 2,500 unique fuzzed references with a SHA-256 digest.
• A sweep of all ten callers (three preflight fields, three chart fields, Compute in both digest modes, both bash tools) over the table: 212 disagreements with Kubernetes on origin/main, 0 on this branch. On main the only Kubernetes-valid references any caller refused were the IPv6 hosts; nothing Kubernetes accepts is refused now.
• The four new table tests fail on origin/main (c5180dd) and pass here.
• CI Required green on f576f70.
• node --test on profile-preflight-chart-parity, bootstrap-volume, production-image-upgrade, logging-packaging, production-kubernetes-packaging, configuration-startup, observability-demo-chart and tests/conformance/kubernetes-compute.test.mjs: all pass.
• The four touched test files (parity, bootstrap-volume, production-image-upgrade, configuration-startup; 67 tests): 20/20 runs pinned to 2 CPUs (taskset -c 0,1), on d19ebdd; f576f70 only changes a resume error string and one docs sentence.
• tsc --build apps/controller/tsconfig.json, eslint and prettier on the touched files, the workspace and module boundary checks: clean.
• Open PRs touching the same files (feat(deploy): configure hosted Codex discovery egress with profiles #1782, Support custom Installation profiles and readable Preset starters #2253, fix(controller): close refused native-admin sockets before audit settlement #1707, fix: prepare native admin access on the first Agent deployment #2259, Enable dedicated OpenClaw from Console with pinned native runtime #1685, feat: assign configured OpenClaw roles to Agent users #906, fix(compute): stub every model provider and strip request params on workspace-node gateways #830, feat(iam): grant the deployer exact read of the revision a deploy admits #1605, Enable Codex OAuth with OpenShell-owned credentials #1559, fix(credentials): release refresh bootstrap references and preserve resend protection #1952, feat: add Agent workload tags #115 and others): git merge-tree shows no new conflicted file or hunk compared with main.
• Not run: a cluster pull of an IPv6-hosted image.

Risks

• Breaking, as noted in breaking-changes.md: profiles, values or tool flags that used one of the refused spellings now fail preflight, helm template, the tools or startup. Kubernetes could not pull those images anyway.
• The two bash tools now need Node.js 24 to check images. upgrade-production-images already required it; prepare-bootstrap-volume runs from the same checkout as the Node-based profile renderer and resolves its helper through symlinks.
• The chart define duplicates the TS rule; the table tests pin them together.

[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: blocked before merge.

What this changes

This PR shares Kubernetes-compatible image-reference validation across profile rendering, Compute, Helm, and deployment tools.

Example: Use [fd00::1]:5000/repo@sha256:aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa.

• Before: The controller-image chart rule and bootstrap tool reject the bracketed registry host.
• After: Both accept the reference while retaining the immutable SHA-256 requirement.

Review scores

Measure	Result	What it means
Overall readiness	🦐 gold shrimp (3/6)	Useful repair with independent parser comparisons and meaningful caller coverage; the installation-guidance defect and unresolved compatibility gate prevent a merge-ready verdict.
Proof confidence	🦐 gold shrimp (3/6)	The supplied maintainer-author classification exempts the ordinary contributor proof gate; reported renderer, Helm, Compute, and tool checks support parser parity, but the persisted parameters.json additions lack an existing-state upgrade/resume demonstration, and the prepared role-sharing screenshots concern unrelated behavior.
Patch quality	🦐 gold shrimp (3/6)	1 actionable review finding remain.

Product

Kind: Bug fix · Worth it: Yes · Fix scope: Complete
User problem: Operators can pass validation with image references Kubernetes later refuses, while some deployment paths reject valid IPv6 registry references.
Reason: The inconsistent rules create real deployment failures; the supplied maintainer-author classification establishes approval for the documented validation cutover.

Merge readiness

⛔ Blocked before merge - 2 items remain

Keep open for maintainer triage: this PR addresses image-validation drift still present on main and preserves valid IPv6 registry references.

Priority: P2
Reviewed head: f576f701b0e49f00261cdaad0bcf605b19976834

Before merge

• Document Node.js as a prerequisite for manual installation (P2) - Every valid bootstrap image now executes host Node.js here. The supported manual-YAML path in docs/guides/deploy/production-installation.md:119-122 explicitly says Node.js is unnecessary and then requires prepare-bootstrap-volume at line 387. An operator following that path without Node now stops before PVC preparation despite satisfying the documented prerequisites. Update the manual installation guidance and the bootstrap preparation step to require Node.js 24+, and remove the obsolete no-Node promise.
• Add data-model compatibility proof - The review found that existing stored data may not work after upgrade. Show that existing data still loads and works with this change.

Findings

• [P2] Document Node.js as a prerequisite for manual installation — scripts/prepare-bootstrap-volume:90-93

Tests

• Missing end-to-end proof: No supplied existing-state upgrade and resume run demonstrates retained Installation settings and safe handling of the changed parameters.json record.

Agent review details

How this fits together

Operator image references enter through profiles, Helm values, deployment flags, and Installation configuration; validation either rejects them before deployment or passes the original references into Kubernetes workloads.

flowchart TD
  A[Operator image references] --> B[Profile renderer and deployment tools]
  A --> C[Helm values]
  A --> D[Compute configuration]
  B --> E[Shared OCC reference validator]
  D --> E
  C --> F[Matching Helm reference validator]
  E --> G[Validated workload images]
  F --> G

Loading

Technical review

Best possible solution:

Retain the shared validator, correct the operator prerequisites, and demonstrate safe existing-state upgrade and resume behavior.

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

Current-main rules visibly accept malformed repository components and reject valid IPv6 references in selected callers; the added case table and reported base-versus-head runs cover these disagreements, but no target code was executed during this review.

Is this the best way to solve the issue?

Sharing the TypeScript rule and checking the necessary Helm equivalent against an independent parser oracle is appropriate; the implementation preserves the prior IPv6 acceptance decisions.

Full review comments:

• [P2] Document Node.js as a prerequisite for manual installation — scripts/prepare-bootstrap-volume:90-93
  Every valid bootstrap image now executes host Node.js here. The supported manual-YAML path in docs/guides/deploy/production-installation.md:119-122 explicitly says Node.js is unnecessary and then requires prepare-bootstrap-volume at line 387. An operator following that path without Node now stops before PVC preparation despite satisfying the documented prerequisites. Update the manual installation guidance and the bootstrap preparation step to require Node.js 24+, and remove the obsolete no-Node promise.
  Confidence: 0.99

Overall correctness: patch is incorrect
Overall confidence: 0.94

AGENTS.md: found and applied where relevant.

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

Provenance checked

• Profile and Compute image validation keeps the original intent (fix(profiles): match auth url, runtime image and proxy checks to their consumers #2342: Runtime images bypass Helm, so applying its old restrictive control-plane rule incorrectly rejected working IPv6 references.)
• Uppercase repository rejection across callers keeps the original intent (fix(chart): match preflight's image path and observability url refusals #2362: Reject uppercase repository paths before Kubernetes reports InvalidImageName.)
• Collector image grammar changes intended behavior with a stated reason (fix(chart): refuse an uppercase repository path in logging.collector.image #2363: Keep IPv6 registry references valid while refusing uppercase repository paths.)
• Shell-tool image checks keeps the original intent (fix(scripts): make operator input checks locale-independent #1854: Make image validation independent of locale so non-ASCII characters cannot pass Bash ranges.)
• Upgrade resume fingerprints keeps the original intent (Make production image upgrades resumable #537: Bind resumable upgrades to reviewed inputs and implementation identities instead of replaying uncertain mutations.)

Testing

Proof path: in-process harness.

Security

None.

Evidence

What I checked:

• Pinned introduction and main behavior: The introduced diff replaces separate image rules with the OCC validator and matching Helm implementation; current main retains the separate rules. The three base-only CI files are outside this PR's introduced changes. (packages/occ/src/image-reference.ts:29, f576f701b0e4)
• New bootstrap-host prerequisite: The bootstrap helper now invokes host Node.js for every valid digest reference, while the production installation guide explicitly offers manual YAML without Node.js and subsequently requires this helper. (scripts/prepare-bootstrap-volume:92, f576f701b0e4)
• Persisted upgrade-record boundary: parameters.json gains imageReferenceSha256 and imageReferenceHelperSha256; resume compares the complete record before proceeding. Supplied verification does not demonstrate the changed record's creation/resume behavior or an existing-state upgrade. (scripts/upgrade-production-images:294, f576f701b0e4)
• Explicit parser oracle: The patch explicitly ports distribution/reference v0.6.0, making its contract relevant. Inspection of ParseNormalizedNamed, splitDockerDomain, Parse, and regexp.go supports the normalization, grammar, and repository-path length rules used here. (normalize.go, ff14fafe2236)
• Preserved prior intent: fix(profiles): match auth url, runtime image and proxy checks to their consumers #2342 deliberately retained IPv6 runtime references, and fix(chart): refuse an uppercase repository path in logging.collector.image #2363 retained IPv6 Collector references; this PR extends that acceptance to other callers rather than restoring the earlier restrictive rule. (scripts/render-installation-profile.mjs:41, ea166b291b1d)
• Accepted validation cutover: The supplied maintainer-author classification establishes direction approval; the PR explicitly records earlier rejection of Kubernetes-invalid references and the new bootstrap Node.js dependency as intended tradeoffs. (f576f701b0e4)

Likely related people:

• freeqaz: Raw commit 362d86b adds scripts/render-installation-profile.mjs:4 relative to its recorded parents. This identifies author metadata, not feature responsibility or a PR merger. (role: source-line author; confidence: high; commits: 362d86bb6507; files: scripts/render-installation-profile.mjs)
• kevinlin-openai: Raw commit ee48017 adds scripts/render-installation-profile.mjs:3 relative to its recorded parents. This identifies author metadata, not feature responsibility or a PR merger. (role: source-line author; confidence: high; commits: ee4801774d14; files: scripts/render-installation-profile.mjs)
• Eric Cai: Raw commit 177f576 adds scripts/render-installation-profile.mjs:870 relative to its recorded parents. This identifies author metadata, not feature responsibility or a PR merger. (role: source-line author; confidence: high; commits: 177f57647b28; files: scripts/render-installation-profile.mjs)

Review metrics

Metric	Value	Why it matters
Production and regression coverage	Production +216/-86; tests and fixtures +337/-19	The shared grammar replaces several weaker rules; most test growth exercises the same independent reference cases through distinct production callers.

Labels

Label changes:

• add rating: 🦐 gold shrimp: Overall readiness is 🦐 gold shrimp; proof is 🦐 gold shrimp and patch quality is 🦐 gold shrimp.
• remove rating: 🧂 unranked krab: Current PR rating is rating: 🦐 gold shrimp, so this older rating label is no longer current.

Label justifications:

• P2: This is a bounded deployment-validation repair affecting malformed image references and valid IPv6 registry references.
• merge-risk: 🚨 compatibility: The intended cutover rejects previously accepted configuration and introduces a host Node.js requirement for bootstrap preparation; upgrade-record compatibility also requires evidence.
• rating: 🦐 gold shrimp: Overall readiness is 🦐 gold shrimp; proof is 🦐 gold shrimp and patch quality is 🦐 gold shrimp.
• status: ⏳ waiting on author: ClawSweeper has contributor-facing work open and is waiting for author action.

Rank-up moves

Optional improvements that raise the rating; they are not merge blockers.

• Supply an existing-state upgrade/resume trace showing settings remain intact and incompatible evidence records stop before mutation.

Rating scale

6/6 🦀 challenger crab · 5/6 🦞 diamond lobster · 4/6 🐚 platinum hermit · 3/6 🦐 gold shrimp · 2/6 🦪 silver shellfish · 1/6 🧂 unranked krab. Overall follows the weaker of proof and patch quality; ✨ marks media proof (a screenshot, video, or linked artifact) that directly shows the changed behavior.

Workflow

ClawSweeper edits this one comment on every review. Comment @clawsweeper re-review for a fresh review only. Repair and merge execution lanes are retired.

Reviewed October 13, 2026, 7:19 PM ET / 23:19 UTC.