picatz / picatz/flowstate

Internal generics wrapper for the plugin/embedding task boundary (not the Temporal SDK)

Open
#786 0 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

design engine kind/design-record
Dominant language
Go
Stars
9
Forks
0
Avg merge
3h 3m
Merged PRs (30d)
509

Description

Problem

The owner asked me to research whether an internal (unpublished) generics-based wrapper around Temporal, in the spirit of jlegrone/temporal-sdk-go-generics, would fit Flowstate — and to think specifically about golang/go#77273 (generic methods, Proposal-Accepted, Go 1.27 milestone). This issue records that investigation and what it actually found.

jlegrone's library wraps Temporal's own SDK: workflow.NewActivityClient(Activity) returns a typed client whose .Run(ctx, args) replaces workflow.ExecuteActivity(ctx, "name", args).Get(ctx, &result). It exists because a typical Temporal app registers many distinct, individually-named Go activity and workflow functions, each with its own request/response shape, and every call site would otherwise stringly-type the name and interface{}-type the result.

Flowstate's own engine does not have that shape. pkg/flowstate/v1/engine/versioning.go:75-101 (Register) installs exactly one workflow (Run) and seven fixed activities (Task, TaskInScope, TaskAuthorized, TaskInScopeAuthorized, WorkflowVars, CheckPlugins, TaskWithPrev), once, for the life of the worker. Every step of every workflow anyone writes dispatches through the same four arms, chosen only by two booleans (scope != nil, needsAuthority) — see the dispatch doc at pkg/flowstate/v1/engine/execute.go:826-877, which says so explicitly: "Four activities rather than one... a compensation goes through the same four arms as an ordinary step." Task identity is data (*v1.Task, a proto message carrying a task name string), not a Go function reference, because task dispatch is DSL-driven. jlegrone's core value — turning many stringly-named, individually-typed Go functions into compile-time-checked calls — has almost nothing to replace here: there are 7 activity names, not 70, and they're already all statically referenced Go identifiers in one file (Task, TaskInScope, etc., not string literals — the two "TaskAuthorized"/"TaskInScopeAuthorized" string names at execute.go:864,872 are RegisterOptions.Name overrides for the same statically-referenced functions, not stringly-typed dispatch). Porting jlegrone wholesale onto this dispatch would be building compile-time protection for a call graph that is already this small and already type-checked by hand — a solution in search of the problem it advertises solving.

Where the pattern's actual value does show up is one boundary over: the plugin/embedding task-authoring surface, which genuinely has the many-individually-typed-functions shape jlegrone targets — just not against the Temporal SDK. Every plugin task function in this repo, and every embedding-example task, follows the identical three-step shape by hand:

func gitLog(ctx context.Context, inputs map[string]*flowstatev1.Value, _ *flowstatev1.Scope) (*flowstatev1.Node_Outputs, error) {
    var in gitv1.LogInputs
    if err := sdk.DecodeInputs(inputs, &in); err != nil {
        return nil, sdk.InvalidInput("%v", err)
    }
    // ... work against &in ...
    return sdk.EncodeOutputs(out)
}

