carlos / platform Public

Reviewed 26065378..ac45a709 at HEAD 4433a788. Two P2 findings and one P3. No builds or tests run; worktree unchanged.

1. P2 — A stalled membership read stops enrollment across all regions. membershipread.go:106 calls Member with the console worker’s unbounded context. Neither AWS describe adds a deadline, and the SDK client has no overall request timeout. A connection that stalls after connecting can block the serial enrollment pass indefinitely. writeBound starts only after this read returns. Fix: bound the membership read, classify timeout as unknown, and retain the attempt without increasing backoff. Add a test with a reader that waits for cancellation and verify subsequent boxes still progress.

1. P2 — Accepted status overrides break the group-report contract. hostenv.go:86 permits CARLOS_STATUS_BUCKET and CARLOS_STATUS_PREFIX, but PrivateBoxReport reads the deployment bucket’s unprefixed key. A different status bucket leaves the console waiting forever for guard readiness. With only a prefix, joining writes the expected unprefixed report, but hosting switches to prefixed reports; the console’s original report then expires. Fix: reject incompatible overrides for group boxes, or make joining, hosting and console readers share one supported destination. Test the production report path through the joining-to-hosting transition.

1. P3 — The allowlist still admits credentials embedded in webhook URLs. hostenv.go:71 accepts CARLOS_ALERT_WEBHOOK, including URLs containing authentication tokens. Its characters pass Render. This contradicts the binding credential exclusion used to remove CARLOS_DNSIMPLE_TOKEN. Fix: remove it from the group allowlist and test refusal without exposing its value, unless the census owner explicitly approves an exception.

Deferred-minor triage:

| Disposition | Items | |---|---| | Before merge | Fix the status mismatch and webhook exclusion above. Update the plan’s obsolete DNSimple-token allowance and CheckConfirmed references. Explicitly require the bootstrap binary to sit outside both protected slots: otherwise installation refuses it, or the subsequent bootstrap-file deletion can remove the installed binary. | | Closed by inspection | Missed write deadlines do not become ErrMembershipStale or increase backoff. Empty-reference errors are retried as unknown. Invited, unarmed boxes re-touch the marker each minute. | | Before activation | Keep the stop/start versus reboot verification. Add the fresh-box guard-arm → recovery → guard-exec restart rehearsal as an explicit activation gate. Static inspection cannot establish that systemd transition. | | Nonblocking follow-ups | Error wrapping and wording; cleanup logging; ownership, zero-timestamp and gate-test gaps; SDK-client reuse; ignored flags; host-env timeout, reseeding and durability details; joining cancellation, flag parsing, log deduplication and response-body cleanup. | | No change required | Command syntax is exempt from copy review; approved CLI prose need not gain backticks. The alleged mixed shared-key race is not supported by this implementation: all shared keys are required, processed in sorted order, and a loser stops at its first conflicting key. Partial progress on errors still warrants narrowing the comment. |

The build ledger records a separate merge hold until platform-1e posts “published” on discussion #288. Its current external status was not checked.

Verdict: not ready to merge.

---

FIRST PROMPT: Final whole-branch review of region-ha-2b-build at 4433a788, read-only, without heavy tests.

WHAT: Review the implementation and triage deferred minors.

STATUS: Review complete: two P2 findings, one P3; no files changed.

NEEDED: Nothing for this review.

RECOMMENDED: Fix the findings and plan discrepancies, add focused regressions, then verify the publication hold is cleared.

SESSION: Safe to close — no processes started or scratch files created.