review round and refactor

[CMGS]

Whole-repo review round, 2026-09-21. Every production file read in full; the hibernate/CocoonSet flow lens and a test lens run by reader agents, every finding adjudicated against the source. No production behaviour changes on purpose: every behaviour question went to an issue.

Commits

• ddd2c16 review: drop the stray sentence above markFailed
• 15dd392 test: the podpatch short-circuits must never reach the client
• 9a2bb21 docs: match the suspend gate, wake short-circuit, sub-agent bind and Failed retry to the code
• cc7c090 build: pin cocoon-common at 5d8d95e and import its CRDs
• 575a9a9 cut: vmLive and vmClonedAndRunning give way to meta.VMLive
• b85d37f cut: drop the two godocs the predicate hoist left behind
• 1c41772 build: bump the docker actions and the golang base image
• e450feb fix: a migration stuck on an unschedulable target raises MigrateNoCapacity
• 68841b2 fix: a dead-lettered pod turns Progressing False with reason DeadLettered
• c1abedf fix: a suspend that cannot complete reports Failed after suspendTimeout
• 8878375 fix: the suspend deadline counts while the registry probe keeps failing
• 3209bc4 fix: MigrateNoCapacity names no target node
• f82de99 fix: a suspend retried after its timeout re-enters Suspending before the probe error returns
• c96f256 docs: the node a pending migration waits on is read from the pod's affinity
• 5351ba9 fix: start hibernation deadlines before returning probe errors
• 2ffce73 docs: the hibernate timeout starts on the first pass and counts failing probes

Findings

