Skip to content

fix(artifact): report creation when a Put promotes another writer's marker - #99

Open
audreyt wants to merge 1 commit into
mnemon-dev:masterfrom
audreyt:fix/artifact-put-creation-flag
Open

fix(artifact): report creation when a Put promotes another writer's marker#99
audreyt wants to merge 1 commit into
mnemon-dev:masterfrom
audreyt:fix/artifact-put-creation-flag

Conversation

@audreyt

@audreyt audreyt commented Aug 13, 2026

Copy link
Copy Markdown
Contributor

Put discards the created flag from its first settlePromotion, and inspectReplay hardcodes Replayed: true. That first settle is not read-only — when another writer has already staged a complete marker, it installs the final link itself. So the writer that actually published the object reports a replay, and the writer that staged the marker replays too (it lost the link race). A stored object can come back with no creator at all.

The post-staging path already handles this (result.Replayed = !created); the replay branch just never did.

Reproduction

TestStoreConcurrentPutHasOneFinalEffectAcrossInstances already catches it under scheduling pressure — the in-process per-digest mutex is per-Store, and the test deliberately uses two instances over one root, so the cross-instance protocol is exercised:

before after
GOMAXPROCS=1, -count=400 94 failures, all created results = 0, want 1 400/400 pass
64 writers × 4 instances, -count=150 49 failures 150/150 pass
GOMAXPROCS=1,2, whole package -count=200 pass

Test

TestStorePutReportsCreationWhenItPromotesAnotherWritersMarker pins the invariant without timing: stage a complete marker on one instance, Put the same bytes on another, and the promoting Put must not claim a replay. It fails on master with Put installed the final link, so it created the object and must not report a replay.

Separate, still open

CI on #98 failed with artifact: corruption: promotion did not settlesettlePromotion exhausting its eight attempts. That is a different path and this PR does not address it. Mechanism, for whoever picks it up: between os.Link(marker, final) and the marker's removal the shared inode has nlink == 2, while every verifier calls readVerifiedFile(..., expectedLinks: 1); the resulting ErrCorruption is classified retryable, and the loop is a fixed 8 attempts with no backoff, so a slow or contended filesystem can exhaust the budget and report corruption for a benign race. I did not reproduce it locally in ~1500 runs across contention profiles, and widening a fail-closed bound seemed like your call rather than something to slip into a fix for an unrelated defect. Happy to open an issue.

…arker

Put discarded the created flag from its first settlePromotion, and
inspectReplay hardcodes Replayed: true. That first settle is not read-only:
when another writer has already staged a complete marker, it installs the
final link itself. The writer that published the object was then reported as
a replay, and the writer that staged the marker replayed too because it lost
the link race — so a stored object could come back with no creator at all,
breaking the exactly-one-creator invariant across Store instances.

Propagate created through the replay branch, matching what the post-staging
path already does.

TestStoreConcurrentPutHasOneFinalEffectAcrossInstances reproduces it under
scheduling pressure: at GOMAXPROCS=1 it failed 94/400 runs with
"created results = 0, want 1", and passes 400/400 after. A 64-writer probe
over four instances failed 49/150 before and 0/150 after. The added test
pins the same invariant without relying on timing: stage a marker on one
instance, Put the same bytes on another, and the promoting Put must not
claim a replay.
Copilot AI lite review requested due to automatic review settings August 13, 2026 00:16

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Fixes Put creation reporting in the artifact CAS when a Put call “promotes” (publishes) a final object by linking another writer’s already-staged promotion marker, ensuring exactly one caller observes Replayed=false for the first effective publish across store instances.

Changes:

  • Preserve and propagate the created result from the first settlePromotion in Store.Put, so a promoting writer is not incorrectly reported as replaying.
  • Adjust the replay inspection path to set result.Replayed = !created consistently in both the “already present” and “post-staging” branches.
  • Add a deterministic regression test that stages a complete marker in one store instance and verifies that a second instance promoting it reports creation (not replay).

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File Description
internal/agency/artifact/store.go Fixes Put’s replay/creation reporting when promotion occurs via another writer’s staged marker.
internal/agency/artifact/store_test.go Adds a regression test covering “promote another writer’s marker” creation semantics across instances.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants