carlos / platform Public

ACCEPT WITH FIXES for a DORMANT merge; this is not approval to enable CARLOS_SWAP.

1. Medium — internal/appconfig/swap.go:106: setting policy can restart instances with the flag off. On an app without config.json, setting hold or concurrency above one creates a bundle and touches its generation. SyncExec then creates the config file and invokes the restart callback. A member configuring future swap behavior can therefore interrupt today’s running instances. Q3 remains an unresolved behavioral decision, not a harmless deferral. Smallest fix: preserve absent-config delivery semantics for a bundle containing only swap policy; test that the first policy change causes no restart.

1. Medium — [deploywatch.go:223](deploywatch.go:223): initial discovery does not respect the watch deadline. Three failed lists incur two five-second sleeps plus request time, then watchVersion receives the original timeout again. Even flag-off deploy --timeout 1s can wait over eleven seconds with immediate list failures, substantially longer with slow requests. Smallest fix: carry one deadline through list requests, retry waits and fallback; pass only the remaining duration to the legacy watch.

1. High — [deploywatch.go:378](deploywatch.go:378): sticky completion can report success after the watched build was replaced. Instance A reaches B while another instance is settling. A then reports C installed and serving. Its state remains “swapped”; once the other instance finishes, the watch succeeds for B. TestWatchRolloutFinalIsStickyThroughADarkBoxAndANewerBuild explicitly requires this incorrect result. The modeURL && urlLive arm also ignores a later rollout report entirely. Smallest fix: retain state across missing observations, but invalidate success on explicit supersession and process later rollout reports even after URL success. Change the tests to reject these false successes.

1. High — [deploywatch.go:235](deploywatch.go:235): failed observation still triggers HTTP polling of reporting hosts. Three failed lists cause unconditional legacy fallback, waking sleeping instances on an armed fleet. TestWatchRolloutThreeFailedFirstListsFallBack pins that behavior. There is also a shorter route: loadBoxJoin suppresses status-read errors, so a successful instance-list response can falsely appear to contain no reports. Smallest fix: propagate status-read completeness, using LoadBoxesComplete, and retain an unknown state on observation failures. Fall back only on affirmative evidence that the targeted hosts use the legacy path. Test both endpoint failures and failed heartbeat reads with zero origin requests.

1. High — internal/host/restart.go:852: unsigned requests can clear another box’s failure holds. The agent passes a host from restart-scope.json directly to RetryHost. The repository’s region-host IAM policy grants writes throughout the deployment bucket. A compromised box can append retries naming another box’s hosts and bump the serial, bypassing console membership and the operate-role check. Restricting the request to locally owned routes does not authenticate its author. Smallest fix: authenticate retry entries with a console-held signing key and verify their account, app, host and failure identity before clearing anything. Keep unsigned retry entries inert. Q4 must be resolved before enabling the feature.

1. Medium — internal/console/apiinstances.go:422: customer-fleet rollout reports are omitted. All three surfaces now agree by reading only platform boxes. A fleet box can report a held build under its fleet prefix while the instance list omits the hold, the retry door returns 409, and deploy falls back to HTTP polling. This loses required behavior rather than merely hiding a button. Smallest fix: give list, page and retry one fleet-aware loader, preserving account/app checks and reporting incomplete reads.

1. Medium — [deploywatch.go:235](deploywatch.go:235): a truncated flag-off list changes legacy watch output. With more than 500 records and no rollout reports, truncated prevents fallback even when every targeted host is present. The command prints the new “roll out box by box” introduction although every target uses URL polling. This violates the required byte-for-byte no-report behavior. Smallest fix: establish completeness for the target set, rather than requiring an untruncated app-wide response, or paginate discovery. Add an exact-output comparison for this case.

1. Medium — internal/console/swapcopy.go:84: hold-mode behavior is described falsely. Retry confirmation promises fallback to the last good build if retry fails; a hold-mode failure stays down. HoldText likewise says the instance “stays on” its good build when AttemptDown means nothing serves. Separately, internal/rolloutwords/words.go:58 says the new build starts beside the running one for hold swaps and socketless worker replacements too. Smallest fix: use mode-neutral drafts where mode is unavailable and explicit down-state text where AttemptDown is known. Test the actual hold and worker emit/render paths.

1. Low — internal/console/apiswap.go:40: the draft-marker and em-dash requirements are incomplete. The new rate-limit response contains an em dash and has no marker. Other examples include the unmarked Save button in templates/app_settings.html:380, new flag help and decoder errors in apps_swap.go, and the new em-dash literal in instances_cli.go:452. These appear through ordinary settings, help, list or error paths. Smallest fix: sweep added member-facing literals, assign review IDs, and remove new em dashes. The preserved pre-existing sidecar sentence is a separate, accepted exception.

Rulings

References below identify each Ruling: entry by its ledger line.

• L12 — Accept: deferring the review server follows the controller’s explicit instruction.
• L13 — Accept: marked drafts are authorized for this review stage; finding 9 identifies incomplete application.
• L14 — Accept: the approved-copy pin cannot honestly precede approval; it remains required before shipping approved copy.
• L15 — Accept: preserving existing flag-off text while marking replacement drafts maintains compatibility.
• L21 — Accept: callers already supply reason words; mapping them twice corrupts classification. Keeping the emit-site diagnostic test is appropriate.
• L22 — Accept: route registration, mutation wrappers and census entries follow current main’s authority checks.
• L23 — Accept: placing new ship refusals outside the legacy-exemption file preserves that gate.
• L24 — Accept: a five-minute heartbeat interval cannot support a reliable short-watch death heuristic. Observation errors still need explicit treatment.
• L25 — Reject: truncation handling and five-second cadence are reasonable; unconditional sticky completion contradicts watched-build completion. Finding 3.
• L26 — Reject: consistency between surfaces does not justify excluding customer-fleet boxes. Finding 6.
• L27 — Accept: person-only policy changes fit §11 and the existing policy-door boundary.
• L28 — Reject as a complete disposition: Q1’s resolution and Q2’s join are sound; Q3 affects dormant operation, and Q4 weakens retry authorization. Findings 1 and 5 need explicit resolution.
• L29 — Accept: Task 14b supplies the index’s missing refusal and wake-reason obligations.
• L30 — Accept: distinguishing known declines from refusals is appropriate; deferring the optional FailureBook display does not itself violate the surface contract.
• L31 — Accept: retaining the existing sidecar sentence preserves flag-off behavior.
• L77 — Accept: a blank REASON cell accurately represents no reported reason.
• L83 — Reject: three failed observations do not establish that HTTP polling is safe. Truncation protection and treating unknown unavailable codes conservatively are sound.
• L91 — Accept: retaining the pushed WIP commit is harmless provided the required squash happens before landing.
• L125 — Accept: CI may supply the full-suite gate, but landing still requires green CI on the final squashed tip; the ledger’s local-check summary is not that evidence.