fix: scope RemoveFromQueue to the calling controller

[HarnageaGabriel]

• Ran make pre-commit -B
• Signed-off commits with Conventional Commit messages
• PR title is a conventional commit message (it becomes the release notes entry)
• Unit or e2e tests cover the change
• For features: an associated issue and a feature description file (make feature-new) — not a feature, bug fix only
• Opened as draft; will mark "Ready for review" once builds are green

Fixes #16737

Motivation

Multi-controller sync with a shared database: CheckWorkflowExistence's garbage collector lists pending sync_state rows by lock name only (no controller filter), so it sees rows from every controller sharing the database. It checks each key against the local informer only, so a workflow owned by another controller always looks missing locally and gets deleted. RemoveFromQueue had no controller filter — unlike ReleaseHeld, which already filters on controller — so the delete went through and removed other controllers' legitimate pending rows, breaking lock handover and queue ordering across controllers.

Modifications

• util/sync/db/queries.go: RemoveFromQueue now takes controllerName and filters the delete on it, mirroring ReleaseHeld.
• workflow/sync/database_semaphore.go: removeFromQueue passes s.info.Config.ControllerName through.
• util/sync/db/mocks/SyncQueries.go: mock updated to match (mockery not available locally; hand-edited to match the existing generated style, e.g. ReleaseHeld's mock).
• workflow/sync/database_semaphore_test.go: regression test asserting the controller name is passed through to RemoveFromQueue.

Existing callers (Release, ReleaseAll, the GC) only ever pass local keys, so behavior for a single-controller setup is unchanged.

Verification

• go build ./util/sync/... ./workflow/sync/...
• KUBECONFIG=/dev/null go test ./util/sync/... ./workflow/sync/... — 121 passed
• gofmt -l clean on changed files
• golangci-lint run clean on changed files (pre-existing gofmt findings on unrelated files in the package, untouched by this change)
• Added regression test covering the controller-scoping fix

Documentation

No user-facing documentation change needed — this is an internal bug fix to sync-queue cleanup, no API/CRD/CLI surface changed.

AI

Claude Code was used to investigate the bug and draft the fix; I reviewed the diff and verified the build/tests myself before pushing.

[github-actions]

✅ PR readiness: all clear

All contributor-fixable checks are passing. A maintainer will take it from here — thanks!

---

_{🤖 Automated PR-readiness helper — it re-checks each time CI finishes. Unit/E2E test results are not covered here. Questions? See the contributing guide or ask a maintainer.}

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: 176da26f-f184-42b7-8b38-3b09bc60248f

📥 Commits

Reviewing files that changed from the base of the PR and between 2cba004 and e30c9f5.

⛔ Files ignored due to path filters (1)

• util/sync/db/mocks/SyncQueries.go is excluded by !**/mocks/**

📒 Files selected for processing (3)

• util/sync/db/queries.go
• workflow/sync/database_semaphore.go
• workflow/sync/database_semaphore_test.go

Included review availability: Your plan includes up to 10 reviews per rolling hour; 9 remain after this review.

---

📝 Walkthrough

Walkthrough

RemoveFromQueue now requires a controller name and filters pending-row deletion by semaphore, holder key, and controller. databaseSemaphore passes its controller name, and a unit test verifies the scoped call.

Changes

Controller-scoped queue removal

Layer / File(s)	Summary
Query contract and controller filter util/sync/db/queries.go	SyncQueries.RemoveFromQueue accepts controllerName. Its deletion query filters rows by controller in addition to semaphore, holder key, and pending state.
Caller wiring and verification workflow/sync/database_semaphore.go, workflow/sync/database_semaphore_test.go	removeFromQueue passes ControllerName. The unit test verifies the expected arguments and successful completion.

Estimated code review effort: 2 (Simple) | ~10 minutes

Merge Risk: ⚪ Minimal · up to e30c9

This change limits pending-row removal to the calling controller, preventing cross-controller queue corruption while preserving single-controller behavior. No actionable merge-blocking risk remains after normal checks and review.

🚥 Pre-merge checks | ✅ 3 | ❌ 2

❌ Failed checks (1 warning, 1 inconclusive)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 66.67% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.
Linked Issues check	❓ Inconclusive	The filter and regression test address #16737, but the required mock update is excluded by !/mocks/ and cannot be verified.	Review util/sync/db/mocks/SyncQueries.go to confirm RemoveFromQueue includes controllerName and matches the updated interface.

✅ Passed checks (3 passed)

Check name	Status	Explanation
Out of Scope Changes check	✅ Passed	The reviewed changes directly support controller-scoped queue removal and regression coverage for issue #16737.
Description check	✅ Passed	The description covers the required checklist, motivation, modifications, verification, documentation, issue reference, and AI disclosure.
Title check	✅ Passed	The title clearly and concisely describes scoping RemoveFromQueue to the calling controller.

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

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