Detect post-apply divergence, guard manifest quality, lock webhook state - #8
Merged
Conversation
…ook state
Three residual risks from v3.0.0. None is a regression; each is a gap
where a partial mitigation was possible but unbuilt.
Post-PATCH verification. The gateway now re-reads each setup after a
successful PATCH and compares the observed state against the approved
target. A mismatch stops the apply, records planned -> pending -> applied
-> conflicted in the journal, prints a critical alert, and points at the
rollback artifact. It deliberately does not attempt remediation: a
corrective PATCH would repeat the same overwrite, and deciding whose
change wins is a human call.
It ignores region test timestamps, server-managed errorMsg, account
ordering, empty-versus-nil containers, and unmodeled response fields.
Flagging ordinary server behavior as a clobber would train operators to
ignore the alert, which is worse than not having it.
This narrows rather than closes the no-CAS gap, and the boundary is worth
stating. It detects unexplained divergence after a write. It cannot
detect the case where a concurrent edit landed between the pre-PATCH
re-read and the PATCH itself: our full-list write erases it and the
result then equals our intent exactly, leaving nothing observable.
Combined with the existing pre-PATCH comparison, what remains uncovered
is an edit arriving inside the PATCH execution window — narrow, silent,
and not addressable without a version token Forward does not expose.
Manifest safeguards. sync-accounts is the only removal path and is
exactly as safe as its manifest. The docs warn against deriving one from
NQE output, since that reproduces by hand the unsound inference v3.0.0
retired, but nothing enforced it. A manifest closely matching observed
NQE inventory while diverging from configured membership now emits
manifest_matches_nqe_observation. It warns rather than blocks — a
legitimate manifest can coincidentally match, and blocking on a heuristic
invites workarounds. Removal previews now state the fraction outright
("removes 968 of 978 accounts (98.98%)") regardless of what the ceilings
permit.
Webhook state locking. Two daemons sharing a state file would corrupt
each other's dedupe records and watermarks; this was documented as
unsupported and is now enforced with a flock on a stable .lock sidecar.
The sidecar matters because the state file is atomically renamed on
write, so locking that inode would silently lose protection.
Existing tests were made stateful so the verification GET observes the
PATCH it accepted. No assertion was removed.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Arw4zhKVDNry9zV1Ej6jRt
The template is rendered verbatim for whatever tag triggers a release, so shipping v3.1.0 against the v3.0.0 text would republish that release's breaking changes as though they were new — telling operators to act on migrations they already completed, and burying what actually changed. Rewrites it for this release: additive hardening, no breaking changes. v2.x migration is now a pointer to docs/upgrading.md rather than the full text, and the known-limitation section states precisely what post-apply verification does and does not detect, so nobody reads it as closing the concurrent-edit gap.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Additive safety hardening on top of v3.0.0. No breaking changes.
Three residual risks were documented but unmitigated at release. None is a regression.
Post-apply verification
Forward exposes no compare-and-swap token, so a concurrent UI edit during an apply is overwritten. That was previously silent, which is the worst version of it.
The gateway now re-reads each setup after a successful PATCH and compares the observed state against the approved target. On mismatch it stops, records
planned → pending → applied → conflicted, prints a critical alert, and points at the rollback artifact.It deliberately does not remediate. A corrective PATCH would repeat the same overwrite, and deciding whose change wins is a human call.
It ignores region test timestamps, server-managed
errorMsg, account ordering, empty-vs-nil containers, and unmodeled response fields. Flagging ordinary server behavior as a clobber would train operators to ignore the alert — worse than not having it.This narrows the gap rather than closing it, and the boundary matters: it detects unexplained divergence after a write, but cannot detect a concurrent edit that arrived between the pre-PATCH re-read and the PATCH itself. The full-list write erases it and the result then equals our intent exactly, leaving nothing observable. Combined with the existing pre-PATCH comparison, what remains uncovered is an edit landing inside the PATCH execution window.
Manifest quality
sync-accountsis the only removal path and is exactly as safe as its manifest. The docs warn against deriving one from NQE output — that reproduces by hand the unsound inference v3.0.0 retired — but nothing enforced it.A manifest closely matching observed NQE inventory while diverging from configured membership now emits
manifest_matches_nqe_observation. It warns rather than blocks: a legitimate manifest can coincidentally match, and blocking on a heuristic invites workarounds.Removal previews now state the fraction outright — "removes 968 of 978 accounts (98.98%)" — regardless of what the ceilings permit.
Webhook state locking
Two daemons sharing a state file would corrupt each other's dedupe records and watermarks. Previously documented as unsupported, now enforced with a
flockon a stable.locksidecar — the sidecar because the state file is atomically renamed on write, so locking that inode would silently lose protection.Validation
209 tests,
-raceclean, webhook package clean at-count=10. Existing tests were made stateful so the verification GET observes the PATCH it accepted; no assertion was removed.PatchCloudAccountstill has exactly one production caller.🤖 Generated with Claude Code
https://claude.ai/code/session_01Arw4zhKVDNry9zV1Ej6jRt