carlos / platform Public
Reviewed HEAD 1297aa7547fa51f41a14d866b7faef2e6943c4b9 read-only. No builds or tests run; working tree unchanged. Findings concern the proposed implementation.
| Open round-3 item | Status | Assessment | |---|---|---| | Unresolved group reference overwrites legacy status | Closed | groupStatusTick publishes nothing until resolution succeeds and retries on subsequent ticks, independently of both flags. | | Historical reports exhaust the 200-key cap | Closed | ListMod pages the listing; old entries are filtered before GETs and the cap. Content-freshness checks remain. | | Remove redundant freshness pre-checks | Closed | Both removed; the bound store remains the write enforcement point. | | Stop/start versus reboot verification | Partly | Explicit activation gate retained; the dev observation remains outstanding. This alone does not block implementation. |
The deadline actually starts at describe start, which is stricter than your summary and permitted by spec rev 18.
New findings:
- P2 — A successful return after the deadline bypasses unknown-outcome recovery.
inFlight, plan line 3224 checks cancellation only when the underlying operation returns an error. A context-ignoring store such asDirStorecan begin its roster write before the deadline, finish afterward, and returnnil. Enrolment then succeeds without recording an unknown outcome or scheduling the fresh describe.
This contradicts the stated requirement that a commit not confirmed before the deadline enters recovery. Check the bound after the operation, including successful returns. Add a test whose Put completes successfully after expiry; the current in-flight test always returns ctx.Err().
- P2 — Read-back accepts a stale describe as fresh.
readBack, plan line 4745 callsconfirmMembershipdirectly and never callsc.fresh. That helper validates membership fields but does not enforce elapsed time.
If the group read succeeds and the subsequent instance read stalls beyond 60 seconds, read-back accepts the old membership answer, logs “a fresh read confirms membership,” and deletes the recovery entry. Apply the freshness check before settling the entry; stale answers should retain it for retry. Test a successful describe taking exactly 60 seconds.
- P2 — Failed paging permanently discards the only automatic escalation.
readBack, plan line 4753 deletes the recovery entry unconditionally afterpageNotMember. That function only logs delivery errors.
A temporary alert-sink failure therefore leaves a detached, running instance enrolled indefinitely without delivering the required page. Subsequent passes skip its existing roster entry. Preserve pending notification state and retry with backoff; suppress further sends after success. Add a failed-delivery-then-success test.
- P2 — The host.env directory check does not establish a protected write path.
runHostEnv, plan line 1415 checks only the immediate directory’s ownership and type. It accepts a root-owned directory writable by other users, follows symlinked ancestors, and subsequently reopens the pathname inhostfloor.Durable.
With --out beneath a writable ancestor, another user can replace or redirect the checked directory before the write. A writable destination directory also permits replacement of the resulting configuration despite its 0600 mode. Anchor the write to a validated directory descriptor and reject writable or symlinked path components. The current test covers only an immediate-parent symlink.
The allowlist, value validation, roster cross-check, and trigger-content independence are represented. The directory issue above leaves the checked-write condition incomplete.
---
FIRST PROMPT: Round 4 review of region HA 2b at 1297aa75.
WHAT: Reviewed round-3 dispositions and the revised safety paths.
STATUS: Prior status defects closed; four new P2 findings. Verdict: not ready to build.
NEEDED: Nothing for this review.
RECOMMENDED: Fix the four gaps and add the boundary tests described above.
SESSION: Safe to close. No files changed or processes started.