#	where	claim	outcome
1	podpatch/podpatch_test.go	the short-circuit tests asserted only a nil error and never proved the client was not called	test rewritten with interceptor.Funcs counting Patch, including the clear-on-a-clean-pod direction; mutation-checked
2	hibernation/reconciler.go	a stray sentence above markFailed restated the recovery path	review commit
3	cocoonset/suspend.go	suspend has no deadline and its terminal hatch is Pod.Phase=Failed only, which vk-cocoon never publishes for a live pod; a macOS agent parks the set in Suspending forever	fixed (#42, owner picked the timeout over a webhook rule): the suspend gate and the release drain share pollSuspend; the entry into Suspending is stamped on cocoonset.cocoonstack.io/suspending-since, after suspendTimeout (3 min) the set reports Failed with a SuspendTimedOut Warning and the next pass re-enters Suspending with a fresh deadline, the HibernateTimedOut cadence of the CR path; the stamp is cleared on Suspended; the gate still skips only kubelet-terminal pods since a lifecycle-failed VM may be live; TestReconcileSuspendTimesOutAfterTheDeadline, mutation-checked; Codex round 6 then showed a persistently failing registry probe bypassed the deadline, so pollSuspend takes the probe error and reports Failed past the deadline while returning the error before it; round 7 then showed the retry after a timeout stayed in Failed and reset the stamp every pass, so pollSuspend persists Suspending before returning the probe error and the test now covers the second timeout (TestReconcileSuspendTimesOutWhenTheRegistryProbeKeepsFailing)
4	cocoonset/migrate.go	sub-agents keep their old-node bind after a migration	closed by decision (#43): the co-location promise is scoped to creation time, as docs/cocoonset.md step 12 states; rebuilding sub-agents after a migration is a mechanism without an observed need
5	cocoonset/rebuild.go, cocoonset/status.go	a dead-lettered pod leaves the CocoonSet reporting converged	fixed (#44): while any owned pod is parked at the current generation the Progressing condition reads False/DeadLettered and names the pods; phase and Ready are unchanged because a parked pod may still be Ready; TestBuildStatusReportsDeadLetteredPods, mutation-checked
6	cocoonset/migrate.go	a stuck migration emits no out-of-capacity Event	fixed (#45): the !VMLive arm reuses podUnschedulable and emits MigrateNoCapacity with the scheduler message plus cocoon_operator_migrate_unschedulable_total, mirroring the released-seat wake; TestMigrationReportsAnUnschedulableTarget; Codex round 6 showed the first cut named the current spec.nodeName although a pending pod stays pinned to the node it was built for, so the event carries the scheduler message alone and the docs no longer claim a retarget moves a pending pod
7	hibernation Failed phase	re-enters Hibernating/Waking on the next event	by design (tests and docs pin the retry), no change
8	cocoonset/restore.go pre-marked restore vs image conflict		fixed on the vk-cocoon side (its marked-restore image guard), nothing to do here
9	hibernation/hibernate.go	the CR path had the same gap as finding 3's suspend retry: a registry probe error on the first request, or on a retry from Failed, returned before Hibernating was entered, so hibernateTimeout never started and the phase starved behind the requeue backoff (found by the owner)	fixed: setPhase(Hibernating) before the probe error is returned; a no-op during an active attempt, so the running clock is preserved, and the completed-snapshot fast path is untouched; TestReconcileHibernateProbeErrorsKeepDeadlineAcrossRetries (fails on the parent tree with phase = "", want Hibernating); docs/hibernation.md states when the timeout starts and that failing probes count

Docs: docs/cocoonset.md (suspend gate scope, wake engage/fall-through conditions, sub-agent bind at creation), docs/hibernation.md (what retries from Failed, what stays Pending), docs/observability.md (which paths treat lifecycle-state=failed as terminal) corrected to the code, including two wording residuals Codex raised; docs/observability.md and docs/cocoonset.md now carry MigrateNoCapacity, SuspendTimedOut, the new metric and the DeadLettered condition.

Cut-list (report only, nothing applied)

candidate	est.	status
cocoonset/migrate.go vmLive and hibernation/wake.go vmClonedAndRunning are the same predicate	−6	applied: both give way to meta.VMLive, which landed on cocoon-common's round branch; this PR pins cocoon-common at that commit's pseudo-version (v0.3.4-0.20260921020807-5d8d95e98bca, fetchable through proxy.golang.org, verified in a Linux container with an empty module cache) and imports its CRDs, so the toolbox-name bound ships too; swap the pin for the tag once cocoon-common v0.3.4 is released
delete.go parseVMNamesAnnotation duplicates cocoon-net's splitTrim	≈0 after imports	skip

LOC (same counting on both ends)

• cocoon-operator origin/main=e181a3f: prod=2761 test=5191 comments=140 blanks=290 effective=2331
• cocoon-operator HEAD=5351ba9: prod=2833 test=5483 comments=137 blanks=295 effective=2401 (prod +72 / +2.6%, all in the four owner-approved fixes, their Codex follow-ups and the owner's CR-path fix; the review, cut and docs commits net −12 prod)

Per commit (prod adds/dels · test adds/dels): review 0/1 prod (−1 comment); test 37/33; docs 0/0; pin + CRD import (go.mod/go.sum only); VMLive cut −6; build 0/0; MigrateNoCapacity 18/0 · 36/0; DeadLettered 25/6 · 33/2; suspendTimeout 49/7 · 48/0; probe-error deadline 12/7 · 34/0; event wording 1/1 · 2/2; retry re-entry 4/1 · 25/0; CR hibernate deadline 3/0 · 110/0. The seven fix commits add zero comment lines.

Gates

• GOWORK=off make fmt-check, GOWORK=off make lint on GOOS=darwin and GOOS=linux: 0 issues.
• asl ./... on both GOOS: 0 findings (the two advisory forwarders hasPhaseDeadline and vmClonedAndRunning stay KEPT per the 2026-09-12 ledger).
• go test -race -count=1 ./...: green (five packages).
• linux/arm64 golang:1.27 container on c1abedf and again on 5351ba9: go build ./... && go vet ./... && go test -race -count=1 ./... green (five packages).
• docker build --target build on the new golang digest (linux/arm64) succeeds.
• linux/arm64 golang:1.27 container: go build ./... && go vet ./... && go test -race -count=1 ./... green at 2b07c2b (the only change since is a docs sentence).
• Every fix commit carries a regression test that was mutation-checked: reverting the fix makes the test fail for the stated reason.
• Fix, test and docs commits add zero comment lines.

Codex

Rounds 1 to 3 on the committed diff raised five docs wording residuals (Failed-retry scope, missing-pod phase, wake engage and fall-through conditions, the recreated-main case); all reworded. Round 4 verdict on 9a2bb21: "No blockers: this diff converges." Round 5 on the pin and the VMLive switch: one P3 (two godocs left behind by the deleted predicates, removed in the follow-up commit) and "No blockers: this diff converges." Round 6 on c1abedf (the Dependabot fold and the three owner-approved fixes) raised two P2s, the probe-error bypass of the suspend deadline and the misleading target node in MigrateNoCapacity, both fixed in their own commits. Round 7 on 3209bc4 raised two more: the retry after a timeout never re-entered Suspending (fixed in f82de99) and the docs told readers to find the waited-on node in a scheduler message that aggregates reasons (fixed in c96f256). Round 8 on c96f256: "No blockers: this diff converges." The owner's 5351ba9 was verified by a full read, the gates and the container without a Codex round, by the owner's instruction.

Follow-ups filed

Closes #42, #44, #45; #43 closed by decision. Dependabot #47, #48, #49 and #50 are folded into 1c41772 and closed. The committed CRD copy under config/crd/bases already carries cocoon-common's toolbox-name bound (imported in cc7c090); once cocoon-common v0.3.4 is tagged, the next dependency-bump commit swaps the pseudo-version for the tag and re-runs make import-crds.