sdk: nothing tells a plugin author that Task.Fn runs concurrently — the SDK synchronizes itself and never says the author must too
Nobody has claimed this yet.
- Dominant language
- Go
- Stars
- 9
- Forks
- 0
- Avg merge
- 3h 3m
- Merged PRs (30d)
- 509
Description
Problem
Concurrent invocations of one plugin task's Fn are a normal occurrence, not an edge: the host's transport allows up to 8 connections per plugin (pkg/flowstate/v1/plugin/transport.go:95, MaxConnsPerHost: 8), and the SDK serves each Connect request on its own goroutine with no serialization around task.Fn. Two steps calling the same task — in one run's parallel branches or across concurrent runs on one worker — execute Fn concurrently.
The SDK knows this about itself: its progress-reporting path takes a mutex precisely because the underlying stream "is not safe for concurrent use" (sdk/sdk.go:1062-1065). It never passes the requirement on: neither sdk.Task/TaskFunc's doc comments nor docs/PLUGINS.md state that Fn must be safe for concurrent use. PLUGINS.md's "Five places the contract is implicit" and "Three traps the code knows about and no authoring surface teaches" sections — which exist to enumerate exactly this class of unwritten contract — do not mention it. An author who keeps unsynchronized package-level state (a lazily-built client, a counter, a cache) gets no signal until it breaks under load, where it is least debuggable.
Desired outcome
State the contract where authors read: sdk.Task.Fn/TaskFunc doc comments say "safe for concurrent use," and PLUGINS.md's implicit-contract section gains the entry, including the one nuance worth a sentence — per-call state is fine, shared mutable state needs the same care any Go HTTP handler needs. If the conformance direction (#713's harness) grows teeth, a concurrent-invocation probe is a natural black-box check.
Acceptance
- The doc comment and PLUGINS.md both state the requirement; the walkthrough's own example remains a correct model (it already is — pure per-call code).
- Whether the connection ceiling should be operator-tunable is explicitly out of scope here (adjacent to #1309's bounding discussion), so this stays a documentation-contract fix.
Adjacency
#713 (the conformance harness this could feed), #1309 (operator ceilings on plugin RPC — the tunability half lives there if anywhere), PLUGINS.md's implicit-contract sections (the pattern this completes).
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 with the sdk.Task.Fn and TaskFunc doc comments, then read the implicit-contract sections in docs/PLUGINS.md. Document that Fn must be safe for concurrent use, including the distinction between per-call state and shared mutable state. Confirm that the walkthrough example remains correct and that both documented locations state the requirement.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- go
- Domain
- documentation
- Issue type
- Documentation
- Difficulty
- 2/5
- Estimated time
- 1-3 hours
- Activity status
- Active
- Clarity
- Clearly specified
- Newbie friendliness
- 88/100