Unify the three keyed-mutex implementations into a shared package
Nobody has claimed this yet.
- Dominant language
- Go
- Stars
- 2.2k
- Forks
- 300
- Avg merge
- 1d 15h
- Merged PRs (30d)
- 184
Description
Context
A duplication review of #5934 (server→client routing in the streamable proxy) found the repo now has three near-identical hand-rolled "per-key mutex" implementations of the same two-phase pattern (acquire a global mutex → look up/create a per-key sync.Mutex → release the global mutex → block on the per-key mutex):
pkg/plugins/pluginsvc/service.go—pluginLock(never evicts; key cardinality bounded by installed plugins)pkg/skills/skillsvc/service.go—skillLock(never evicts; bounded)pkg/transport/proxy/streamable/keyed_mutex.go—keyedMutex(added in #5934; ref-counted / self-evicting because keys are arbitrary client-supplied resource URIs with unbounded cardinality, and it is deliberately held across a blocking upstream round-trip)
The keyedMutex variant intentionally diverges (eviction is required to avoid an unbounded-map leak — do not collapse it into the never-evicting variants), but the core acquire/release logic is duplicated three ways.
Proposed work
Extract a single generic keyed-mutex into an internal package (e.g. pkg/keyedmutex) supporting both modes:
- non-evicting (for the small, bounded
pluginsvc/skillsvckey sets), and - evicting / ref-counted (for unbounded, client-supplied keys like the streamable proxy's URIs).
Port all three call sites to it. Preserve each site's current semantics exactly (especially the streamable proxy's ref-counted eviction and its "held across a blocking call" usage — see the doc comment on keyedMutex).
Notes
- Not a behavior change; pure consolidation + tests for the shared package.
- Confirmed as a follow-up (not a blocker) by the #5934 review panel.
Generated with Claude Code
Contributor guide
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
Research direction
Start by reading the three existing implementations in pkg/plugins/pluginsvc/service.go, pkg/skills/skillsvc/service.go, and pkg/transport/proxy/streamable/keyed_mutex.go, including the streamable proxy's doc comment. Then inspect their current usage and tests, and define the shared package's non-evicting and ref-counted modes. Done means all three call sites use it while preserving their existing locking, eviction, and blocking-call semantics.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- go
- Domain
- backend
- Issue type
- Refactor
- Difficulty
- 4/5
- Estimated time
- 3-5 days
- Activity status
- Quiet
- Clarity
- Mostly clear
- Newbie friendliness
- 52/100