[freeqaz]

Problem

Kubernetes Compute removes repository material Secrets on stop, retire and Namespace delete whenever the Installation sets drivers.compute.configuration.network.repositoryCredentials (#3234, finding 2201). The chart grants the tenant worker Secret list only with Helm repositoryCredentials.enabled, which is native broker delivery. An Installation switched from the native broker to protected OpenShell Git (or an installed sender) by following protected-openshell-git.md / selection.md kept the peer: the docs said to omit controlSocket, publicCaPath and repository.nativeSender, but not the peer. Deploys still worked, then every stop, retire and delete listed Secrets, got a 403 and retried forever (finding 2212).

Change

• Fail early: Installation load refuses drivers.compute.configuration.network.repositoryCredentials on bundled Kubernetes Compute unless repository.nativeSender selects native delivery. This covers bundled protected Git, an installed sender and an Installation with no repository section. The chart's pre-install/pre-upgrade check runs the same loader, so helm upgrade stops before the API rolls.
• No RBAC change and no Driver-side change: native delivery keeps its exact peer check (port 8443), and protected delivery never used the peer (validateRepositoryCredentialSupport returns early for a Sandbox with repository access; repository egress is empty there).
• Docs: protected Git and installed-sender procedures say to remove the peer; the native guide marks it native-only; a 2026-10-23 breaking-change notice gives the message and the fix. The 2026-10-13 notices move to the earlier-notices page to keep that page under the word limit.

Verification

• New cases fail on main (Missing expected rejection) and pass here:
  • tests/integration/sandbox-driver-startup.test.mjs "configured protected OpenShell Git admits only the selected execution and network policy": the protected Git Installation plus a broker peer is refused.
  • tests/integration/configuration-startup.test.mjs "repository startup rejects unmatched ownership, ...": the peer without any repository section is refused.
  • tests/integration/installation-change-preflight.test.mjs "the upgrade hook check refuses a repository broker peer until the operator removes it": the shipped check-installation.mjs (the pre-upgrade hook script) refuses the leftover peer with the exact message, then accepts the same Installation once only the peer is deleted (the documented recovery). Native delivery keeps loading with its peer: "repository startup constructs the same local resolver ..." in configuration-startup still passes.
• All three touched tests pinned 20/20 at 2 CPUs (taskset -c 0,1).
• Full files: configuration-startup 35/35, sandbox-driver-startup 113 pass / 1 skipped (existing skip), qa-repository-installation 3/3, installation-change-preflight 4/4, postgres-external-protected-git 2 pass / 5 skipped (no PostgreSQL locally).
• tsc --build tsconfig.json tests/tsconfig.json, eslint on touched files and Prettier check: clean.
• Every in-repo Installation that sets the peer through the loader also sets repository.nativeSender (profile renderer, occ dev repository setup, examples, tests).
• Internal review (high effort): changes requested for a breaking-change notice and a no-repository test; both applied, plus the full field name in the message and the native-only guide wording.

Risks

• An Installation that kept the peer after switching away from native delivery now fails startup and the upgrade check until the operator deletes the peer; the breaking-change notice covers it. Native Secrets left from before such a switch stay where they were (unchanged from main: the worker cannot list them without the native chart grant).

[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 rejects a Kubernetes repository broker peer unless native delivery is selected and documents how operators remove a leftover peer.

Example: An Installation switches to protected Git but retains network.repositoryCredentials on port 8443.

• Before: Startup accepts it, but Agent retirement can repeatedly fail when Secret listing is forbidden.
• After: Startup and upgrade preflight reject the leftover peer and tell the operator to remove it.

Review scores

Measure	Result	What it means
Overall readiness	🦐 gold shrimp (3/6)	The focused patch is source-correct and has useful regression coverage, but release-upgrade validation remains incomplete.
Proof confidence	🦐 gold shrimp (3/6)	The collaborator-authored PR is outside the ordinary live-proof gate. Added coverage invokes the shipped upgrade-check script and checks rejection followed by peer-removal recovery. Upgrade compatibility remains insufficient: the supplied run claims use a fresh fixture rather than a released Installation carried forward with its settings and data.
Patch quality	🦐 gold shrimp (3/6)	No actionable review findings were identified.

Product

Kind: Bug fix · Worth it: Yes · Fix scope: Complete
User problem: Agents can remain stuck during lifecycle cleanup after operators switch repository delivery using instructions that leave the native broker peer configured.
Reason: This is a narrow repair of a concrete configuration mismatch, and the collaborator author has explicitly selected the fail-early behavior.

Merge readiness

⛔ Blocked before merge - 1 item remains

Keep this PR open: it addresses a concrete repository-cleanup failure with a focused startup check, and no introduced correctness defect was found.

Priority: P1
Reviewed head: ab150e712094e7748b9645d37fb745d6c3035f55

Before merge

• 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

None.

Tests

• Missing end-to-end proof: The added fixture-based recovery test does not demonstrate fresh-install and released-Installation upgrade behavior with existing settings and data retained.

Agent review details

How this fits together

Installation composition reads Driver and repository configuration before API or worker startup; Kubernetes Compute subsequently uses the selected peer to manage repository material Secrets.

flowchart TD
  A[Installation configuration] --> B[Installation loader]
  C[Upgrade check] --> B
  B --> D[Repository delivery selection]
  D --> E[Reject leftover broker peer]
  D --> F[Start API and worker]
  F --> G[Kubernetes Compute lifecycle]

Loading

Technical review

Best possible solution:

Keep the focused composition check, preserve native delivery, and use the documented peer-removal recovery after existing repository sessions are settled.

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

Current source shows that a protected-delivery Installation retaining the peer reaches Secret cleanup despite the chart withholding Secret list permission; no target code was executed during this review.

Is this the best way to solve the issue?

The existing Installation loader is the appropriate owner for rejecting this inconsistent selection, and the patch preserves the chart’s least-privilege permissions.

AGENTS.md: found and applied where relevant.

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

Provenance checked

• Repository delivery selection and native peer validation keeps the original intent (3115679: Credential Proxy Implementation #2487 separates native and protected delivery while retaining original cleanup ownership.)
• Operator repository configuration keeps the original intent (2fd3d79: Repository credentials: platform lifecycle and Kubernetes delivery #235 integrated native material delivery and cleanup with Agent lifecycle; the new wording identifies that peer as native-only.)
• Breaking-change archive rotation keeps the original intent (c742f82: The archive rotation established by docs(deploy): split the breaking-changes archive to make room #2276 moves older notices to preserve the documentation length limit.)

Testing

Proof path: in-process harness.

Security

None.

Evidence

What I checked:

• Pinned production change: The introduced change adds one rejection branch after the existing native-delivery peer check, without changing RBAC, delivery implementation, or persistence. (apps/controller/src/composition/installation-config.ts:875, ab150e712094)
• Current-main cleanup trigger: Compute projectsRepositoryMaterial treats a configured repository peer as sufficient to trigger material cleanup for a repository-bearing revision; stop and retirement invoke that cleanup. (apps/controller/src/drivers/compute/kubernetes/index.ts:8937, 9c10e6d57133)
• Secret-list permission boundary: The tenant-worker role grants Secret list only when repositoryCredentials.enabled is true, supporting the reported failure after switching away from native delivery. (deploy/helm/openclaw-enterprise/templates/rbac.yaml:68, 9c10e6d57133)
• Recovery coverage added since the previous review: The new regression case invokes the shipped installation-check script, expects rejection with the leftover peer, deletes only that peer, and expects installation-startup-ready; its input comes from a fresh test fixture. (tests/integration/installation-change-preflight.test.mjs:240, ab150e712094)
• Related merged repair: fix(kubernetes): skip repository material cleanup without a credential endpoint #3234 fixed cleanup when the peer is absent and recorded the underlying Secret-list failure; this PR handles the distinct leftover-peer configuration. (9063f24f3f58)
• Accepted startup tradeoff: The collaborator-authored PR explicitly accepts rejecting previously accepted leftover-peer configurations and documents peer removal as recovery; this is an owner direction decision. (docs/guides/deploy/breaking-changes.md:18, ab150e712094)

Likely related people:

• Kevin: Raw commit 849b2b2 adds docs/guides/repository-credentials/installation.md:124 relative to its recorded parents. This identifies author metadata, not feature responsibility or a PR merger. (role: source-line author; confidence: medium; commits: 849b2b24111f; files: docs/guides/repository-credentials/installation.md)

Labels

Label changes:

No label changes.

Label justifications:

• P1: The affected configuration can prevent repository-bearing Agents from completing stop, retirement, and deletion.
• merge-risk: 🚨 compatibility: Previously accepted leftover-peer configurations now fail startup; the maintainer-authored PR explicitly accepts this behavior and documents recovery.
• rating: 🦐 gold shrimp: Overall readiness is 🦐 gold shrimp; proof is 🦐 gold shrimp and patch quality is 🦐 gold shrimp.

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.

History

Review history (1 earlier review cycle)

• reviewed 2026-10-23T00:54:04.641Z sha 8f68e2e :: blocked before merge. :: none

Reviewed October 22, 2026, 9:00 PM ET / October 23, 2026, 01:00 UTC (Revision 2).

[freeqaz]

@clawsweeper re-review

[clawsweeper]

🦞👀
Exact review queued.

Re-review progress:

• State: Complete
• Detail: The durable review result and its route handoff completed.
• Run: https://github.com/openclaw/clawsweeper/actions/runs/39881654947
• Updated: 2026-10-23T01:02:27.120Z