picatz / picatz/flowstate

sdk: nothing tells a plugin author that Task.Fn runs concurrently — the SDK synchronizes itself and never says the author must too

Open Beginner friendly
#1,341 0 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

documentation engine enhancement
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

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. 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

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.