carlos / platform Public
ACCEPT WITH FIXES
Reviewed only d9a1f684..c0a23eab. All five requested fixes satisfy R19’s scoped requirements. One new P2 remains: publication errors introduce member-facing wording.
| Finding | Verdict | Evidence | |---|---|---| | F1(a) | ADDRESSED | versionsweep.go:77 adds GenerationVersions pins and returns without sweeping on error. That provider includes both version and path entries. | | F1(b) | ADDRESSED | generations.go:86 publishes staged paths independently of Version. pins.go:364 resolves and pins those paths without requiring a registry row. Legacy schema-1 files remain readable. | | F1(c) | ADDRESSED | workerbuild.go:83 records and publishes the pending identity. backend_exec.go:1784 requires that hook to succeed before spawnInCgroup. Failed publication prevents execution. | | F1(d) | ADDRESSED within the specified protocol | pins.go:291 writes intent before JSON; pins.go:213 rejects incomplete or mismatched publications. An intent-write failure prevents spawning; the previous snapshot then remains conservative. | | N4 | ADDRESSED | pins.go:114 includes the chain’s fully resolved endpoint, and adopt.go:1804 compares the candidate through Refers. The regression at internal/release/stage_reuse_test.go:324 covers the exact two-relative-hop layout. |
Publication, crash consistency and locking
- Agent dies between writes: intent N remains ahead of JSON N−1, so subsequent readers refuse the set. If death occurs before the intent rename, the previous publication remains valid and the new build has not executed.
- Agent restart: pins.go:246 calculates the next sequence from the larger readable on-disk intent/JSON sequence. Ordinary restart does not reset it. Corrupt intent recovery is conservative while unreadable, although ignoring malformed content cannot establish historical monotonicity after arbitrary corruption.
- Locking: generations.go:129 takes
genMu, snapshots undera.mu, releasesa.mu, and retainsgenMuthrough I/O. The hook releasesa.mubefore publication. Wake and respawn replace pending identity with installed identity in one critical section. I found no new lock inversion. - Shared reader: both
GenerationPins(pins.go:356) andGenerationVersions(pins.go:407) callReadGenerations; neither bypasses sequence/schema/completeness validation. Production writer and consumers useGenerationsPath(dataDir()).
New defect
N5 — P2: Internal publication errors become unreviewed member-facing copy. workerbuild.go:86.
An unwritable generations directory makes a wake return ErrBuildNotPublished plus its storage error. The existing route through activator.go:1002 and logsink.go:67 emits that text to members under wake failed:. Path redaction does not remove the new wording. Calling it a sentinel does not make its rendered text operator-only.
Smallest fix: map this sentinel at the member-facing boundary to existing reviewed failure wording, retaining publication details in the operator journal. Add a focused assertion that the sentinel text and storage details do not reach the member event. Deferring that mapping to plan 6 leaves plan 4’s explicit no-new-member-strings constraint broken.
Fixer’s remaining concerns
- Full disk: confirmed. Once intent advances but JSON cannot be replaced,
versionsweep.go:78refuses every sweep until publication succeeds. If/dataand/optshare the exhausted filesystem, staged-version cleanup cannot free the space needed for recovery. This is the availability cost explicitly accepted by R19, not grounds to restore unsafe sweeping. Recovery requires space from another source or reserved publication capacity. - Root/agent race: confirmed, but pre-existing. Root can inspect pins before the pending publication, then mutate the captured artifact afterward; publication-before-exec does not synchronize root’s destructive operation. Likewise, a reader overlapping publication can return the preceding snapshot because it reads intent once. These are limits of snapshot-based coordination, not an indefinitely trusted stale file after a completed failed publication. Re-reading intent would narrow one window; closing the destructive race requires shared synchronization covering artifact capture/publication and root’s check/mutation.
Out of scope
- Power-loss durability: parent-directory fsync remains absent at
pins.go:325; this previously recorded limitation is distinct from agent-process death. - Unchanged root/agent synchronization and later-plan scheduler,
GoodVersion, and generation-lifecycle integration were not re-reviewed.
No files modified and no Go tests run. git diff --check d9a1f684..HEAD passed; the working tree is clean.