Repository navigation
fix(config): name the refused field for a leftover repository broker peer - #3268
Conversation
…peer A Kubernetes Compute network.repositoryCredentials peer without repository.nativeSender was refused with a plain Error, so API and worker startup logged only STARTUP_FAILED / WORKER_STARTUP_FAILED. Throw an InstallationFieldError with the field's JSON Pointer instead: startup logs and the chart's upgrade check now report DRIVER_CONFIGURATION_INVALID with that path and a fixed message. Correct the breaking-changes notice to match.
…er-peer refusal The upgrade check now prints DRIVER_CONFIGURATION_INVALID with the field's pointer for a leftover repository broker peer. Update the preflight test's exact line and let it own the upgrade-check assertion, so the startup test boots only the API and worker.
|
🦞👀 Pull request received. I will update this pull request when review starts. ClawSweeper review completeClawSweeper finished reviewing this revision. The review result is being finalized. |
clawsweeper
Bot
added
clawsweeper:intake:bug
|
Codex review: needs maintainer review before merge. What this changesThis PR names the leftover repository broker peer in API, worker, and upgrade-check configuration errors. Example: An Installation retains network.repositoryCredentials on port 8443 after repository.nativeSender is removed.
Review scores
ProductKind: Bug fix · Worth it: Yes · Fix scope: Complete Regression provenanceRegression provenance — verified: source commit Merge readiness✅ Ready for maintainer review Keep this PR open: it fixes a current-main diagnostic gap with a focused change, and no actionable correctness or security defect was found. Priority: P2 Before mergeNone. FindingsNone. Agent review detailsHow this fits togetherInstallation composition validates operator configuration before API and worker startup; shared error formatting sends refused-field details to startup logs and the Helm upgrade check. flowchart TB
A[Installation configuration] --> B[Installation loader]
B --> C[Repository peer validation]
C --> D[Installation field error]
D --> E[Shared diagnostic formatter]
E --> F[API and worker logs]
E --> G[Helm upgrade check]
Technical reviewBest possible solution: Use the existing field-error owner to report the refused peer while preserving validation and peer-removal recovery. Do we have a high-confidence way to reproduce the issue? Current-main source shows that a Kubernetes Installation with the peer and no native sender throws a plain Error, which API and worker startup reduce to generic codes; no target code was executed during this review. Is this the best way to solve the issue? The patch uses the established shared diagnostic mechanism and preserves the original refusal, making it the smallest complete repair for the stated problem. AGENTS.md: found and applied where relevant. Codex review notes: model internal, reasoning medium; reviewed against 92caa7ac6250. Provenance checked
TestingProof path: in-process harness. SecurityNone. EvidenceWhat I checked:
Likely related people:
LabelsLabel changes:
Label justifications:
Rating scale6/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. WorkflowClawSweeper edits this one comment on every review. Comment Reviewed October 22, 2026, 11:54 PM ET / October 23, 2026, 03:54 UTC. |
Internal review nits: the replaced table case used a port below 8443, so keep that setup; compare the booted processes' full detail as #3268's test does; pin the endpoint REQUIRED wording.
Problem
Since #3250, startup refuses an Installation that sets
drivers.compute.configuration.network.repositoryCredentialswithoutrepository.nativeSender. The refusal was a plainError, so the API logged onlySTARTUP_FAILEDand the worker onlyWORKER_STARTUP_FAILED: the startup log keeps no message. Only the chart's upgrade check printed the text, and it runs on upgrade only. A fresh install, or a Secret edit followed by a restart, crash-looped without naming the field.docs/guides/deploy/breaking-changes.mdsaid startup showed the message.Change
installation-config.ts: the refusal is now anInstallationFieldError, the same way fix(egress): name the refused field in an authority block #3260 reports authority-block faults. It has one detail: the pointer/drivers/compute/configuration/network/repositoryCredentials, codeINVALID_VALUEand the existing fixed message, which holds no configured values.startupConfigurationFailuremaps it toDRIVER_CONFIGURATION_INVALID, so the API'sstartup-error, the worker'sworker.startup-errorand the upgrade check all print the field. The upgrade check's line is nowinstallation-check: API and worker startup would refuse the Installation: DRIVER_CONFIGURATION_INVALID: [{"path":"/drivers/compute/configuration/network/repositoryCredentials",...}].configuration-startup.test.mjs: a new test next to fix(egress): name the refused field in an authority block #3260's cases. It boots the real API, worker andcheck-installation.mjswith a leftover peer and asserts the code and the detail in each. The existing message assertion in the repository refusal table still holds.breaking-changes.md: the 2026-10-23 notice's "How to tell" now names the code and the path the logs show. The page stays under the word limit (2480 by the repo counter; it was 2477).Verification
STARTUP_FAILED) and passes with the fix.tsc --buildfor the controller, Prettier and ESLint on the touched files: clean.Risks
tests/helpers/egress-installed-resource-cleanup.mjskeeps its 500-then-observe branch. The prepared withdrawal race now answers 409CREDENTIAL_TOKEN_UNSETTLED, but any other 500INTERNAL_ERRORfrom a prepared source delete still reaches that branch, so it is not provably dead.