Skip to content

feat(embed): libconduit B1 Go embedding API (New/Engine/Handle/Import)#2667

Merged
devarismeroxa merged 3 commits into
mainfrom
feat/embed-libconduit-b1
Jul 23, 2026
Merged

feat(embed): libconduit B1 Go embedding API (New/Engine/Handle/Import)#2667
devarismeroxa merged 3 commits into
mainfrom
feat/embed-libconduit-b1

Conversation

@devarismeroxa

Copy link
Copy Markdown
Contributor

Summary

Implements B1 of the libconduit / embedded v1 workstream (v0.19 Workstream 2, design doc #2651, plan workstreams/embed-libconduit.md): a frozen, semver-committed root package github.com/conduitio/conduit giving a Go application an embeddable Conduit engine — conduit.New(ctx, Options) -> Engine.Run(ctx) -> Handle.Stop(ctx) — with host-injected *slog.Logger and prometheus.Registerer, so nothing writes to a global logger, the default Prometheus registry, or os.Stdout, and nothing calls os.Exit.

B2 (pipelines-in-code builder) is explicitly out of scope — this PR ships Engine.Import(ctx, PipelineConfig) against hand-built PipelineConfig literals only; the NewPipeline(id).With...().Build() builder is a later PR.

What shipped

pkg/conduit seams (additive, inert on the CLI path — AC-5):

  • RuntimeOption + WithLogger, WithMetricsRegisterer, WithDialContext on NewRuntime(cfg, ...opts).
  • OpenStore now takes a context.Context (bounds the Postgres/SQLite dial instead of context.Background()); its one other caller (conduit doctor's store-reachable check) updated.
  • Per-Runtime configureEmbeddedMetrics (fresh registry + grpc stats handler per embed Runtime, registered only into the host's registerer) alongside the unchanged CLI-path configureMetrics() (package sync.Once + promclient.MustRegister).
  • TestNewRuntime_CLIDefaultsUnchanged proves the seams are inert with zero options passed.

Root package (conduit.go, doc.go, log_adapter.go):

  • Options (Logger, MetricsRegisterer, DB, PipelinesDir, API), Engine, Handle, New.
  • Engine.Run/Handle.Stop per AC-4's exact two-resolution-path shape (Ready vs. a failed Run, never blocking).
  • Handle.Stop reuses Runtime.Run's existing ctx-cancellation-driven tomb drain unchanged — Invariant 7, no new drain mechanics.
  • PipelineConfig — a type alias (not a copy) of provisioning/config.Pipeline; doc comment added to parser.go extending the deprecation policy by name (grep-checked: constrained by.*PipelineConfig).
  • Engine.Import enriches + validates the config (provisioning/config.Enrich/Validate) before delegating to provisioning.Service.Import — see self-review finding below.
  • Engine.StartPipeline/StopPipeline thin delegates to Orchestrator.Pipelines.
  • Internal zerolog→slog adapter (log_adapter.go) so Conduit's existing zerolog-based logging routes through a host *slog.Logger without teaching pkg/foundation/log about slog.

Trimmed per review must-fix: no new conduiterr codes. Run-called-twice, Stop-on-nil-Handle, and Stop-deadline-exceeded all return the existing conduiterr.CodeInvalidArgument. AC-8's "under 15 minutes" is split: TestExampleHelloPipeline_WithinBudget asserts a 60s CI wall-clock ceiling on the example's own execution; the human-copy-paste-timing claim is left as a doc/UX statement, not a test.

Tests: TestNew_ConstructsEngine, TestNew_Nil{Logger,Registerer}_*, TestRun_FailsBeforeReady, TestRun_AlreadyCanceledContext (Engine- and Runtime-level), TestRun_CalledTwice_ReturnsCodedError, TestStop_{NilHandle,ConcurrentIdempotent,DeadlineExceeded}, TestNew_StoreAlreadyOpen_ReturnsCodedError, TestNew_MetricNameCollision_ReturnsCodedError, TestPipelineConfig_IsAliasOfProvisioningConfig, TestImport_RoundTripsThroughRunningEngine, TestTwoEngines_LoggerIsolated (must pass), TestTwoEngines_MetricsCrossTalk_KnownLimitation (documents, doesn't fix), TestObservabilityIsolation_Subprocess (subprocess re-exec proving zero bytes to real stdout/stderr — AC-2c), Example_helloPipeline + its budget test.

⚠️ Tier-1 commit requiring DeVaris sign-off

Commit d5922defix(conduit): guard Runtime.Run against an already-canceled context is isolated from the Tier-2 embed work per the design doc's escalation rule (anything touching Runtime.Run's tomb/cleanup mechanics is Tier 1, regardless of diff size).

This is not just Tier-1-caution — it's a real bug, found empirically while writing TestRun_AlreadyCanceledContext: without the one-line ctx.Err() guard at Run's entry, calling Runtime.Run with an already-canceled context panics (tomb.Go called after all goroutines terminated) instead of returning an error. tomb.WithContext's own cancellation-watcher goroutine kills the tomb before initServices's later t.Go calls (e.g. serveGRPC) run, so those calls hit an already-dead tomb.

conduit run never triggers this (a live process's context is only canceled by a signal arriving after Run is already executing), so the CLI path is unaffected — confirmed by removing the guard locally and re-running the full pkg/conduit suite plus the new CLI-parity test, all green except the two tests this fix targets. The guard is a pure early return before the tomb or registerCleanup is constructed; it does not touch drain/cleanup logic itself.

Requesting explicit DeVaris review of commit d5922de before merge, per the standing Tier-1 gate.

Failure-mode analysis (Tier-1 commit)

  • What could this break: nothing on the CLI path — the guard is unreachable there (conduit run never calls Run with a pre-canceled context). On the embed path, it changes a panic into a clean error return, strictly safer.
  • Which metric/alert would show a regression: N/A — no metric surfaces a panic-vs-error distinction today; a regression would show as a crashed process / non-zero-without-log exit in whatever harness runs conduit run (unaffected) or as an embedder's process crashing on Engine.Run (this fix's whole point).
  • Rollback: revert commit d5922de alone; it has no dependents (the Tier-2 commit that touches Engine.Run was written and tested with this guard in place, so reverting would reintroduce the empirically-confirmed panic for the embed path specifically, not affect the CLI).

Adversarial self-review

  • Error paths: Engine.Import originally delegated straight to provisioning.Service.Import per the design doc's citation — but a hand-built PipelineConfig{} has a nil DLQ.WindowSize/WindowNackThreshold (zero-value *int), and provisioning.Service's createPipelineAction.Do dereferences those pointers directly, assuming the file-based path's config.Enrich already ran. Caught this via TestImport_RoundTripsThroughRunningEngine, which panicked (nil-pointer deref) before the fix. Import now calls config.Enrich+config.Validate first, mirroring provisionPipeline's own convention exactly.
  • Concurrency: Handle.stopOnce + buffered done/select ensures Stop is safe under concurrent callers (TestStop_ConcurrentIdempotent, race-clean). Engine.started uses atomic.Bool.CompareAndSwap so two goroutines racing Run() can't both proceed.
  • Resource cleanup: Engine.Run's failure branch (Ready never closes) calls cancel() before returning to release the derived runCtx, even though the tomb has already exited by construction.
  • Known, accepted limitation (not fixed here, per design doc's Non-goals): a failed configureEmbeddedMetrics call (e.g., the metric-name-collision case) still leaks a foundation/metrics/prometheus.Registry into pkg/foundation/metrics's process-global bookkeeping, because the collision can only be detected after metrics.Register(reg) has already populated reg's descriptors — there's no Unregister. Harmless (never scraped, since the host's own Register call is what failed) but real; flagging so it doesn't reappear as a surprise later.
  • Invariant 7: verified Handle.Stop calls only cancel() (no reimplementation) and reads Runtime.Run's real return value; registerCleanup/V1/V2 are untouched by either commit.
  • /metrics HTTP endpoint gap (documented, not fixed): if Options.API.Enabled is true, the HTTP /metrics route still serves promhttp.Handler() (the default gatherer), not the host's injected registerer — out of scope for this PR's ACs (none of AC-1..8 test the HTTP metrics route), noting for a follow-up.

Open questions for DeVaris

  1. Confirm the Tier-1 commit's review is what you want before merge (per the standing gate) — I have not merged this PR.
  2. Confirm the /metrics HTTP-route gap above is an acceptable known limitation for v1, or should be tracked as a follow-up issue.

Gate results

  • make generate && git diff --exit-code: clean, no drift.
  • golangci-lint run ./... --new-from-rev=main: 0 issues.
  • go build ./...: clean.
  • go test -race ./...: full repo green (including this PR's new packages).

Roadmap: v0.19 Workstream 2 (libconduit embed v1, B1). Risk tier: Tier 2 overall, Tier 1 for commit d5922de specifically.

🤖 Generated with Claude Code

https://claude.ai/code/session_015GQFzakPShAYj8CcwajYDD

devarismeroxa and others added 2 commits July 23, 2026 13:40
…port)

Adds the root-level github.com/conduitio/conduit package (v0.19 Workstream 2,
design doc #2651): New(ctx, Options) -> Engine.Run(ctx) -> Handle.Stop(ctx),
with host-injected *slog.Logger and prometheus.Registerer so an embedder
never touches os.Exit, a global logger, or the default Prometheus registry.

Seams added to pkg/conduit (additive RuntimeOption, inert on the CLI path):
WithLogger bypasses log.GetWriter's os.Stdout default and skips the
zerolog.DefaultContextLogger global assignment; WithMetricsRegisterer builds
a per-Runtime metrics registry/grpc stats handler instead of the CLI's
package-level configureMetrics(); WithDialContext lets OpenStore's
Postgres/SQLite dial honor a caller context instead of context.Background().
OpenStore's signature changes to take a context.Context (updates its one
other caller, `conduit doctor`'s store-reachable check).

Engine.Run/Handle.Stop resolve via exactly one of two paths (Ready or a
failed Run), never blocking indefinitely; Handle.Stop reuses Runtime.Run's
existing ctx-cancellation-driven drain unchanged (Invariant 7) and is
idempotent under concurrent callers. Run-called-twice and Stop-on-nil-Handle
return the existing conduiterr.CodeInvalidArgument rather than new
speculative codes. Engine.Import enriches/validates a PipelineConfig (a type
alias of provisioning/config.Pipeline) before delegating to
provisioning.Service.Import, matching the file-based provisioning path's own
convention.

Does not include Task 6 (the Runtime.Run ctx.Err() entry guard) — that lands
in a separate, clearly-marked commit per the design doc's Tier-1 escalation
rule for anything touching Runtime.Run's tomb/cleanup mechanics.

Roadmap: v0.19 Workstream 2 (libconduit embed v1, B1). Risk tier: Tier 2.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_015GQFzakPShAYj8CcwajYDD
TIER 1 — flagged for DeVaris sign-off per the embed workstream plan's
escalation rule: this touches Runtime.Run's entry, where invariant 7's
tomb/cleanup mechanics live, even though the change itself is a one-line,
non-behavior-changing-for-the-CLI guard clause.

Without this guard, Runtime.Run panics ("tomb.Go called after all goroutines
terminated") when handed a context that is already canceled before Run does
any work: tomb.WithContext's own cancellation-watcher goroutine kills the
tomb (and lets it finish) before initServices's later t.Go calls (e.g.
serveGRPC) run, so those calls hit an already-dead tomb. This was found
empirically while adding Engine.Run's AC-4 test coverage in the prior commit
(TestRun_AlreadyCanceledContext, at both the Runtime and Engine level) —
removing the guard and re-running that test reproduces the panic.

`conduit run` never calls Run with an already-canceled context (a live
process's context is only canceled by a signal that arrives after Run is
already executing), so this is inert on the CLI path. It matters for the
embed calling convention (Engine.Run), where a host can legitimately pass an
already-canceled or already-expired ctx (e.g. one derived from an expired
request deadline).

Does not touch registerCleanup or the tomb's drain mechanics — it is a pure
early return before either is constructed.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_015GQFzakPShAYj8CcwajYDD
Runtime.Run's already-done-context guard (added for the embed API) returned
before registerCleanup ever scheduled r.DB.Close(), so an Engine whose Run
call hit that guard - or whose Run was simply never called - permanently
leaked the database New() opened eagerly (Badger file lock, PG pool, SQLite
handle), with Engine.started already latched true so Run couldn't be retried.

- Add Runtime.CloseDB(), an idempotent (sync.Once) close point, and route
  registerCleanupV1/V2's r.DB.Close() through it instead of calling it
  directly, so the normal Run->Stop drain and the new release path never
  race or double-close the same handle.
- Add Engine.Close(ctx), the embed API's resource-release path: safe to call
  after New with no Run, after Run hits the guard, or after a successful
  Run->Stop cycle (double-close-safe by construction, not by luck). Document
  the full New/Run/Stop/Close state matrix on Engine, including that started
  flips on the Run *call*, not on success.
- The guard now returns a conduiterr.CodeInvalidArgument-coded error instead
  of a bare context.Canceled/DeadlineExceeded, matching New/Run's documented
  promise of a *conduiterr.ConduitError where classifiable, and reusing the
  same code this package already uses for lifecycle-contract violations
  (double Run, nil Stop, Stop timeout) rather than minting a new one.
- Wire TestNew_StoreAlreadyOpen_ReturnsCodedError to Close e1 instead of
  discarding it, and add TestClose_NoRun_ReleasesDB,
  TestClose_AfterRunHitsAlreadyDoneContextGuard_ReleasesDB (both proven via a
  real Badger file lock, not just a nil-error check),
  TestClose_DoubleCloseIsSafe, and TestClose_AfterSuccessfulStop_IsSafe.
- File and cross-reference two tracked follow-ups accepted as v1 limitations:
  configureEmbeddedMetrics leaking a registry into pkg/foundation/metrics on
  a failed Register (#2669), and the /metrics HTTP route always serving
  promclient.DefaultGatherer instead of a host-supplied MetricsRegisterer
  (#2670).
- Fix doc.go overstating that Connector/Processor/DLQ are annotated "the
  same way" as Pipeline - it's one shared comment block above Pipeline
  covering all four by name, not per-type docs.

Tier 1 (embed lifecycle + DB resource release on a frozen public API,
docs/design-documents/20260722-embed-libconduit-v1.md). Self-review: reused
CloseDB's sync.Once for both the tomb-driven cleanup path and Engine.Close so
neither needs to know whether the other ran first - the alternative (an
Engine-level guard alone) would not have prevented a double r.DB.Close() call
after a successful Stop. Verified with -race that TestClose_* pass alongside
the existing Stop/Run suite. Still needs DeVaris sign-off before merge - do
not merge on this review alone.

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

Copy link
Copy Markdown
Contributor Author

Review must-fixes applied (Tier 1 — still needs DeVaris sign-off before merge)

Addressed the fresh-context review findings on the embed lifecycle/DB-leak issue.

1. Added Engine.Close(ctx) error — the resource-release path for the database New opens
eagerly. Covers all three states the review called out:

  • NewClose (Run never called)
  • NewRun (hits the already-done-context guard) → Close
  • NewRunStopClose (safe no-op; the drain already closed it)

Idempotency is enforced at the Runtime level: added Runtime.CloseDB() (a sync.Once around
r.DB.Close()), and routed both registerCleanupV1/V2's cleanup goroutines and
Engine.Close through it, so neither the normal drain path nor the new release path needs to
know whether the other already ran — no double-close race either way.

TestNew_StoreAlreadyOpen_ReturnsCodedError now defers e1.Close(ctx) instead of discarding
e1. Added TestClose_NoRun_ReleasesDB, TestClose_AfterRunHitsAlreadyDoneContextGuard_ReleasesDB
(both proven against a real Badger file lock — a second New at the same path only succeeds if
the first Engine's DB was actually released), TestClose_DoubleCloseIsSafe, and
TestClose_AfterSuccessfulStop_IsSafe.

2. The already-done-context guard now returns a coded errorconduiterr.CodeInvalidArgument
wrapping the original ctx.Err(), instead of a bare context.Canceled/DeadlineExceeded. Reuses
the existing code rather than minting a new one, consistent with how this package already codes
the double-Run and Stop-timeout cases.

3. Documented the lifecycle contract on Engine's doc (and referenced from New, doc.go):
started flips on the Run call, not on success, so a failed Run retires the Engine for
running — and Close is the resource-release path regardless of whether Run was ever called
or succeeded.

4. Filed and cross-referenced two tracked follow-ups (accepted as v1 limitations, not
blocking this PR):

5. Fixed the doc.go overstatement that Connector/Processor/DLQ are annotated "the same
way" as Pipeline — it's one shared comment block above Pipeline in parser.go covering all
four by name, not per-type docs. Clarified.

Gates run locally

  • make generate && git diff --exit-code — clean, no generated-file drift.
  • golangci-lint run ./ ./pkg/conduit/... — 0 issues.
  • go build ./... — clean.
  • go test -race -count=1 ./ ./pkg/conduit/... — all green, including the new TestClose_*
    tests and the existing Run/Stop suite.
  • gofmt -l . — only a pre-existing unrelated golden-testdata file, not touched by this diff.

Self-review note

The alternative I considered — an Engine-level sync.Once around Close alone — would not
have prevented a double r.DB.Close() call after a successful Stop (the tomb-driven cleanup
path and Close would each think they were the only closer). Pushing the sync.Once down into
Runtime.CloseDB() and having both paths call it is what actually makes "safe after a
successful Stop" true, not just "safe if you only ever call Close once."

Per CLAUDE.md: this is Tier 1 (embed lifecycle + DB resource release on a frozen public API). Not
merging — needs DeVaris review with fresh context in a separate session.

Co-Authored-By: Claude Opus 4.8 (1M context) noreply@anthropic.com

@devarismeroxa
devarismeroxa merged commit e6a8b9f into main Jul 23, 2026
5 checks passed
@devarismeroxa
devarismeroxa deleted the feat/embed-libconduit-b1 branch July 23, 2026 18:45
devarismeroxa added a commit that referenced this pull request Jul 23, 2026
Adds the fluent PipelineBuilder (NewPipeline/NewSourceConnector/
NewDestinationConnector/NewProcessor/NewDLQ) to the root github.com/conduitio/conduit
package, producing the same config.Pipeline the YAML provisioner parses so
Engine.Import accepts a hand-built pipeline exactly like a parsed one
(v0.19 Workstream 2 / roadmap "Embedded v1", following B1 in #2667).

Build() reuses provisioning/config.Enrich+Validate the same way `conduit
pipelines validate` does, so a Build-time error catches the same class of
mistake the CLI would (missing id/plugin/type, duplicate connector/processor
ids, ...), but returns the raw unenriched struct so a hand-built
PipelineConfig stays indistinguishable from a parsed one.

Round-trip coverage: a 432-case deterministic combinatorial matrix
(status/name/description/DLQ variants including the *int nil-vs-explicit-
zero distinction/connector and processor counts) asserts build(x) ==
parse(marshal(fromConfig(build(x)))), plus 3 fixture cases reproducing the
existing pkg/provisioning/config/yaml/v2/testdata/pipelines1-success.yml
pipelines field-for-field via the builder.

The round-trip test caught two pre-existing, previously-uncovered bugs in
pkg/provisioning/config/yaml/v2 (FromConfig had no production caller before
this workstream):
- fromProcessorsConfig silently dropped Processor.Condition
- Connector/Processor/DLQ.ToConfig didn't normalize an empty Settings map
  to nil the way the sibling slice fields already do, so a nil map
  round-tripping through FromConfig+yaml.Marshal came back non-nil

Both are fixed with regression tests that fail without the fix and pass
with it.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_015GQFzakPShAYj8CcwajYDD
devarismeroxa added a commit that referenced this pull request Jul 24, 2026
…ir (B1 DX hardening)

A DX audit of the merged libconduit B1 API (#2667) found two frozen-Options-
contract bugs worth a fast-follow while adoption is still zero:

1. Leaving Options.PipelinesDir empty fell through to pkg/conduit's CLI-
   oriented cwd/pipelines default. Since that directory usually doesn't exist
   for a pure-embed caller, provisioning then failed as a swallowed ERROR log
   while Run still returned a healthy Handle — contradicting New's doc promise
   that every failure is returned as an error. Fixed by adding
   pkgconduit.Config.Pipelines.Disabled (no CLI flag, zero value false, so
   `conduit run` is unaffected) and having the embed layer's New set it
   whenever PipelinesDir is empty, short-circuiting file provisioning
   entirely instead of scanning cwd. A genuine provisioning failure with a
   *configured* PipelinesDir now surfaces as a returned error from Run via a
   new, purely-additive Runtime.ProvisioningErr field the CLI never reads.

2. New eagerly opened the database, so a constructed-but-never-Run Engine
   leaked a Badger file lock / Postgres pool, and Close was mandatory even
   without Run. Fixed by deferring pkgconduit.NewRuntime construction to the
   first Run or Import call (ensureRuntime); New now only validates Options.
   Close is a safe no-op if nothing was opened. TestNew_StoreAlreadyOpen_
   ReturnsCodedError is updated: the contention now has to be forced via Run,
   since New no longer opens anything. A runtime.SetFinalizer safety net logs
   a leak warning (through the injected logger, else slog.Default()) if an
   Engine that did open resources is GC'd without Close.

Updates doc.go and example_test.go for the lazy-open timing change, and adds
regression tests for both fixes (empty-PipelinesDir never scans cwd, a
configured-but-malformed PipelinesDir surfaces as an error, and the metrics/
store-collision tests moved from New to Run since that's where opening now
happens).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_015GQFzakPShAYj8CcwajYDD
devarismeroxa added a commit that referenced this pull request Jul 24, 2026
…ir (B1 DX hardening) (#2676)

* fix(embed): lazy DB open + skip file provisioning on empty PipelinesDir (B1 DX hardening)

A DX audit of the merged libconduit B1 API (#2667) found two frozen-Options-
contract bugs worth a fast-follow while adoption is still zero:

1. Leaving Options.PipelinesDir empty fell through to pkg/conduit's CLI-
   oriented cwd/pipelines default. Since that directory usually doesn't exist
   for a pure-embed caller, provisioning then failed as a swallowed ERROR log
   while Run still returned a healthy Handle — contradicting New's doc promise
   that every failure is returned as an error. Fixed by adding
   pkgconduit.Config.Pipelines.Disabled (no CLI flag, zero value false, so
   `conduit run` is unaffected) and having the embed layer's New set it
   whenever PipelinesDir is empty, short-circuiting file provisioning
   entirely instead of scanning cwd. A genuine provisioning failure with a
   *configured* PipelinesDir now surfaces as a returned error from Run via a
   new, purely-additive Runtime.ProvisioningErr field the CLI never reads.

2. New eagerly opened the database, so a constructed-but-never-Run Engine
   leaked a Badger file lock / Postgres pool, and Close was mandatory even
   without Run. Fixed by deferring pkgconduit.NewRuntime construction to the
   first Run or Import call (ensureRuntime); New now only validates Options.
   Close is a safe no-op if nothing was opened. TestNew_StoreAlreadyOpen_
   ReturnsCodedError is updated: the contention now has to be forced via Run,
   since New no longer opens anything. A runtime.SetFinalizer safety net logs
   a leak warning (through the injected logger, else slog.Default()) if an
   Engine that did open resources is GC'd without Close.

Updates doc.go and example_test.go for the lazy-open timing change, and adds
regression tests for both fixes (empty-PipelinesDir never scans cwd, a
configured-but-malformed PipelinesDir surfaces as an error, and the metrics/
store-collision tests moved from New to Run since that's where opening now
happens).

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

* fix(embed): ensureRuntime rejects Run/Import after Close

Review found: ensureRuntime never checked the closed flag, so calling
Run or Import on an Engine after Close (with no prior Run/Import)
silently opened a fresh runtime and database the caller believed was
already released. ensureRuntime now checks closed first and returns
the existing conduiterr.CodeInvalidArgument (same code used for double
Run / nil-Handle Stop) instead of minting a new one.

Also documents, as known v1 behavior (no code change), that only the
first caller's ctx bounds the lazy DB dial: a concurrent Import with a
short-lived ctx can cause a concurrent Run's dial to observe that
cancellation.

Regression test: New -> Close -> Run/Import returns the coded error
and opens no database (verified via a second Engine at the same
BadgerDB path opening without contention). Confirmed to fail without
the fix and pass with it.

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

---------

Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
devarismeroxa added a commit that referenced this pull request Jul 24, 2026
* feat(embed): B2 pipelines-in-code builder

Adds the fluent PipelineBuilder (NewPipeline/NewSourceConnector/
NewDestinationConnector/NewProcessor/NewDLQ) to the root github.com/conduitio/conduit
package, producing the same config.Pipeline the YAML provisioner parses so
Engine.Import accepts a hand-built pipeline exactly like a parsed one
(v0.19 Workstream 2 / roadmap "Embedded v1", following B1 in #2667).

Build() reuses provisioning/config.Enrich+Validate the same way `conduit
pipelines validate` does, so a Build-time error catches the same class of
mistake the CLI would (missing id/plugin/type, duplicate connector/processor
ids, ...), but returns the raw unenriched struct so a hand-built
PipelineConfig stays indistinguishable from a parsed one.

Round-trip coverage: a 432-case deterministic combinatorial matrix
(status/name/description/DLQ variants including the *int nil-vs-explicit-
zero distinction/connector and processor counts) asserts build(x) ==
parse(marshal(fromConfig(build(x)))), plus 3 fixture cases reproducing the
existing pkg/provisioning/config/yaml/v2/testdata/pipelines1-success.yml
pipelines field-for-field via the builder.

The round-trip test caught two pre-existing, previously-uncovered bugs in
pkg/provisioning/config/yaml/v2 (FromConfig had no production caller before
this workstream):
- fromProcessorsConfig silently dropped Processor.Condition
- Connector/Processor/DLQ.ToConfig didn't normalize an empty Settings map
  to nil the way the sibling slice fields already do, so a nil map
  round-tripping through FromConfig+yaml.Marshal came back non-nil

Both are fixed with regression tests that fail without the fix and pass
with it.

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

* feat(embed): B2 DX fixes — ImportPipeline, nil-arg errors, snapshot docs

Addresses three must-fix findings from the libconduit DX audit on #2673:

1. Engine.ImportPipeline(ctx, *PipelineBuilder) error — Build-then-Import in
   one call, the key DX win: gets the embed quickstart to a genuine <=15
   lines and removes the second provisioning/config import for the common
   case. doc.go's package example and example_test.go's Example_helloPipeline
   now use it instead of a hand-built PipelineConfig{} literal.

2. WithConnector(nil)/WithProcessor(nil)/WithDLQ(nil) no longer panic. Each
   records a conduiterr.CodeInvalidArgument error (no new code minted,
   matching Run/Stop misuse's existing precedent) that surfaces — joined
   with any other accumulated or validation errors — from the next Build
   call. ConnectorBuilder.WithProcessor(nil) propagates the same way once
   the connector is attached to a PipelineBuilder. Build on a nil
   *PipelineBuilder is likewise a coded error, not a nil-pointer panic.

3. PipelineBuilder's godoc now states the snapshot-at-attach contract
   explicitly, with a worked example showing a *ConnectorBuilder reused
   across two pipelines with a mutation in between — resolving the audit's
   "write-once/single-owner footgun" concern without changing the
   (already-correct) snapshot-at-attach behavior itself.

New tests: nil-connector/processor/DLQ at pipeline scope, nested nil
processor at connector scope, nil *PipelineBuilder.Build(), multi-nil-arg
error joining, cross-pipeline builder-reuse semantics, and an
ImportPipeline round trip through a real running Engine (success path and
Build-error propagation path).

Note for reviewers: conduit.go is also touched by a parallel B1-hardening
branch (lazy DB open) — expect a merge conflict there; sequencing is by the
maintainer at merge time, not this PR.

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

* fix(embed): deep-copy Settings/Processors on build() to close snapshot-isolation gap

Review found that PipelineBuilder's documented snapshot-at-attach guarantee
("does not keep a live reference") held only for scalar fields. Connector/
Processor/DLQBuilder.build() returned their config struct by value, which
copies a Settings map or Processors slice header, not its contents — so
WithSetting/WithSettings/WithProcessor on a builder after it was already
attached (and built) silently mutated the already-built PipelineConfig too.

Fix: build() now deep-copies Settings maps and Processors slices (recursing
into each Processor's own Settings) so an attached snapshot is fully
isolated from any later mutation of the source builder, matching what the
doc already promised.

Regression tests reproduce review's exact scenarios (WithSetting/WithSettings
after attach, and a nested Processor's Settings mutated after two-level
attach) for all three Settings-bearing builders; each fails without the
deep-copy fix and passes with it (verified via git stash).

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

---------

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