Skip to content

feat(cli): add conduit connector new / conduit processor new scaffolding#2577

Merged
devarismeroxa merged 5 commits into
mainfrom
feat/cli-connector-processor-new
Jul 8, 2026
Merged

feat(cli): add conduit connector new / conduit processor new scaffolding#2577
devarismeroxa merged 5 commits into
mainfrom
feat/cli-connector-processor-new

Conversation

@devarismeroxa

Copy link
Copy Markdown
Contributor

Summary

Implements conduit connector new --lang go and conduit processor new --lang go
per docs/design-documents/20260707-connector-processor-scaffolding.md (open in
#2571 — that design doc + the shared 20260707-cli-output-conventions.md spec
this PR follows are not yet merged; this PR's exit codes / error envelope /
preflight wiring were built directly against their reviewed content regardless
of merge order). Roadmap: Phase 1 "Plugin scaffolding" (ROADMAP.md).

Approach (design doc's Option C): orchestrate the canonical upstream
templates (ConduitIO/conduit-connector-template,
ConduitIO/conduit-processor-template) rather than reimplementing their
boilerplate. pkg/scaffold/template vendors a pinned go:embed'd snapshot of
each (SHAs in embed.go) and Go-ports each template's setup.sh rename step
(Rewrite) — no shell/sed/bash dependency. pkg/scaffold/steps installs the
right code-gen tool and runs it, pkg/scaffold/scaffold.go's Generate
orchestrates preflight -> stage -> rewrite -> generate -> verified go build ./... -> git init -> atomic rename into place.

The connector/processor asymmetry (the top risk called out in review)

Verified against both templates' actual setup.sh + Makefile (cloned at the
pinned SHAs) before writing any code:

  • Connector setup.sh: rename -> (if conn-sdk-cli missing) make install-tools -> make generate, which runs go generate ./... (triggers
    //go:generate conn-sdk-cli specgen in connector.go) then conn-sdk-cli readmegen -w.
  • Processor setup.sh: rename only — it does not call
    install-tools/generate at all. Its Makefile's generate target is a
    single go generate ./..., which triggers a different directive,
    //go:generate paramgen -output=paramgen_proc.go ProcessorConfig, in
    processor.go.

pkg/scaffold/steps does not assume symmetry: InstallTool/Generate
each resolve the tool (conn-sdk-cli vs paramgen) from a
map[template.Kind]string, and the engine deliberately always runs
install+generate for both kinds (unlike upstream's processor setup.sh,
which skips it) — a freshly generated repo should reflect a real, verified
run of its own generator, not just whatever happened to be pre-committed in
the snapshot. --skip-generate is the escape hatch (both templates ship
pre-generated output, so the result still builds either way — verified by
TestGenerate_SkipGenerate_StillBuilds).

Must-fix items from the design review — how each was handled

  • Error --json envelope carries suggestion/configPath/fix: every
    validate() failure builds a *conduiterr.ConduitError with Suggestion
    set (see request.go's newErr helper); cecdysis.CommandWithResult's
    shared envelope carries it through to --json's error.suggestion.
  • Exit codes via pkg/conduit/exitcode: pkg/scaffold/codes.go registers
    scaffold.* conduiterr.Codes classified so toolchain-preflight failure ->
    3 (environment), bad --module/--lang python/destination-exists-without-
    --force -> 2 (validation), a code-gen or go build failure once files are
    staged -> 1 (runtime). Verified end-to-end via the CLI binary and
    pkg/scaffold's tests (exitcode.ExitCode(err) asserted directly).
  • --lang python is an honest gated error, not a half-scaffold: exit 2,
    names the blocker (no Python connector SDK), points at
    docs/design-documents/20260707-python-connector-sdk.md.
  • Preflight before any write, no partial directory on failure: Generate
    runs preflight (reusing pkg/conduit/check's BinaryOnPath/DirWritable
    — not a forked doctor, per the shared check-engine ADR) before creating
    anything; on any hard failure past that point the temp staging directory
    (a sibling of the destination, same filesystem) is os.RemoveAll'd before
    returning, so the requested destination path never exists in a half-written
    state. --force only removes a pre-existing destination immediately before
    the final rename, after the staged tree has already passed the build step.
  • Toolchain preflight via the shared pkg/conduit/check engine: composed
    from check.BinaryOnPath("go", minGoVersion), check.BinaryOnPath("git", ""), check.DirWritable(gopathBin, ""). preflight() deliberately does
    not delegate to check.Report.ExitCode()'s per-check classification
    (that method's DirWritable default is validation-class, which is right for
    doctor's "point config at a writable dir" case but wrong for scaffold's
    "your Go install is broken" case) — every check in this set is an
    environment question, so the whole preflight collapses to one bucket-3
    CodeToolchainUnavailable error. Documented explicitly in preflight.go.

What's covered now vs. deferred

Covered / tested now:

  • connector new --lang go s3 --yes scaffolds ./conduit-connector-s3,
    go build ./... compiles with no edits (TestGenerate_Connector, and
    verified manually via the built CLI binary).
  • Setup.sh -> Go rewrite is golden-file tested, byte-stable
    (template/rewrite_test.go against template/testdata/golden/{connector,processor}).
  • Preflight with go removed from PATH -> exit 3, before any write, no
    partial directory (TestGenerate_MissingToolchain_NoPartialDirectory).
  • --lang python -> gated exit 2 (TestGenerate_UnsupportedLanguage).
  • --json envelope conforms to the CLI output conventions' shape
    (TestResultMarshalsToTheDocumentedEnvelope, plus a manual jq-able run).
  • Processor path produces a building repo using paramgen, not specgen
    (TestGenerate_Processor asserts paramgen_proc.go's generated header).
  • Destination-exists-without---force -> exit 2; --force overwrite works.

Deferred to #2575 (tracked, not
blocking)
— matches the design doc's own risk-tiering, which already
declares the Windows-CI-green criterion "an EXTERNAL dependency... tracked
separately, not this repo's own deliverable":

  1. The <30-min, CI-measured matrix job (ubuntu/macos/windows ×
    connector/processor, clean runner, new -> make test -> make build -> smoke-run in a pipeline).
  2. The nightly job that regenerates the embedded snapshot from upstream
    main + latest SDK and fails loudly on drift (today it's a hand-synced
    pin, recorded in template/embed.go's ConnectorRef/ProcessorRef).
  3. Upstream Windows-matrix PRs to the two template repos (their test.yml is
    ubuntu-latest only today) — coordinated PRs to those repos, not an
    in-repo edit (which would diverge the vendored snapshot).

Not implemented, documented rather than silently missing: the design
doc's TTY guided-prompt flow for a missing name/module. --yes/-y is
accepted (per the CLI output conventions' flag vocabulary for mutating
commands) but the command is always non-interactive today — a missing name
or --module is a deterministic CodeInvalidModule error either way, not a
prompt.

A go:embed wrinkle worth flagging explicitly

Both templates have their own go.mod (and a nested tools/go.mod), which
makes go:embed's directory-embedding refuse them outright ("cannot embed
directory: in different module"), and — independently — would make this
repo's own go build ./.../golangci-lint/make generate try to treat
their loose .go files as this module's source if the go.mod boundary were
removed some other way. Fixed by suffixing every go.mod and *.go path in
the vendored snapshot with .tmpl on disk (see embed.go's tmplSuffix
doc); Extract restores the real names on write-out. Confirmed
validate-generated-files-equivalent stays green: make generate at the
repo root produces zero diff under pkg/scaffold/template (nested go.mod
correctly excludes it from ./...).

Verification

  • go build ./..., go vet ./... clean.
  • go test ./pkg/scaffold/... ./cmd/conduit/... -race -count=1 — all green,
    including full end-to-end scaffold.Generate runs for both connector and
    processor that actually shell out to go install/go generate/go build/git and assert the result builds.
  • golangci-lint run ./... — 0 issues in every file this PR touches (the
    36 pre-existing issues golangci-lint reports repo-wide are all in
    pkg/plugin/processor/builtin/internal/diff/** and
    pkg/foundation/cerrors/conduiterr/conduiterr.go, none touched here).
  • Manually ran the built CLI binary end-to-end: human output (glyphs, step
    timing, "Next steps"), --json, --force, --skip-generate, --no-git,
    missing-toolchain, bad-module, destination-exists, and both connector new
    / processor new — all matched the design doc's UX mockup and exit-code
    table exactly.

Adversarial self-review

  • Connector/processor asymmetry — the top named risk. Handled by
    verifying both templates' real setup.sh+Makefile first (not assumed),
    and encoding the tool choice as an explicit map[template.Kind]string in
    steps/install.go with a doc comment pointing at exactly which
    //go:generate directive and Makefile line justifies each entry.
  • Atomic rename / no partial directory — the other named risk. Verified
    with dedicated tests (TestGenerate_MissingToolchain_NoPartialDirectory,
    TestGenerate_ValidationFailure_NoWrite) that assert the destination
    directory doesn't exist, and that no staging directory is left behind in
    its parent either.
  • Residual, documented, accepted risk: in finalize, if --force's
    os.RemoveAll of a pre-existing destination succeeds but the subsequent
    os.Rename then fails (disk full, a permissions race), the destination is
    left absent rather than reverted — the same risk an ordinary rm -rf old && mv new old has. Not a silent clobber (the error is returned, nothing is
    reported as success); commented explicitly at the call site in
    scaffold.go.
  • gosec (G122/G703) flagged rewrite.go's original
    filepath.WalkDir-callback file I/O as symlink-TOCTOU-prone. Fixed for
    real (not suppressed) using os.Root (Go 1.25) scoped at the staging
    directory for every read/write/remove/rename Rewrite performs.
  • Ran golangci-lint run ./... repo-wide, not just on touched packages, to
    confirm zero new findings anywhere (see Verification).

Files

  • pkg/scaffold/ — engine (scaffold.go, request.go, codes.go,
    preflight.go, sdkversion.go, template/, steps/) + tests.
  • cmd/conduit/internal/scaffoldcmd/ — shared command body.
  • cmd/conduit/root/connectors/new.go, cmd/conduit/root/processors/new.go
    — thin per-kind wrappers, registered in each group's SubCommands().
  • go.modgithub.com/stretchr/testify promoted from indirect to direct
    (this PR's tests import it directly; it was already in the module graph
    transitively, not a new external dependency).

Tier 2 (Features — connectors, processors, CLI; per the design doc's explicit
declaration). No data-path/protocol changes.

🤖 Generated with Claude Code

Co-Authored-By: Claude Opus 4.8 (1M context) noreply@anthropic.com
Claude-Session: https://claude.ai/code/session_01Tapg9dLWXKMZJoL65R24vd

…folding

Roadmap: Phase 1 "Plugin scaffolding" (ROADMAP.md). Design doc:
docs/design-documents/20260707-connector-processor-scaffolding.md (PR #2571,
not yet merged — this PR's exit-code/error-envelope/preflight wiring follows
its "must-fix" review notes regardless of merge order).

Ships pkg/scaffold, the engine behind both commands, plus the two thin CLI
wrappers. Option C from the design doc: orchestrate the canonical upstream
templates (ConduitIO/conduit-connector-template,
ConduitIO/conduit-processor-template), don't reimplement their boilerplate.

- pkg/scaffold/template: a pinned, go:embed'd snapshot of each template
  (SHAs recorded in embed.go), and Rewrite — a Go port of each template's
  setup.sh rename step (module/token substitution, README promotion,
  self-delete), so there is no shell/sed/bash dependency. go.mod and *.go
  files in the snapshot carry a .tmpl suffix on disk (embed.go's tmplSuffix)
  because go:embed refuses to cross a nested module boundary and a stray
  go.mod would otherwise make this repo's own tooling try to compile the
  snapshot; Extract restores the real names on write-out. A golden-file test
  (rewrite_test.go) asserts Extract+Rewrite's output is byte-stable.
- pkg/scaffold/steps: InstallTool/Generate implement the connector/processor
  asymmetry the design review's must-fix called out — the connector
  template's generate step installs and runs conn-sdk-cli (specgen +
  readmegen), the processor template's installs and runs paramgen
  (paramgen_proc.go) — verified against both templates' real setup.sh and
  Makefile before coding, not assumed symmetric. Build verifies `go build
  ./...`; GitInit is best-effort (a buildable scaffold with no git history
  still succeeds the command).
- pkg/scaffold/scaffold.go: Generate validates input, runs the toolchain
  preflight (reusing pkg/conduit/check's BinaryOnPath/DirWritable, not a
  forked doctor, per the shared check-engine ADR), then stages the entire
  write in a temp directory and only os.Rename's it into place once every
  step (including the verified build) has succeeded — no partial directory
  is ever left at the destination on failure.
- Exit codes route through pkg/conduit/exitcode via registered conduiterr
  Codes (codes.go): toolchain preflight failure -> 3 (environment); bad
  --module, --lang python, destination-exists-without---force -> 2
  (validation); a code-gen or `go build` failure once files are staged -> 1
  (runtime). Every validate() error carries a Suggestion, per the CLI output
  conventions' requirement that scaffolding's error envelope isn't a bare
  {code,message}.
- cmd/conduit/internal/scaffoldcmd: the shared command body (flags, args,
  --json envelope via cecdysis.CommandWithResult, human rendering via
  internal/ui) behind both `connector new` and `processor new`, so the two
  commands can't drift on flag names, exit codes, or output shape.

Deferred to #2575 (tracked, not
blocking): the <30-min CI-measured matrix job (AC-10), the nightly
upstream-drift job, and the upstream Windows-matrix PRs to the two template
repos (AC-9) — none of these are required for the command to scaffold a
building repo today, which is verified in-process by every scaffold.Generate
call and by pkg/scaffold's integration tests.

Not implemented in this PR (documented, not silently missing): the design
doc's TTY guided-prompt flow for a missing name/module. --yes/-y is accepted
per the CLI output conventions' flag vocabulary but the command is always
non-interactive today; a missing name or --module is a deterministic
CodeInvalidModule error either way.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Tapg9dLWXKMZJoL65R24vd
@devarismeroxa
devarismeroxa requested a review from a team as a code owner July 8, 2026 03:39
devarismeroxa and others added 4 commits July 7, 2026 20:40
…snapshot

pkg/scaffold/template/{connector,processor} (and its byte-identical
testdata/golden copies) are an exact, unmodified copy of the upstream
templates' own markdown — not hand-authored prose subject to this repo's
markdownlint config. markdownlint-cli2's globs: **/*.md is not Go-aware and
doesn't respect the nested go.mod boundary the way go build/go generate do,
so it was linting vendored content and failing on upstream's own style
choices. Excluded the same way pkg/web/openapi/** already is.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Tapg9dLWXKMZJoL65R24vd
Generate's --force path (finalize's RemoveAll-then-rename) had only manual
CLI verification, not an automated test. Adds a direct assertion that a
pre-existing directory's content is replaced (not merged) and the result
still builds.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Tapg9dLWXKMZJoL65R24vd
TestGenerate_{Connector,Processor,Force_Overwrites,SkipGenerate} go install
external codegen tools + build a full connector — unreliable in the standard
CI test job (module-proxy/rate limits caused 'installing code-gen tool'
failures). They are E2E coverage for the dedicated scaffold-e2e job (#2575).
Skip in CI (unless CONDUIT_SCAFFOLD_E2E is set); still run locally.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@devarismeroxa
devarismeroxa merged commit 9720ce2 into main Jul 8, 2026
6 checks passed
@devarismeroxa
devarismeroxa deleted the feat/cli-connector-processor-new branch July 8, 2026 04:19
devarismeroxa added a commit that referenced this pull request Jul 8, 2026
… fix CI flake (#2578)

Fixes the CI flake noted on #2534 that failed the `test` job on PRs #2576
and #2577 (TestServiceLifecycle_PipelineError, TestServiceLifecycle_PipelineStop).

The immediate trigger was a raw time.Sleep(100ms) used to let 10 records
"finish flowing" before stopping the pipeline in TestServiceLifecycle_PipelineStop
and its sibling TestServiceLifecycle_PipelineSuccess. Under CI load 100ms isn't
always enough, so Stop/StopAll can race the data path, under-deliver records,
and fail the source/destination mocks' record-count assertions in t.Cleanup.
Both are replaced with waitForRecordsAcked, which polls the source connector's
exported, lock-guarded State field (set by Source.Ack) until every record has
actually been acked, instead of guessing a duration.

Digging into *why* the flake also showed a bare status mismatch
("SystemStopped != Running") turned up four distinct, real concurrency bugs in
pkg/lifecycle-poc, found and fixed via `-race -shuffle=on` repetition:

1. service.go WaitPipeline: a lookup-after-delete race could return a false
   nil instead of the pipeline's terminal error/status if the cleanup
   goroutine removed the pipeline from runningPipelines between the lookup
   and return. This is the identical bug already diagnosed and fixed in the
   sibling pkg/lifecycle package (see
   docs/design-documents/20260706-forceful-stop-test-determinism.md); ported
   the same terminalErrors-map fix here.

2. service.go runPipeline: the worker goroutine's tomb.Kill(err) for a fatal
   error only happened in tomb.v2's own post-return bookkeeping, which raced
   the cleanup goroutine waking from workersWg.Wait() and reading
   rp.t.Err() — losing the race made a fatal stop get reported as a clean
   graceful stop. Fixed by killing the tomb synchronously before returning.

3. service.go runPipeline (the actual root cause of the CI-observed
   "SystemStopped != Running"): the function's own initial
   UpdateStatus(StatusRunning) call and the cleanup goroutine's terminal
   UpdateStatus call are not safe to run concurrently on the same
   *pipeline.Instance (SetStatus is lock-guarded, but the Error field write
   and the store's JSON-encode of the whole struct for persistence are not).
   With mocks providing zero I/O latency, the cleanup goroutine could call
   its own UpdateStatus while the initial one was still mid-flight and win
   the race, clobbering a correct terminal status back to "running".
   Confirmed by a genuine `-race` data race report. Fixed by gating the
   cleanup goroutine's UpdateStatus on a channel closed only after the
   initial one fully completes (kept the two tomb.Go registrations adjacent
   to avoid a related tomb.Go-after-death panic that a naive reordering
   introduced).

4. funnel/worker.go Worker.Stop: tore down the source (nils the plugin)
   before setting w.stop, so a concurrent Read() racing the stop could
   observe plugin.ErrPluginNotRunning before doTask's graceful-stop check
   (w.stop.Load()) saw stop=true, misreporting a graceful stop as a real
   failure. Fixed by setting w.stop before tearing down.

Also added the missing `defer persister.Wait()` to PipelineError/PipelineStop
(PipelineSuccess already had it) — without it the persister's background
flush goroutine can log via log.Test(t) after the test function returns,
which panics and can corrupt whatever test happens to be running concurrently
in the same process.

Verification (pkg/lifecycle-poc/):
- go test -run 'TestServiceLifecycle' -race -count=500: 0 failures
- yes >/dev/null (x3) & go test -run 'TestServiceLifecycle' -count=300: 0 failures
- go test ./... -race -shuffle=on -count=50: 0 failures
- go build ./...: clean
- golangci-lint run ./pkg/lifecycle-poc/...: 0 issues

Self-review: re-read the diff for error paths, concurrency, and resource
cleanup. Confirmed close(startupDone) is unconditional (both the success and
UpdateStatus-error paths reach it), so the cleanup goroutine can never hang.
Confirmed both tomb.Go registrations stay adjacent so tomb.alive can't reach
0 before the cleanup goroutine exists. Not fixed here (flagged for a
follow-up, out of scope for a test-flake PR): pkg/lifecycle-poc's force-stop
path kills the tomb with a bare pipeline.ErrForceStop instead of
cerrors.FatalError(...) like pkg/lifecycle does, and the "recovery not
implemented" branch in the cleanup switch is a complete no-op that leaves a
dead pipeline's status stuck at "running" forever instead of reporting
Degraded — both are real gaps but change lifecycle semantics beyond what a
flaky-test fix should carry.

Roadmap: Phase 0 stabilization — CI reliability for the arch-v2 (funnel)
pipeline lifecycle package ahead of it replacing pkg/lifecycle.


Claude-Session: https://claude.ai/code/session_01Tapg9dLWXKMZJoL65R24vd

Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant