[IAmJSD]

Root cause

MarkCollectionStarted hands the refresh handler an owner token that identifies the marker, and that token rides in the async self-invoke payload. Lambda async delivery is at-least-once, so a duplicate delivery carries the same token. The delivery that wins the task advisory lock collects; the duplicate is lock-skipped and releaseSkippedCollectionMarker calls ClearCollectionStarted(token). The token still matches, so the marker is cleared while the winner is still collecting. The banner drops early, and a refresh during the rest of the run wins a new marker, gets lock-skipped, releases it and returns 202 without collecting.

Fix

This is the invocation-scoped identity described in the issue:

• New store method ClaimCollectionInvocation(ctx, markerToken) (internal/config/store_postgres_recommendations.go, added to StoreInterface). It is a single UPDATE on recommendations_state row 1 that sets last_collection_owner_id to a fresh UUID when the marker is:
  • still owned by markerToken. started_at is left as the refresh recorded it.
  • free or stale under the same 5-minute rule as MarkCollectionStarted. started_at is set to NOW().
    It returns ("", false, nil) when a different run holds a fresh marker, and an error for an empty token.
• runScheduledTask (which HandleScheduledTask wraps) calls it through claimCollectionInvocation (new internal/server/collection_invocation.go) right after it acquires the advisory lock. This only happens for collect_recommendations runs that carry a token. The run then collects, and its deferred clear runs, under the new token. After that, a lock-skipped duplicate's ClearCollectionStarted(markerToken) no longer matches and does nothing.
• The free/stale branch closes the window between the winner taking the lock and its re-stamp. A duplicate that is skipped inside that window still matches and clears, so the winner re-establishes the marker instead of collecting with none. The same branch handles a late redelivery that arrives after the original run finished. Behavior change: that redelivery now shows the in-progress banner again while it re-collects; before, it collected with no banner.
• If another run holds a fresh marker, the winner owns nothing: it collects without a token and never clears. On a store error it keeps the marker token, which is the pre-Collection marker release is scoped to the marker, not the invocation #84 behavior, rather than failing a collection the lock already admitted.
• Unchanged: a token-carrying run that is skipped behind a tokenless cron run still releases its own marker, which is the case releaseSkippedCollectionMarker exists for. Paths with no locker (tests, the non-Lambda HTTP mode) do not claim.
• No migration: last_collection_owner_id already exists (000093).

Concurrency: production only claims while holding the advisory lock, so the race that matters is the winner's claim against the duplicate's clear. Both are single-row UPDATEs on row 1. They serialize on the row lock, and under READ COMMITTED the second re-checks its WHERE against the first's committed row. Either order leaves the winner owning a set marker: if the claim lands first the clear does nothing, and if the clear lands first the claim re-establishes the marker. The race test below runs this on real Postgres.

Verification

All commands ran from the repo root with AWS_CONFIG_FILE=/dev/null AWS_SHARED_CREDENTIALS_FILE=/dev/null, because this machine's ~/.aws/config default profile is invalid.

Fail before / pass after (unit level, in-memory marker store plus a shared advisory-lock fake). New tests in internal/server/collection_invocation_test.go:

• TestHandleScheduledTask_DuplicateDeliveryKeepsWinnersMarker: the winner blocks mid-collect, the duplicate is delivered and lock-skipped, and the test checks that the marker is still set.
• TestHandleScheduledTask_DuplicateClearsBeforeWinnerClaims: the duplicate is interleaved between the winner's lock and its claim.
• TestHandleScheduledTask_SkippedBehindCronStillReleases: the cron-overlap release still works.
• TestHandleScheduledTask_ClaimInvocationOutcomes: claimed, held by another run, and store error.
• TestHandleScheduledTask_ClaimOnlyForTokenCarryingCollect.

Before the fix (with the store method and mock already present, without the handler change):

go test -race ./internal/server/ -run 'TestHandleScheduledTask_' -count=1
--- FAIL: TestHandleScheduledTask_DuplicateDeliveryKeepsWinnersMarker
    collection_invocation_test.go:171: Expected true: the lock-skipped duplicate must not clear the marker while the winner is still collecting
--- FAIL: TestHandleScheduledTask_ClaimInvocationOutcomes (3 subtests: scheduler got tok-marker, ClaimCollectionInvocation never called)
FAIL

After: ok github.com/LeanerCloud/cloud-commitments-platform/internal/server (also passed with -count=3 -race).

Real Postgres (testcontainers, integration tag). New file internal/config/store_postgres_collection_invocation_test.go. Its tests take their database from testhelpers.RequireMigratedDatabase (the package's shared container, #1165), so TestIntegrationTestsShareOnePostgresContainer passes:

go test -tags integration ./internal/config/ -run 'TestPostgresStore_ClaimCollectionInvocation|TestPostgresStore_ClearCollectionStarted' -count=1 -v
--- PASS: TestPostgresStore_ClaimCollectionInvocation (owned re-stamp + duplicate clear is a no-op, cleared marker re-established, fresh foreign marker left alone, stale foreign marker taken over, empty token error)
--- PASS: TestPostgresStore_ClaimCollectionInvocation_Races
    orderings observed: claim-first=13 clear-first=37   (50 iterations of claim vs duplicate clear, both orderings hit)
    concurrent claims elect one owner (8 claimers x 20 iterations, exactly one wins)
--- PASS: TestPostgresStore_ClearCollectionStarted_CompareAndClear

Mutation probe: dropping the free/stale branch from the claim's WHERE makes re-established, stale ... taken over and the claim-vs-clear race test fail.

Other checks

• go build ./..., and go vet on server/config/mocks/scheduler/analytics, with and without -tags integration.
• go test -count=1 for ./internal/server/ (also with -race and -tags integration), ./internal/mocks/ (repo-wide mock guards), ./internal/scheduler/, ./internal/analytics/, ./internal/config/ and ./internal/api/: all ok. go test -tags integration ./internal/config/ -run 'Collection|Recommendations': ok.
• golangci-lint v2.10.1 on the changed packages, with and without --build-tags integration: 0 issues.
• gocyclo -over 10 -ignore "_test\.go" on internal/server, config, mocks and scheduler: silent.
• scripts/gosec-hook.sh on the changed files: clean.
• git-secrets on the changed files in a fresh isolated repo: clean. scripts/test-git-secrets-allowlist.sh: 42 PASS, 0 FAIL.

Coverage gap: no real Lambda duplicate-delivery test. The duplicate is simulated by calling HandleScheduledTask twice with the same token against a shared lock fake. The SQL is exercised on real Postgres.

Out of scope

• The issue's "related, separate" note: the vacuous AssertNotCalled shape in internal/scheduler/scheduler_test.go, and the wider audit of AssertNotCalled call sites. Not touched here.
• internal/config/store_postgres_recommendations.go was already over the 500-line guideline before this change (603 lines) and grows by this method. Splitting it would be a separate refactor.

Maintainer: please mirror the issue's triage labels onto this PR: priority/p2, severity/medium, urgency/this-quarter, impact/few, effort/m, type/bug, triaged.

Closes #84

🤖 Generated with Claude Code

[coderabbitai]

Caution

Review failed

The pull request is closed.

Finishing Touches

🧪 Generate unit tests (beta)

• Create a new PR

• Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

_{Comment @coderabbitai help to get the list of available commands.}

[cristim]

VERDICT: APPROVE 52f4044

Reviewed at head 52f4044 (1 commit on base 8caed39). It merges cleanly onto current origin/main b6a0649 (git merge-tree gives tree 3d8e2413, no conflicts). All tests below ran on that merged tree.

Findings (none blocking):

• nit internal/server/collection_invocation_test.go:23 - The comment says the real SQL is tested in internal/config/store_postgres_recommendations_test.go. It is tested in store_postgres_collection_invocation_test.go. A reader who follows the pointer finds no test.
• nit internal/analytics/collector_test.go:500, internal/server/test_helpers_test.go:351 - These hard-code "mock-invocation-token" where mocks.MockInvocationToken exists (internal/mocks/stores.go:26). Harmless because no test asserts on these values.
• nit (process) - The single commit is 777 changed lines (760+/17-), over the 400-line cap. 630 of those lines are tests. Non-blocking.
• Out of scope, as the PR body says: the vacuous AssertNotCalled noted under "Related, separate" in Collection marker release is scoped to the marker, not the invocation #84 is still open.

Does it fix #84? Yes. The lock winner re-stamps last_collection_owner_id with a fresh UUID right after it takes the advisory lock (internal/server/handler.go:115). A lock-skipped duplicate then calls ClearCollectionStarted(markerToken), which no longer matches. This is the invocation-scoped identity the issue asked for.

Attacked:

• Overlapping Lambda runs.
  • Duplicate delivery: the winner keeps the marker. The real-PG claim-vs-clear race leaves the winner owning a set marker in both orderings (I observed claim-first=13 and clear-first=37 over 50 iterations).
  • Cron winner with a token-carrying run skipped: the skipped run still releases its own marker (unchanged).
  • A new refresh B whose marker is fresh: the winner gets ok=false, collects tokenless and never clears B's marker. B's skipped run releases B's marker. Same as before.
• Crash or timeout.
  • The owned branch keeps started_at from MarkCollectionStarted (store_postgres_recommendations.go:640), and the free/stale branch sets NOW(). Either way the marker goes stale at most 5 minutes after it was stamped, and MarkCollectionStarted's stale rule is unchanged. A stale marker cannot block collection forever.
  • A Lambda retry after a timeout, when the dead run still holds the lock, no longer drops the banner early. The marker expires through the 5-minute window.
  • A retry after an error re-establishes the marker through the free branch and collects again. Before this change it collected with no marker, so this is an improvement.
• ScheduledTaskParams. It has only OwnerToken (handler.go:420), so return ScheduledTaskParams{} on !ok drops nothing else.
• Other StoreInterface implementers. go build ./..., go vet ./... and go vet -tags integration ./... are all clean on the merged tree.
• Mutation probes (scratch copy; each one was restored and checked with cmp):
  • M1: remove the claim call at handler.go:115. The DuplicateDeliveryKeepsWinnersMarker, DuplicateClearsBeforeWinnerClaims and ClaimInvocationOutcomes tests FAIL.
  • M2: on !ok, keep the marker token. ClaimInvocationOutcomes FAILs.
  • M3: on a store error, drop the token. ClaimInvocationOutcomes FAILs.
  • M4: claim for tokenless runs. ClaimOnlyForTokenCarryingCollect FAILs.
  • M5: claim for any task type. ClaimOnlyForTokenCarryingCollect FAILs.
  • M6 (integration): drop the free/stale WHERE branch. "re-established", "stale ... taken over" and the claim-vs-clear race test FAIL.
  • M7 (integration): reset started_at in the owned branch. The "owned marker is re-stamped" test FAILs.
  • M8 (integration): drop the owned branch. The owned test, the race test and "concurrent claims elect one owner" FAIL.

Local results (merged tree, macOS, AWS_CONFIG_FILE=/dev/null):

• go test -race -count=1 ./internal/server/ ./internal/scheduler/ ./internal/mocks/ ./internal/analytics/: 1057 passed.
• go test -tags integration -count=1 ./internal/config/ -run 'ClaimCollectionInvocation|ClearCollectionStarted|Collection': all PASS on testcontainers Postgres, run under the heavy lock.

[cristim]

Independent review 2 (claude-fleet, cold): APPROVE 52f4044

Findings (none blocking):

• [non-blocking] internal/server/collection_invocation_test.go:22 - The markerStore doc points to internal/config/store_postgres_recommendations_test.go for the real SQL tests. They are actually in store_postgres_collection_invocation_test.go.
• [non-blocking] internal/server/collection_invocation.go:32 - The "another run holds the marker" branch returns ScheduledTaskParams{} instead of clearing OwnerToken on a copy of params.
  • Nothing breaks today, because the struct has only OwnerToken (handler.go:420).
  • A field added later would be silently dropped on this branch.
• [non-blocking] internal/analytics/collector_test.go:500 and internal/server/test_helpers_test.go:351 hard-code "mock-invocation-token". The PR adds mocks.MockInvocationToken for exactly this value.
• Behaviour change, documented and acceptable: a late redelivery after the original run finished now sets the banner again while it re-collects. Before, it collected with no banner.

Against #84: this matches the issue's proposed design and fixes the root cause.

• After winning the advisory lock, the winner swaps the shared marker token for its own invocation token (handler.go:115). A duplicate delivery that skips the lock (handler.go:111) therefore no longer matches.
• The free/stale branch in the SQL (store_postgres_recommendations.go:645-650) covers the gap between taking the lock and the re-stamp.
• Tokens are validated as UUIDs at parse time (handler.go:451).

Attacked:

• Interleavings:
  • a duplicate delivered while the winner collects;
  • a duplicate that clears the marker before the winner claims;
  • a late redelivery after completion;
  • a claim against a fresh marker from a newer refresh;
  • a stale foreign marker;
  • the cold-start collect, which marks without the lock;
  • a claim store error, which falls back to the old behaviour;
  • a run skipped behind cron, which still releases its marker.
• Blast radius:
  • All 5 implementers of ClaimCollectionInvocation compile.
  • Of HandleScheduledTask's 3 callers, lambda.go:340 carries a token. http.go:377 and cmd/server/main.go:61 pass none and are unaffected.
• Mutation probes on a scratch copy, each restored and confirmed identical with cmp. All 9 were caught:
  • Claim call removed: 3 tests FAIL.
  • "Held by another run" keeps the marker token: ClaimInvocationOutcomes FAILS.
  • A store error drops the token: ClaimInvocationOutcomes FAILS.
  • Task-type guard dropped: ClaimOnlyForTokenCarryingCollect FAILS.
  • Empty-token guard dropped: ClaimOnlyForTokenCarryingCollect FAILS.
  • Claim before the lock: DuplicateClearsBeforeWinnerClaims and TestHandleScheduledTaskReleasesMarkerWhenSkipped FAIL.
  • SQL without the free/stale branch: integration tests FAIL.
  • The CASE always sets NOW(): the started_at-preserved assertion FAILS.
  • SQL without the owned branch: the owned re-stamp and both race subtests FAIL.

Verified locally, on git archive exports with no cloud credentials, under the heavy lock:

• Head:
  • gofmt -l on the 10 changed files: clean.
  • go build ./..., with and without -tags integration: OK.
  • go vet and go test -count=1 on server, config, mocks, scheduler and analytics: ok.
  • go test -race -count=3 -run TestHandleScheduledTask_ ./internal/server/: ok.
  • go test -tags integration -run 'TestPostgresStore_ClaimCollectionInvocation|TestPostgresStore_ClearCollectionStarted' ./internal/config/ (testcontainers Postgres): all PASS, with both orderings observed.
• Fail-before: on base with the new store method, mock and tests but without the handler changes, DuplicateDeliveryKeepsWinnersMarker, DuplicateClearsBeforeWinnerClaims and ClaimInvocationOutcomes FAIL.
• Merged with origin/main:
  • merge-tree gives a clean tree.
  • Main has since made HandleScheduledTask a wrapper around runScheduledTask, and the claim lands inside runScheduledTask.
  • On that tree, build, vet, the 5 packages, the race run and the integration store tests all pass.
• No migration, because the column already exists (000093).

Reviewed by an independent agent with no author context (second reviewer). Review only: nothing was pushed, and the GitHub review state is unchanged.

[IAmJSD]

Addressed the VERDICT and Independent review 2 findings on 52f4044. I merged origin/main (a14dc8f) into the branch. Nothing was rebased.

Merge with main

• git merge origin/main had no textual conflicts. Two things broke on the merged tree, and both are fixed in d0f151e:
  • TestIntegrationTestsShareOnePostgresContainer (test(config): share one migrated Postgres container across integration tests #1165) flagged this PR's two new integration tests, because each started its own container. They now get their database from testhelpers.RequireMigratedDatabase(ctx, t, getMigrationsPath()), the same pattern as setupRecommendationsStore (internal/config/store_postgres_collection_invocation_test.go:65, :165).
  • Main added AdvisoryLockHeldFor to TaskLocker (chore: minor follow-ups from the #373/#376/#380/#383 reviews #394), so the test fake sharedTaskLocker no longer compiled under go vet. It now reports a held lock as freshly taken, so a lock-skipped run still takes the normal "already_running" path (internal/server/collection_invocation_test.go:118).

Findings

• nit, stale pointer to the real-SQL tests: fixed. It now points at store_postgres_collection_invocation_test.go (internal/server/collection_invocation_test.go:24). The claim's doc comment also now names runScheduledTask, where the claim lives since main split HandleScheduledTask (internal/server/collection_invocation.go:9).
• non-blocking, return ScheduledTaskParams{} on !ok: fixed. Both branches now set OwnerToken on the incoming params and return them, so a field added later is not dropped (internal/server/collection_invocation.go:32, :35).
• nit, hard-coded "mock-invocation-token": fixed. Both fakes now return mocks.MockInvocationToken (internal/analytics/collector_test.go:516, internal/server/test_helpers_test.go:367).
• Behavior change (a late redelivery shows the banner again): documented in the PR body's Fix section.
• nit (process), 777-line commit: left as is. The follow-up commit is small.
• Out of scope: the vacuous AssertNotCalled from Collection marker release is scoped to the marker, not the invocation #84's "Related, separate" note. Left alone, as the body says.

New head: d0f151e

Verification on the merged tree (Linux, Go 1.26.9, AWS_CONFIG_FILE=/dev/null)

• go build ./... with and without -tags integration: OK.
• go vet, with and without -tags integration, on server, config, analytics, mocks and scheduler: clean.
• go test -count=1, one package at a time, on server, analytics, mocks, scheduler and config: ok. go test -race -count=3 -run TestHandleScheduledTask ./internal/server/: ok.
• go test -tags integration -timeout 20m ./internal/config/ -run 'TestIntegrationTestsShareOnePostgresContainer|TestPostgresStore_ClaimCollectionInvocation|TestPostgresStore_ClearCollectionStarted': all PASS. The race test saw both orderings (claim-first=9, clear-first=41).
• Whole ./internal/config/ package with -tags integration, skipping only TestPostgresStoreDB_ResolveStaleRIExchange_ValidatesCloudCheck (fixed separately in fix(config): compare resolve checked_at at the database's microsecond precision #1252): ok (305s).
• Mutation probe on the shared-container tests: removing the free/stale branch of the claim's WHERE makes "re-established", "stale ... taken over" and the claim-vs-clear race test FAIL. The source was restored afterwards.
• ./internal/server/ with -tags integration: everything passes except TestServerIntegration. That test is not touched here. It fails before reaching any of this code: its 30s testutil.TestContext runs out while the Postgres container is still starting ("matched 0/1 times, expected 2") on a shared, loaded machine.
• golangci-lint v2.10.1 on server, config and analytics, with and without --build-tags integration: 0 issues. gocyclo -over 10 on non-test files: silent.
• git-secrets: the changed files were added to a fresh isolated repo and came out clean, and a planted fake key was flagged. scripts/test-git-secrets-allowlist.sh: 137 PASS, 0 FAIL.

[cristim]

VERDICT: APPROVE d0f151e

Re-review of #1099 (closes #84). Old head 52f4044, new head d0f151e. origin/main is 00a3f40, merge-tree c80afe0b, clean.

Delta (PR-only diff, old base 8caed39 vs new base a14dc8f):

• Merges origin/main (1520270). The hook now sits in runScheduledTask (main renamed it from HandleScheduledTask).
• internal/server/collection_invocation.go:31-36: the claim now changes params.OwnerToken in place and keeps every other ScheduledTaskParams field. The old code returned a fresh struct, which on the new main would have dropped any other params fields. This is a real fix.
• internal/mocks/stores.go:23: adds a MockInvocationToken constant, used in the analytics and server test mocks.
• internal/server/collection_invocation_test.go: the sharedTaskLocker fake gains AdvisoryLockHeldFor, which main's stuck-lock check (chore: minor follow-ups from the #373/#376/#380/#383 reviews #394) needs.
• d0f151e: the config integration tests now use testhelpers.RequireMigratedDatabase (shared container).

Findings: none blocking.

• nit internal/config/store_postgres_recommendations.go:639-641: when the marker is still owned by markerToken, the claim keeps started_at. If a delivery is delayed more than 5 minutes, the re-stamped marker is already stale, so a refresh can take it over mid-collect, and that refresh's lock-skipped delivery clears it. The result is an early banner drop, same as before this PR. Setting NOW() unconditionally would close it. This matches the documented design, so it is not a regression.
• nit: the branch updates main by merge commits, not by rebasing.

Concurrency, re-checked against the claim UPDATE (row-lock serialized, READ COMMITTED re-check):

• Claim before the duplicate's clear: the clear does nothing.
• Clear before the claim: the claim takes the free marker.
• Fresh foreign marker: the run collects with no token and never clears.
• Store error: the run keeps the marker token (pre-Collection marker release is scoped to the marker, not the invocation #84 behavior).
• Late redelivery: it re-establishes the marker.
• Cron overlap: still releases.
• A refresh landing between the winner's lock and its claim leaves the same early-clear residual as before.
• Non-Lambda paths and the cold-start kick never reach the claim, which is correct.

Results on the merged tree:

• go build ./... and go vet (unit + integration tags): ok.
• go test for server, analytics, scheduler and mocks: 1075 passed.
• -tags integration for ClaimCollectionInvocation, _Races and ClearCollectionStarted_CompareAndClear: pass.

Mutation probes (all reverted):

• Drop the claim call in handler.go: 3 server tests FAIL.
• Keep the token when the claim returns !ok: ClaimInvocationOutcomes FAIL.
• Drop the token on a claim error: ClaimInvocationOutcomes FAIL.
• Drop the SQL free/stale branch: 3 integration subtests FAIL.
• Drop only the stale branch: "stale marker taken over" FAIL.
• Make the CASE always NOW(): "owned marker re-stamped" FAIL.

CI on d0f151e (run 39894096060, 2026-10-23, base before the Go bump):

• Integration Tests fail in TestPostgresStoreDB_ResolveStaleRIExchange_ValidatesCloudCheck. That test is unrelated to this PR, the same failure appears on fix(config): use the earliest row when purchase_history duplicates a purchase_id #1103, and it passes locally on the merged tree.
• Security Scanning fails: govulncheck on go1.26.9. Main fixed this in 52edcf5 (Go 1.26.10).
• Docker amd64/arm64 fail: image advisory with a published fix.
• None of the failures is caused by the PR; they come from the old base. Main CI is green at 00a3f40. A rebase or re-run is needed before merge.

@IAmJSD CI on this head is red only from the old merge base (govulncheck on go1.26.9, fixed on main by #1236; a Docker image advisory; and TestPostgresStoreDB_ResolveStaleRIExchange_ValidatesCloudCheck, which passes on the merged tree). Please merge or rebase onto current main so CI runs on a fresh merge ref.

[cristim]

VERDICT: APPROVE dff10b0

Delta re-review after the refresh onto main (previously approved at d0f151e).

• Only new commit: dff10b0, a merge of origin/main (503eb2e) into d0f151e. git show --remerge-diff is empty: no conflicts and no hand edits.
• The PR diff against its merge base is byte-identical to the approved one (git diff a14dc8f2 d0f151ec vs git diff 503eb2ef dff10b03). Only the hunk offsets and blob indexes differ.
• Semantic overlap: main's feat(api): scope-aware SQL totals for the History summary cards #1200 added HistorySummaryAggregate to StoreInterface and the mocks (interfaces.go, mocks/stores.go, analytics/collector_test.go, server/test_helpers_test.go). Both sets of methods coexist and go build ./... passes.
• Tests on the new head (fresh archive): go vet on the touched packages is clean. Unit tests pass for internal/{server,scheduler,analytics,config,mocks}. The -tags integration -run Collection ./internal/config/ run passes, including ClaimCollectionInvocation and its _Races test, CompareAndClear, and SetRecommendationsCollectionError_DoesNotClearOwnerMarker.
• CI: gh pr checks reports 33 pass and 1 skipping. mergeStateStatus is CLEAN. The PR is still a draft.
• Note: main has since moved to 2ded648 (fix(api): reject an explicit max_amount of 0 on API-key create #1406, fix(ci): verify the rollback image in AWS_ACCOUNT_ID's registry #1112, an auth test commit). Those touch only internal/server/app.go and two unrelated test files, with no overlap with this PR's files.

[cristim]

Independent review 2 (claude-fleet, cold), re-review: APPROVE dff10b0

Brief correction: #1099 is fix/issue-84 (scheduler collection marker scoped to the invocation, Closes #84), not the ladder PR. f1bdefa is #1199's commit. The previous #1099 head in the local fetch reflog is 52f4044, which IS an ancestor of dff10b0: the branch was fast-forwarded with merges, not rebased, so there is no range-diff to compare. Delta reviewed: 52f4044..dff10b0.

Delta (first-parent): merge 1520270 (main a14dc8f), d0f151e "test(config): move the #84 store tests onto the shared Postgres container", merge dff10b0 (main 503eb2e). Both merges have an EMPTY remerge-diff (no hand resolution). dff10b0 + current main (2ded648) merges cleanly.

d0f151e: the two ClaimCollectionInvocation integration tests use testhelpers.RequireMigratedDatabase; sharedTaskLocker gains AdvisoryLockHeldFor (main added it to TaskLocker); claimCollectionInvocation mutates a copy of params instead of returning a fresh struct (internal/server/collection_invocation.go:31-35). ScheduledTaskParams has only OwnerToken (internal/server/handler.go:434), so the behavior is identical today, and future fields stay intact. The other changes are test fakes moved to mocks.MockInvocationToken.

Findings: none.

Attacked:

• The claim of red CI from the old base. 52f4044 merged with main fails in two ways. go vet ./internal/server/ fails to compile: *sharedTaskLocker does not implement TaskLocker (missing method AdvisoryLockHeldFor). TestIntegrationTestsShareOnePostgresContainer FAILS, flagging store_postgres_collection_invocation_test.go:66 and :169. Both pass on the new head merged with main.
• Interaction with main's stuck-lock path (internal/server/handler.go:114-120): the skip still releases only its own token before stuckTaskLockError. A retry finds the marker cleared, and the claim's free branch re-establishes it. No regression.
• Probes on the merged export (each restored, cmp-verified):
  • P1: !ok keeps the marker token. ClaimInvocationOutcomes FAILS.
  • P2: success keeps the marker token. Three server tests FAIL.
  • P3: claim never called in runScheduledTask. DuplicateDeliveryKeepsWinnersMarker, DuplicateClearsBeforeWinnerClaims and ClaimInvocationOutcomes FAIL.
  • P4: drop the free/stale branch from the claim SQL. Three integration subtests, including the claim-vs-clear race, FAIL.

Verified locally (go1.26.10, export of merge-tree origin/main + dff10b0, under the heavy lock):

• go build ./... ok.
• go vet server/config/analytics/scheduler/mocks ok, also with -tags integration for config and server.
• gofmt -l on the changed files: clean.
• go test -count=1 on the same five packages: ok. -race on server collection tests: ok.
• go test -tags integration ./internal/config/ -run 'ClaimCollectionInvocation|ClearCollectionStarted|ShareOnePostgresContainer|Collection': all PASS (orderings claim-first=13, clear-first=37).
• No migration.

Reviewed by an independent agent with no author context (second reviewer). Review only: nothing was pushed, and the GitHub review state is unchanged.

[IAmJSD]

Local verification at dff10b0352a60b261655da3a26e1c64cad5a4f2d merged onto main (1c5787a0), together with #1099, #1119 in that order (combined tree a047b78c, Linux, Go 1.26.10): go build ./..., go vet with and without -tags integration, the full go test -race -short -count=1 ./..., and go test -race -tags integration -count=1 on every changed package, against Docker Postgres. All pass. CI Success and pre-commit are green on this head, with 1 approving independent review(s) on it. Nothing was run against any cloud.

[IAmJSD]

Merged by IAmJSD's agent at IAmJSD's request (IAmJSD is acting owner while the maintainers are away), under the bar IAmJSD set: 1 approving independent review(s) on dff10b03, green CI, and the combined-tree verification above. Squash-merged at exactly dff10b0352a60b261655da3a26e1c64cad5a4f2d.