(plugins/git/log.go:27-31,90; the same shape recurs at plugins/git/read_file.go, plugins/git/refs.go, plugins/git/commit_push.go, plugins/vcs/log.go, plugins/vcs/diff.go, plugins/github/issue_get.go:26-30, plugins/github/issue_list.go, plugins/github/issue_comment.go, plugins/github/pull_request_get.go, plugins/github/pull_request_list.go, plugins/github/pull_request_files.go, plugins/sql/query.go, plugins/sql/exec.go, plugins/codex/exec.go, and the worked example at pkg/flowstate/v1/plugin/sdk/sdk.go's package doc / pkg/flowstate/v1/plugin/examples/flowstate-plugin-example/main.go:160-201.)

sdk.Task (pkg/flowstate/v1/plugin/sdk/sdk.go:202-296) and embed.Task (pkg/flowstate/v1/embed/tasks.go:19-44) both declare Input/Output (or Inputs/Outputs) as proto descriptors separately from Fn, whose signature is the untyped TaskFunc func(ctx, map[string]*Value, *Scope) (*Node_Outputs, error) (pkg/flowstate/v1/plugin/sdk/sdk.go:301). Nothing today ties the declared Input/Output message types to what Fn actually decodes into and encodes from — a task could declare Input: &FooInputs{} and internally DecodeInputs(inputs, &BarInputs{}), and it would compile and run, silently doing the wrong thing per CLAUDE.md's "Diagnostics are a feature" concern about silent wrongness, just one layer down (a Go-level mismatch rather than a Flowfile-level one). This is exactly the class of boilerplate-plus-latent-bug jlegrone's pattern removes, applied to Flowstate's own plugin SDK boundary rather than to the Temporal SDK.

Desired outcome

A small, internal (unexported from the module's public promise — same posture as the rest of plugin/sdk, not "for external consumption" per the task framing), generic helper that lets a plugin or embedding task be written as a typed function and turned into the existing untyped TaskFunc mechanically — removing the hand-written DecodeInputs/EncodeOutputs pair at every call site and making the declared Input/Output descriptor agree with Fn's actual parameter/return types by construction, not by convention.

Explicitly not in scope: anything touching workflow.ExecuteActivity, worker.RegisterActivity, or the engine's dispatch function in execute.go. Per the investigation above, that surface does not have the problem this pattern solves, and forcing it there would be a second, needless dispatch mechanism sitting beside the one execute.go:826 already documents as deliberately minimal.

Constraints

  • Proto-first stays intact. The wrapper introduces no new schema. In/Out remain the same generated proto.Message types (gitv1.LogInputs, examplev1.GreetOutputs, etc.) the task already declares in .proto; the wrapper only removes the hand-written glue between them and the untyped Fn. It does not become a fourth spelling of "task shape" beside TaskDef, TaskDescription, and plugin/v1's TaskManifest (pkg/flowstate/v1/registry.go:40-57 already names why those three exist and are derived, not duplicated) — this wrapper produces a TaskFunc/Task, it does not describe a task's shape to anything outside the process.
  • Both-drivers-agree does not apply. This is entirely on the plugin/embedding side of the process boundary — a plugin process, or an embedding program's own binary — never inside pkg/flowstate/v1/engine, so it cannot create a local/durable disagreement; both drivers call the same TaskFunc this produces, unchanged.
  • No behavior change for an existing plugin unless that plugin is deliberately migrated. The wrapper has to compile down to today's DecodeInputs/EncodeOutputs calls exactly, not a reimplementation of them.
  • Cost of building and keeping it: one new small package, its own tests (encode/decode round-trip, nil-message handling matching DecodeInputs/EncodeOutputs's existing nil checks at pkg/flowstate/v1/plugin/sdk/values.go:34-37,274-277), and a "why this exists and where it stops" doc comment in the tradition CLAUDE.md's proto-first section asks for boundary types. It's a maintenance surface nobody has today, in exchange for deleting ~5 lines at each of ~15 real call sites and closing the Input/Output-vs-Fn mismatch class above.

Acceptance criteria

  • A capability is not done until it's reachable (CLAUDE.md), so this lands only if at least one real plugin task in this repo is migrated to it and its existing tests still pass unchanged — proving the wrapper is a strict rewrite of the current shape, not a new one living beside it.
  • go vet/staticcheck clean, per the usual gate.
  • The wrapper package's doc comment states plainly that it is internal-only glue for the plugin/embedding boundary and is not a Temporal SDK wrapper, so nobody mistakes it for the broader thing this issue explicitly declined to build.

Sketch — illustrative, not the landed shape

// package tasktyped (name illustrative) — pkg/flowstate/v1/plugin/sdk/tasktyped/tasktyped.go

// Func is a task's typed body: decode already done, encode still to do.
type Func[In, Out proto.Message] func(ctx context.Context, in In, scope *flowstatev1.Scope) (Out, error)

// Wrap turns a typed task body into the sdk.TaskFunc every Task.Fn expects,
// using In's own zero value to decode and Out's descriptor to encode — the
// same two calls every hand-written task body makes today
// (sdk.DecodeInputs / sdk.EncodeOutputs), just no longer duplicated per task.
func Wrap[In, Out proto.Message](fn Func[In, Out]) sdk.TaskFunc {
    return func(ctx context.Context, inputs map[string]*flowstatev1.Value, scope *flowstatev1.Scope) (*flowstatev1.Node_Outputs, error) {
        in := newProto[In]()
        if err := sdk.DecodeInputs(inputs, in); err != nil {
            return nil, sdk.InvalidInput("%v", err)
        }
        out, err := fn(ctx, in, scope)
        if err != nil {
            return nil, err
        }
        return sdk.EncodeOutputs(out)
    }
}

What plugins/git/log.go would look like after migrating (only the signature and the two boilerplate lines change; everything below in.GetUrl() etc. is untouched):

func gitLog(ctx context.Context, in *gitv1.LogInputs, _ *flowstatev1.Scope) (*gitv1.LogOutputs, error) {
    repoURL, err := validateRepositoryURL(in.GetUrl())
    // ...unchanged...
    return doLog(ctx, logParams{ /* ... */ })
}

registered as Fn: tasktyped.Wrap(gitLog) in plugins/git/main.go, with Input: &gitv1.LogInputs{}, Output: &gitv1.LogOutputs{} now redundant with — and checkable against — gitLog's own signature at the tasktyped.Wrap call site, rather than living as two unconnected facts the way sdk.Task{Input, Output, Fn} does today.

The Go 1.27 generic-methods angle (#77273)

#77273 proposes methods that declare their own type parameters (func (*Reader) Read[E any](...)), independent of any type parameter on the receiver — accepted, Go 1.27 milestone, not shipped. #495 already flagged this in the release as "worth knowing... not a reason to change working code," with no concrete site named. This investigation found one.

embed.Tasks (pkg/flowstate/v1/embed/tasks.go:19-100) is deliberately a non-generic concrete type — it holds a set of heterogeneous tasks, each with its own In/Out, the same reason sdk.Task.Input/Output are plain proto.Message rather than a type parameter (a registry of many different-shaped things cannot itself be generic over one shape). Its one registration method is func (t *Tasks) Register(task Task) error (tasks.go:100), and Task.Fn is untyped v1.TaskFunc.

With today's Go, there is no way to add a typed registration method on *Tasks itself — func (t *Tasks) Register[In, Out proto.Message](fn Func[In, Out]) error is exactly the method-level type parameter #77273 proposes and current Go forbids. The only options today are (a) a free function taking *Tasks as a parameter, tasktyped.Register[In, Out](tasks *embed.Tasks, name string, fn Func[In, Out]) error, which is what the sketch above amounts to when wired into registration, or (b) building a Task value with tasktyped.Wrap and passing it to the existing untyped Register method, which is what the sketch above actually proposes, precisely because a method-shaped API isn't available pre-1.27. Once #77273 lands, tasks.Register[In, Out](name, fn) becomes the natural spelling and the free-function/wrap indirection can be dropped — a concrete, named place this project would take something from that release, beyond #495's "worth knowing" note.

This is not a reason to wait for Go 1.27 to build the wrapper above; the free-function/Wrap shape works fine today and is what's proposed. It's the answer to the "think about the generic-methods angle" half of this task: yes, there is a real site, and it's this one.

Relationship to other issues

  • Extends #495's Go 1.27 comment (the "Generic methods... worth knowing" paragraph) with the one concrete site this investigation found; not a duplicate, since #495 explicitly deferred all Go-1.27-specific action until the toolchain bumps, and this issue's tasktyped.Wrap sketch works on the current go 1.26 pin.
  • Not a duplicate of #516 (protobuf Opaque API) — different question, similar "decide and record" shape.
  • No existing issue mentions generics, jlegrone, or a Temporal SDK wrapper (searched repos/picatz/flowstate/issues for "generic" in title/body: no hits).

Decision needed

  1. Build tasktyped (or similarly named) as scoped above, migrate one real plugin task to prove it, and stop there — or decline entirely?
  2. If built: which package path, and does it live under plugin/sdk/ (plugin-only) or somewhere both plugin/sdk and embed can import (since both have the identical TaskFunc shape)?

Generated by Claude Code

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 pkg/flowstate/v1/plugin/sdk/sdk.go and values.go, then compare the task shapes in pkg/flowstate/v1/embed/tasks.go. Inspect the existing pattern in plugins/git/log.go and its registration in plugins/git/main.go before designing the internal typed wrapper. Done means a tested wrapper, one real plugin migration, unchanged existing plugin tests, and clean go vet/staticcheck output.

Written by the indexing model from the issue text.

Assessment

Tech stack
go
Domain
developer-experience
Issue type
Feature
Difficulty
4/5
Estimated time
3-5 days
Activity status
Quiet
Clarity
Clearly specified
Newbie friendliness
55/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.