review(s3frontend): named gate wrappers; withTags becomes applyTagsIfPresent

Per review on #81: the require-lock boolean disappears from call sites
behind four thin wrappers — mutateVersionLock / mutateVersionTags and
resolveLockTarget / resolveTagTarget — so each operation family sets its
own gate and a bare true/false never appears at a call site. withTags is
renamed applyTagsIfPresent: the old name implied a non-mutating derive
(the Go WithX convention) while the function mutates its argument in
place; the new name and its comment state the mutation outright.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

Author: frrist

s3frontend/copy.go

@@ -178,7 +178,7 @@ func (b *Backend) CopyObject(ctx context.Context, input s3response.CopyObjectInp
 	// Commit to the destination via the write rule: splice + reference index.
 	// The new claims use the DESTINATION bucket/space; the same digests gain
 	// another reference.
-	node, effState, err := b.commitVersion(ctx, bucketState, dstKey, dstMf, withTags(initState, dstTags), nil)
+	node, effState, err := b.commitVersion(ctx, bucketState, dstKey, dstMf, applyTagsIfPresent(initState, dstTags), nil)
 	if err != nil {
 		return s3response.CopyObjectOutput{}, err
 	}

s3frontend/multipart.go

@@ -579,7 +579,7 @@ func (b *Backend) CompleteMultipartUpload(ctx context.Context, input *s3.Complet
 		// Validated at CreateMultipartUpload; a parse failure here is a bug.
 		return s3response.CompleteMultipartUploadResult{}, "", fmt.Errorf("s3frontend: parse session tagging: %w", err)
 	}
-	initState = withTags(initState, tags)
+	initState = applyTagsIfPresent(initState, tags)
 
 	// Commit through the §5 write rule (docs/s3-versioning.md): seq allocation,
 	// supersession per the bucket's versioning state, and the post-commit

s3frontend/object.go

@@ -73,7 +73,7 @@ func (b *Backend) PutObject(ctx context.Context, input s3response.PutObjectInput
 	if err != nil {
 		return s3response.PutObjectOutput{}, err
 	}
-	initState = withTags(initState, tags)
+	initState = applyTagsIfPresent(initState, tags)
 
 	// PRECONDITIONS (no lock): If-Match / If-None-Match. Evaluated here to fail
 	// fast before ingest, then RE-CHECKED under the per-bucket lock at commit so

s3frontend/objectlock.go

@@ -93,7 +93,8 @@ func lockStateFromHeaders(st *registry.State, mode types.ObjectLockMode, retainU
 // docs/s3-object-tagging.md §2): bucket, key existence, the lock-enabled gate
 // when the operation requires it (the lock methods do; tagging has no gate),
 // then versionId grammar and resolution (via resolveVersion) and the
-// delete-marker sentinel. Key
+// delete-marker sentinel. Call through resolveLockTarget /
+// resolveTagTarget, which set the gate for their operation family. Key
 // existence outranks the lock-enabled check — a missing key on a bucket
 // without lock is NoSuchKey, never the missing-configuration error
 // (GetObjectRetention_non_existing_object pins it, matching posix).
@@ -127,6 +128,13 @@ func (b *Backend) resolveStateTarget(ctx context.Context, bucketName, key, versi
 	return rv, nil
 }
 
+// resolveLockTarget resolves a lock method's target: resolveStateTarget
+// with the lock-enabled gate required. The tagging twin is
+// resolveTagTarget (objecttag.go).
+func (b *Backend) resolveLockTarget(ctx context.Context, bucketName, key, versionID string) (*resolvedVersion, error) {
+	return b.resolveStateTarget(ctx, bucketName, key, versionID, true)
+}
+
 // versionStateOf seeks the resolved version's VersionState block: leaf →
 // state tree → block. Nil (with no error) when the key or version carries no
 // explicit state — the callers' ErrNoSuchObjectLockConfiguration case.
@@ -153,7 +161,7 @@ func (b *Backend) versionStateOf(ctx context.Context, rv *resolvedVersion) (*msb
 // verbatim (§6). An expired retention is returned as stored; expiry is the
 // controller's judgment.
 func (b *Backend) GetObjectRetention(ctx context.Context, bucket, object, versionId string) ([]byte, error) {
-	rv, err := b.resolveStateTarget(ctx, bucket, object, versionId, true)
+	rv, err := b.resolveLockTarget(ctx, bucket, object, versionId)
 	if err != nil {
 		return nil, err
 	}
@@ -171,7 +179,7 @@ func (b *Backend) GetObjectRetention(ctx context.Context, bucket, object, versio
 // the §2 no-such-configuration sentinel; an explicit OFF is &false (§4.1's
 // tri-valued hold).
 func (b *Backend) GetObjectLegalHold(ctx context.Context, bucket, object, versionId string) (*bool, error) {
-	rv, err := b.resolveStateTarget(ctx, bucket, object, versionId, true)
+	rv, err := b.resolveLockTarget(ctx, bucket, object, versionId)
 	if err != nil {
 		return nil, err
 	}
@@ -186,12 +194,19 @@ func (b *Backend) GetObjectLegalHold(ctx context.Context, bucket, object, versio
 	return &on, nil
 }
 
+// mutateVersionLock runs a per-version state write for the lock methods:
+// mutateVersionState with the lock-enabled gate required. The tagging twin
+// is mutateVersionTags (objecttag.go).
+func (b *Backend) mutateVersionLock(ctx context.Context, bucketName, key, versionID string, mutate func(*msbucket.VersionState)) error {
+	return b.mutateVersionState(ctx, bucketName, key, versionID, true, mutate)
+}
+
 // PutObjectRetention stores the controller's retention document on the
 // resolved version. Mode-transition policy (same-mode replacement,
 // COMPLIANCE never weakened, governance bypass) ran in the controller before
 // this is called.
 func (b *Backend) PutObjectRetention(ctx context.Context, bucket, object, versionId string, retention []byte) error {
-	return b.mutateVersionState(ctx, bucket, object, versionId, true, func(vs *msbucket.VersionState) {
+	return b.mutateVersionLock(ctx, bucket, object, versionId, func(vs *msbucket.VersionState) {
 		vs.Retention = retention
 	})
 }
@@ -202,7 +217,7 @@ func (b *Backend) PutObjectLegalHold(ctx context.Context, bucket, object, versio
 	if status {
 		hold = msbucket.LegalHoldOn
 	}
-	return b.mutateVersionState(ctx, bucket, object, versionId, true, func(vs *msbucket.VersionState) {
+	return b.mutateVersionLock(ctx, bucket, object, versionId, func(vs *msbucket.VersionState) {
 		vs.LegalHold = hold
 	})
 }
@@ -212,7 +227,9 @@ func (b *Backend) PutObjectLegalHold(ctx context.Context, bucket, object, versio
 // version's state block (mutate owns its fields and every other field is
 // carried, §4.1 rule 3), and the leaf/state-tree/top-MST splice. A
 // manifest-arm key upgrades to a leaf on its first state write (§4.1 rule 1);
-// an empty merged block is elided rather than stored. The check order runs
+// an empty merged block is elided rather than stored. Call through
+// mutateVersionLock / mutateVersionTags, which set the gate for their
+// operation family. The check order runs
 // entirely inside the commit (a missing bucket surfaces through
 // mapCommitError); key existence outranks lock-enabled, which outranks the
 // versionId grammar, matching posix and the pinning conformance cases.

s3frontend/objecttag.go

@@ -11,14 +11,28 @@ import (
 // tenant beside object lock. The controller parses and validates the
 // PutObjectTagging XML (tag count, key/value limits) and hands us a clean
 // map; the x-amz-tagging creation-time header is stamped by the write paths
-// (§4). Tagging has no bucket gate, so the shared check order runs with
-// requireLock false, and absence is a success: a version without tags
-// answers the empty set, never a sentinel.
+// (§4). Tagging has no bucket gate — its wrappers below run the shared
+// check order without the lock-enabled step — and absence is a success: a
+// version without tags answers the empty set, never a sentinel.
 
-// withTags merges a creation-time tag set into a version's initial state,
-// allocating the state when only tags are present (docs/s3-object-tagging.md
-// §4). Returns vs unchanged when there are no tags.
-func withTags(vs *msbucket.VersionState, tags map[string]string) *msbucket.VersionState {
+// resolveTagTarget resolves a tagging method's target: resolveStateTarget
+// with no bucket gate (docs/s3-object-tagging.md §2).
+func (b *Backend) resolveTagTarget(ctx context.Context, bucketName, key, versionID string) (*resolvedVersion, error) {
+	return b.resolveStateTarget(ctx, bucketName, key, versionID, false)
+}
+
+// mutateVersionTags runs a per-version state write for the tagging methods:
+// mutateVersionState with no bucket gate.
+func (b *Backend) mutateVersionTags(ctx context.Context, bucketName, key, versionID string, mutate func(*msbucket.VersionState)) error {
+	return b.mutateVersionState(ctx, bucketName, key, versionID, false, mutate)
+}
+
+// applyTagsIfPresent merges a creation-time tag set into a version's initial
+// state (docs/s3-object-tagging.md §4). When tags are present it MUTATES vs
+// in place — allocating it when nil — and returns it; when there are no tags
+// it returns vs untouched. Callers pass freshly built state, never a shared
+// value.
+func applyTagsIfPresent(vs *msbucket.VersionState, tags map[string]string) *msbucket.VersionState {
 	if len(tags) == 0 {
 		return vs
 	}
@@ -32,7 +46,7 @@ func withTags(vs *msbucket.VersionState, tags map[string]string) *msbucket.Versi
 // GetObjectTagging returns the resolved version's tag set; the empty map
 // when it carries none (the controller renders an empty <TagSet/>).
 func (b *Backend) GetObjectTagging(ctx context.Context, bucket, object, versionId string) (map[string]string, error) {
-	rv, err := b.resolveStateTarget(ctx, bucket, object, versionId, false)
+	rv, err := b.resolveTagTarget(ctx, bucket, object, versionId)
 	if err != nil {
 		return nil, err
 	}
@@ -54,15 +68,15 @@ func (b *Backend) PutObjectTagging(ctx context.Context, bucket, object, versionI
 	if len(tags) == 0 {
 		tags = nil
 	}
-	return b.mutateVersionState(ctx, bucket, object, versionId, false, func(vs *msbucket.VersionState) {
+	return b.mutateVersionTags(ctx, bucket, object, versionId, func(vs *msbucket.VersionState) {
 		vs.Tags = tags
 	})
 }
 
 // DeleteObjectTagging clears the resolved version's tag set. Idempotent: a
 // version without tags is a success no-op.
 func (b *Backend) DeleteObjectTagging(ctx context.Context, bucket, object, versionId string) error {
-	return b.mutateVersionState(ctx, bucket, object, versionId, false, func(vs *msbucket.VersionState) {
+	return b.mutateVersionTags(ctx, bucket, object, versionId, func(vs *msbucket.VersionState) {
 		vs.Tags = nil
 	})
 }