carlos / platform Public

Reviewed c267f5f5 against the accepted spec and existing code. This was a static review; no files changed or proposed implementation tests ran.

1. P0 — Plan 1, Tasks 7, 12 and 13: the installed worker cannot pass readiness. SelfView calls net.Interfaces(), but the planned service permits only AF_UNIX at line 5527. Linux Go enumerates interfaces through NetlinkRIB, which opens an AF_NETLINK socket. Enumeration therefore fails, and the probe rejects the empty interface list at plan line 5248. Fixtures override Self, hiding this failure. Fix: obtain network-namespace evidence without opening a prohibited socket, propagate observation errors, and test the default implementation inside the installed unit.

1. P0 — Plan 1, Task 15; Plan 2, Task 7: new test helpers collide with existing declarations. Plan 1 defines waitFor, already declared with a different signature in housekeeper_test.go:139. Plan 2 defines hookStore, already declared in temphook_test.go:16. Both affected test packages fail to compile. Fix: give the new helpers feature-specific names and update every call and receiver.

1. P0 — Plan 1, Task 15: successful payload reports are parsed as invalid JSON. The child runs m.Run() at line 6278, then payload decodes its last stdout line. Successful m.Run() prints PASS after the test finishes, including with verbosity disabled. Fix: use a dedicated report pipe or a distinguished JSON record instead of the final test-runner output line.

1. P0 — Plan 1, Task 15: readiness tests remove the wrong service file. The test manipulates /run/systemd/system/carlos-dbread@.service. Its installer calls ensureSystemUnits("/etc", ...), whose real implementation writes beneath <etcDir>/systemd/system. Removing the /run file leaves the installed service available, so the missing-service assertion fails. Fix: manipulate the actual installed file, check every operation, and restore it through cleanup. Exercise masking at the effective installed path too.

1. P0 — Plan 1, Task 17: the census fingerprint bootstrap cannot execute. Line 6937 instructs loading an unfinished overlay into a fresh inventory to compute fingerprints. The loader requires valid fingerprints before accepting declarations (lines 413–421); @remove also requires an existing base entry (line 470). Fix: calculate fingerprints directly from the seven-field declaration rows, then validate the completed overlay over main’s layered inventory. Update the procedure inherited by plans 2 and 3.

1. P0 — Plan 2, Tasks 4–6: evidence fixtures attempt create-only updates. putEdgesFile always passes an empty ifMatch. The fixture seeds uk-1 at line 753; tests at lines 786 and 1010 then write that key again. DirStore.Put explicitly rejects this. Fix: make the edges-fixture helper conditionally replace an existing object using its etag. Keep protocol writes create-only.

1. P0 — Plan 2, Task 6: the admission gate depends on Task 7’s implementation. TestDBReadAdmissionBounds opens two requests and expects a third to receive 429. Task 6’s stream stub at line 1298 immediately returns, releasing both earlier requests’ counts. The third receives 200. Fix: move this test and its passing gate into Task 7, or provide an interim stream that remains open until cancellation.

1. P0 — Plan 3, Task 4: its installed-unit pull test precedes the agent support it requires. The acceptance test at line 466 opens output as StreamLTX. At this task boundary, the agent still uses Plan 1’s StreamRows writer. Task 5 introduces the stream selection at lines 653–660. Fix: move the installed-agent pull acceptance test and run after Task 5, while retaining the isolated worker tests in Task 4.

1. P1 — Plan 1, Task 11: a result can advertise an unconfirmed chunk. flushLocked increments seq before confirming the create. On failure, attempt returns an interrupted result carrying that sequence at line 4779; serve still publishes it at line 4620. This contradicts spec §5.4: every chunk must be confirmed before the result is written. Fix: distinguish an incomplete output protocol from a publishable result. Retain ambiguous chunk bytes for confirmation and publish no result referencing an unconfirmed sequence. Add a fault test where chunk confirmation fails but result creation would succeed.

1. P1 — Plan 1, Tasks 9–11: retry reuses a reservation while the previous worker may still hold scratch. The retry loop immediately starts another attempt after a vanished file. Closing the previous connection does not establish unit termination. The next grow overwrites the reservation under the same request ID at line 3229, and dial does not reconcile outstanding units. This breaks J5 and can consume the probe’s reserved connection. Fix: retain each attempt’s reservation until its unit stops, or await confirmed retirement before retrying. Test with deliberately delayed teardown and space sufficient for only one attempt.

1. P1 — Plan 2, Task 6: admission can write after request cancellation. The handler calls s.bumpSerial(ctx) followed by auditActivity. Existing bumpSerial deliberately uses context.WithoutCancel; the audit path uses a context-free Exec. A disconnect can therefore be followed by serial and audit writes, although J1 permits only the best-effort cancel afterwards. Fix: provide request-bound announcement and audit operations for this route, including cancellation-aware underlying writes. Test cancellation between the queue create and these operations.

1. P1 — Plan 2, Task 7: the request bounds do not constrain blocking work. streamDBRead uses bare r.Context() for storage calls. Pickup expiry is checked after GET; owner expiry is checked after result GET, LIST and the entire chunk relay. A result can even be sent before checking an already-expired deadline at lines 1861–1865. Blocked storage also prevents the required progress lines. Fix: use deadline-bound contexts for both phases, bound storage operations, check deadlines during relay, and clamp write deadlines to the request bound. Add stalled-store and slow-relay tests.

1. P2 — Plan 2, Task 6: the 20-second pre-header timeout does not bound request-body reading. The handler extends the socket deadline at line 1130, then creates a separate timeout context. readJSONLimit still blocks directly on r.Body. A slowly delivered body can exceed both the specified 20 seconds and backhaul’s header timeout. Fix: enforce a short socket/body deadline during admission, propagate its context throughout admission, and extend the read deadline for streaming afterwards. Add a stalled-body test.

1. P2 — Plan 1, Tasks 10, 12 and 13: draining boxes can win reads. consider checks the envelope and admission but never draining state; host wiring supplies no such state. Console filtering cannot exclude a draining contender when another eligible box exists or evidence is incomplete. This contradicts spec §7.1. Existing edges records already carry Draining. Fix: supply authoritative draining state to the loop and check it before election, including after jitter. Test a draining contender beside a healthy one.

1. P2 — Plan 1, Task 11: some worker-start failures do not trigger the required probe. askProbe runs for dial and header-write failures, but pre-ready file-send/read failures merely return interrupted at lines 4741–4748. Socket activation can accept the connection before service startup fails, leaving capability advertised until the periodic probe. Fix: detect successful worker startup explicitly, or request a probe after every pre-ready transport failure once its connection closes. Test this through a read against a failing installed service.

1. P2 — Plan 1, Task 15: the disk-admission test assumes submission order is queue order. Lines 6620–6625 enqueue both reads before starting the agent, then assume the second remains unpicked. Same-second queue entries sort by random ID; the real store sorts keys lexicographically. The test can fail with correct admission behavior. Fix: observe the first worker running before submitting the second, or identify the losing request from the actual election.

1. P2 — Plan 1, Tasks 15 and 17: T3 does not prove the PrivateNetwork mutation. The installed-unit witness attempts TCP connections. Removing PrivateNetwork leaves RestrictAddressFamilies=AF_UNIX, so those attempts still fail. A textual unit-file assertion is not the required runtime witness. Fix: assert actual network-namespace evidence from inside the unit using a method compatible with its restrictions. Also use a valid SQLite fixture for the ATTACH witness; querying a plain-text “registry” file fails even when readable.

1. P2 — Plan 2, Tasks 6–7: T15 does not cover both admission counts or restart. The disconnect test closes one request before pickup and one after, then succeeds when one replacement fits. It does not prove both slots were released. The admission test covers only a person’s third request, and no restart case implements spec T15. Fix: separately fill and disconnect both person slots before and after pickup; exercise the seventeenth console request across multiple people; restart the console during open reads and verify no recovery state remains.

1. P2 — Plan 2, Task 12: T16 can pass without late output. The disconnect case waits only for cancel and result objects, without establishing publication after handler return. The timeout case explicitly accepts nothing-replicated at line 3382, which need exercise neither timeout nor late chunks. Fix: pause the real agent immediately before publication, force and observe handler termination, then release chunk/result writes. Assert operation ordering for both timeout and disconnect under the strict versioned-MinIO store.

1. P2 — Plan 2, Task 14: the result-before-list mutation requirement is weakened. The mutation table combines listing first with removing the last-sequence wait, explicitly acknowledging that listing first alone stays green. The accepted spec requires these mutations independently. Fix: add an operation-order witness that rejects LIST before result GET, independently of chunk-completeness checks.

1. P2 — Plan 3, Task 1: the required throughput measurement depends on telemetry nobody builds. Line 99 requires worker-reported restore/query durations and journal timestamps for successful pickup/result publication. Plan 1’s worker messages at lines 2382–2400 contain no phase timings, and its successful serve path at lines 4604–4625 emits no such measurements. The prescribed relay rate cannot be calculated from the supplied implementation. Fix: add the necessary phase instrumentation before the measurement, or define a reproducible measurement using observations the implementation actually exposes.

1. P2 — Plan 3, Task 7: malformed file sizes can panic the CLI. openDBPull checks only the total size, file-list presence and final transaction. A sealed summary containing sizes -1 and N+1 can satisfy a total of N; line 1132 then slices plaintext with a negative bound. Fix: validate individual sizes, coordinates, duplicate paths and overflow-safe totals before creating files. Add malformed-manifest tests that require DBF-13, no panic and no output copy.

1. P2 — Plan 3, Tasks 7 and 9: the cleanup tests do not exercise cleanup. TestForwardedPullLeavesNothingWhenTheSpoolFails checks only that readDBForward returns an error. The Ctrl-C test at line 925 uses an already-cancelled context and a memory sink. Neither invokes dbPullForward or checks directory removal. Its comment promises a binary cleanup test in Task 9, but that task supplies none. Fix: run the real pull path, fail after some spool files exist, and interrupt an active binary pull. Assert no --out file and removal of the specific spool directory.

1. P2 — Plan 3, Task 10: several listed mutations cannot turn their named tests red. In the mutation table, removing NewAgent’s clamp cannot affect TestPullCapOnlyLowers, which calls only dbReadsSettings. Removing the file-size sum check still fails later with the same error through incomplete file splitting. Sending run before check B need not create a bucket chunk because the agent can refuse before starting relay. Fix: test NewAgent directly; assert rejection of invalid manifests before file creation; and observe worker commands to prove no run occurs before successful check B.

Plan 1: REVISE. Plan 2: REVISE. Plan 3: REVISE.

REVISE