feat: conduit pipelines repair + MCP repair with fix-synthesis (§3/§2)#2614
Merged
Conversation
…thesis (§3/§2) Implements docs/design-documents/20260712-repair-command.md: the honest finding there is that conduiterr.Fix already round-trips end-to-end but NO error site populates it, so this PR's real work is fix synthesis, not command wiring. Producers (v1 starter set, §6, items #1-#4): - deprecated processor `type` -> `plugin` rename (pkg/provisioning/config/yaml linter, new config.field_renamed code carrying the compound-rename Fix) - unambiguous invalid `/status` normalization (pkg/provisioning/config/validate.go) - negative processor `/workers` clamp to 1 (same) - over-long `/description` truncation (same) Shared engine (cmd/conduit/internal/repair): Collect/CollectContent run the existing validate/lint engine, classify every fix default-deny (safe/restart/ data_path) against the ack/position/checkpoint-adjacent invariants, and Apply edits a comment-preserving *yaml.Node tree (github.com/conduitio/yaml/v3, already a dependency) rather than a marshal-from-struct round trip. repair never opens the provisioning store or a running pipeline — see the new ADR (docs/architecture-decision-records/20260713-repair-edits-file-not-store.md). CLI: `conduit pipelines repair <file> [--apply] [--plan-hash] [--fix]... [--escalate] [--json]`, diff-first like deploy/apply, atomic (temp+rename) file write. MCP: `repair` (read, always registered) and `repair_apply` (write, gated by --allow-mutations), registered via the mcp.Catalog()/tool() pattern. There is deliberately no `escalate` field on repair_apply's schema — a data-path fix is always refused for auto-apply from MCP; only the CLI's --escalate is a valid override, keeping the Tier-1 human gate real. New error codes: config.field_renamed, repair.plan_stale, repair.fix_no_longer_applies, repair.data_path_fix_refused, repair.ambiguous_fix, repair.no_fixes_available. llms.txt/llms-full.txt regenerated (go generate) to include them and the two new MCP tools. Roadmap: docs/design-documents/20260704-phase-1-execution-plan.md §2, §3. Risk tier: 2 (feature; repair never reaches the data path by construction). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01B3cLokwNJYLLBpdyt3cgGr
… classifier Resolves the independent review's blocking findings on #2614: - B1 (BLOCKER, data loss): repair --apply silently dropped trailing YAML documents. The guard counted PIPELINES, not DOCUMENTS; parseDoc decoded only the first document and marshalDoc re-emitted only that, so a multi-document file whose first document holds one pipeline and whose trailing documents contribute zero pipelines slipped past every guard, and --apply rewrote the file with everything after the first document deleted. parseDoc now rejects multi-document files at the document level (a second Decode that isn't io.EOF). Regression test added (TestCollect_RejectsMultiDocument) — the suite previously had none. - M1 (MAJOR, the Tier-1 gate): the data-path classifier's safe/restart allowlist matched on the bare final segment name, so a future field ending in status/description/type/workers in a non-producer location would be auto-classified safe/restart. Anchored the allowlist to the full path SHAPES the v1 producers emit (extracted to classifyAllowlisted, keeping classify under the cyclomatic limit) so an unrecognized shape falls through to default-deny. Over-match negative cases added to the classifier table. - n1 (honesty): corrected the ADR — repair does import pkg/provisioning/config (a pure config/validation package for CodeFieldRenamed); it's the store/service packages it does not import. Verified: go build ./... clean; go test -race green on the repair package; golangci-lint 0 issues; ADR markdownlint clean; no llms.txt drift. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01B3cLokwNJYLLBpdyt3cgGr
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Implements
conduit pipelines repair+ MCPrepair/repair_applyperdocs/design-documents/20260712-repair-command.md.Advances execution-plan §3 (CLI verb parity) and §2 (agent-native "human isn't
a second-class citizen to the agent").
Risk tier: 2 (feature — connectors/processors/CLI/UI/registry).
repairedits a config file only; it never opens the provisioning store or a
running pipeline (see the new ADR below), so it cannot reach the data path.
The design doc's honest headline, which this PR verifies and closes: the
structured
Fix{ConfigPath, Op, Value}field already existed onConduitErrorand already round-tripped over gRPC/JSON — but no error sitepopulated it. The load-bearing work here is fix synthesis, not command
wiring: four producer sites now emit a real
Fix, a shared engineclassifies and applies it, and the CLI/MCP shells are thin callers of that
one engine.
What shipped
Producers (v1 starter set, design doc §6, items #1-#4 — deliberately not
wider; see "explicitly out of scope" in §6):
type(v2.2 rename toplugin)safe/statusinvalid but case/whitespace-normalizablesetto canonical enumsafe/workerssetto1restart/descriptionsetto truncated valuesafeEverything else keeps its existing human
Suggestionand anilFix—repairreports "no machine-appliable fix" honestly rather than fabricatingone (matches §6's out-of-scope table: plugin-name/type/ID-duplicate fixes
need closest-match logic or a store-state check that don't exist yet).
Shared engine (
cmd/conduit/internal/repair, new package):Collect/CollectContentrun the existingvalidate/lintengine(
Options{Warnings: true}) and classify every fix.classifyis default-deny: connectorsettings, a connector's ownplugin/type, DLQ config, anyidfield →data_path; anunrecognized
ConfigPathalso falls through todata_path— neversilently
safe.Applyedits an in-memory, comment-preserving*yaml.Nodetree(
github.com/conduitio/yaml/v3, already a dependency — no new dep) ratherthan a marshal-from-struct round trip, which would strip comments. Verified
against a fixture with comments: only the targeted lines change.
fails closed, in CI, not in a user's pipeline).
repairv1 scope is intentionally narrowed to one file, one v2-formatpipeline document — the same constraint
deploy/applyalready impose(
deploy.ParseSinglePipeline). Documented in the packagedoc.go.CLI:
conduit pipelines repair <file> [--apply] [--plan-hash H] [--fix PATH]... [--escalate] [--json] [--no-color]— diff-first, mirrorsdeploy/apply's hash-bound UX exactly.--applywithout--fixappliesevery
safefix only; adata_pathfix named explicitly via--fixwithout--escalaterefuses the whole run, nothing written. Atomic file write(temp + rename in the same directory).
MCP:
repair(read, always registered) andrepair_apply(write, gatedby
--allow-mutations), registered through themcp.Catalog()/tool()pattern that landed on
mainafter this branch's original base (#2608,#2613) — I rebased onto latest
mainmid-implementation to pick this uprather than hand-rolling
sdkmcp.Tool{}literals.repair_apply's inputschema has no
escalatefield at all — adata_pathfix selected viaselectalways comes backskipped, never applied, and there is noagent-settable way to force it (asserted directly against the tool's JSON
schema in
TestRepairApply_ToolSchema_HasNoEscalateField).New error codes (registered in the one
conduiterrregistry):config.field_renamed,repair.plan_stale,repair.fix_no_longer_applies,repair.data_path_fix_refused,repair.ambiguous_fix,repair.no_fixes_available.llms.txt/llms-full.txtregenerated(
go generate) — both the 6 new codes and the 2 new MCP tools appearautomatically via the existing AST-scan/
Catalog()generators, zero manualdoc duplication.
New ADR:
20260713-repair-edits-file-not-store.mdrecords the store-vs-file decision the design doc's §11 said should be
promoted once built.
Two corrections to the task brief, found while implementing
commit my worktree started from (
bfe093d) — it landed one commit later,in
#2613(2966987), alongside thellmsgengenerator (#2608). Irebased this branch onto latest
mainmid-implementation (see commithistory / the two-phase build below) specifically to land on the real
pattern rather than the manual
sdkmcp.Tool{}literals I'd started with.TestCatalog_MatchesRegisteredToolSetsis green.llms.txtgeneration — same story: didn't exist in the base commit,exists on latest
main. Rango generate ./...; both new error codes andboth new MCP tools show up automatically (diffs above), confirming the
generator's AST-scan/
Catalog()approach picked this feature up with zerospecial-casing.
Both are stated here per CLAUDE.md ("do not claim a bar is met when it
structurally isn't" / "say what was actually run") — the discrepancy was
between the task brief and the commit my worktree happened to start from, not
a false claim in the design doc itself.
Acceptance criteria (design doc §8) — status
Met: AC-1 (golden test per producer class —
TestCollect_Rename/Status/Workers/DescriptionTestValidate_RepairFixes_GoldenPerClass), AC-2 (Fix survives producer →gRPC status → back, extended to real producers —
TestValidate_RepairFixes_SurviveGRPCStatusRoundTrip,TestParseWithWarnings_RenamedField_SurvivesGRPCStatusRoundTrip), AC-3(invalid op / uncoercible value aborts the whole call, never written —
TestApply_InvalidOp_AbortsEntireCall,TestApply_UncoercibleNumericValue_AbortsEntireCall),AC-4 (
Collect/CollectContentbyte-identical —TestCollect_CollectContent_Identical), AC-5 (default-deny classifier tabletest), AC-6 (pure recompute+edit — structurally true:
cmd/conduit/internal/repairimports neither
pkg/provisioningnorpkg/pipelinein any non-test file),AC-7 (read mode never writes), AC-8 (
--applyneeds--plan-hash/--yes),AC-9 (stale hash refused, file unchanged), AC-10 (re-validation gate —
TestRevalidate_CatchesAFixThatDidNotClearItsFinding), AC-11 (atomic write,crash-mid-write leaves original intact —
TestWriteFileAtomic_*), AC-12(comments preserved), AC-13 (single JSON object — inherited structurally from
the shared
cecdysis.CommandWithResultDecoratorevery other command uses),AC-14 (data-path fix named via
--fixwithout--escalaterefuses the wholeCLI run), AC-15 (MCP never applies a
data_pathfix; noescalatefield inschema), AC-16 (
repair_applyabsent without--allow-mutations), AC-17(default = safe fixes only), AC-18 (no store mutation — structural, see
AC-6), AC-19 (partial success — one selected fix no-longer-applying doesn't
block others), AC-20 (ambiguous-fix guard), AC-21 (
no_fixes_availableonempty
--apply; clean read is exit 0), AC-22 (codes registered;llms.txt/tool catalog regenerated), AC-23 (godoc + package
doc.goinvariants).Partially met, honestly: AC-14/AC-15's data_path branch has no
end-to-end fixture, because no v1 producer classifies a fix
data_path(by design — see the out-of-scope table). I exercised the gate logic
(
gateFix,firstDataPathRefusal, the classifier itself, and the MCP tool'sschema) directly against synthetic/crafted inputs instead — documented at
each such test (
TestGateFix,TestFirstDataPathRefusal,TestBuildProposedFix_DataPathFix_NeverRendersBeforeAfter). This is a realcoverage gap that will close naturally once a wider fix producer set exists.
Adversarial self-review (per CLAUDE.md)
Found and fixed two real bugs before opening this PR:
collect()originally checked"exactly one pipeline" before checking whether the file parsed at all, so
a genuinely broken YAML file (0 pipelines) reported "repair requires
exactly one pipeline definition per file (found 0)" instead of the real
parse error. Fixed to surface the actual parse finding's message/suggestion
when the pipeline count is zero; regression test:
TestCollect_UnparseableFile_SurfacesParseError.Before/Afterdiffrendering was populated unconditionally; it happened to never leak a
secret only because no current producer targets connector
settings.Hardened
buildProposedFixto skipBefore/Afterentirely for anyFixClassDataPathfix, closing the gap structurally before a futureproducer could reopen it. Test:
TestBuildProposedFix_DataPathFix_NeverRendersBeforeAfter.Other things checked and found sound: no goroutines/shared mutable state in
the engine (race detector clean); every temp-dir path has a
defer RemoveAll;WriteFileAtomiccreates its temp file in the same directoryas the target (same-filesystem rename guarantee) and is exercised under a
simulated write failure; the rename-vs-remove+add compound-fix
implementation is documented as a deliberate simplification (in-place key
rename, not literal remove-then-add) that preserves field order/comment
attachment better than the design doc's literal framing, with no behavioral
difference. Known, accepted gap: two concurrent
repair --applyinvocations against the same file can race (last writer wins) — this is
inherent to editing a plain file with two processes, not a data-path/store
concern
pkg/provisioning'spipelineLocksaddresses, and is unchanged fromhow any other CLI file-editing tool behaves.
Testing
go build ./...,go vet ./...— clean (one pre-existing, unrelatedgo vetwarning inpkg/lifecycle-poc/funnel/worker.go, not touched by thisPR).
go test -race -short ./...— 77 packages, 0 failures (run twice: oncepre-rebase, once post-rebase on top of
#2608/#2613).golangci-lint run(bare, matchingmake verify) — 0 issues in any filethis PR touches. 34 pre-existing issues remain in unrelated files (the
vendored
pkg/plugin/processor/builtin/internal/diffpackage andconduiterr.go's constructors) — confirmed present before this PR, out ofscope here.
npx markdownlint-cli2@0.18.1on the full repo globmake verifyuses —0 errors.
go generate -x ./...+buf generate— stable, zero drift after thechanges in this PR (the "validate-generated-files"
make verifystep).pkg/provisioning/config/validate_test.go,pkg/provisioning/config/yaml/parse_warnings_test.go), the engine(
cmd/conduit/internal/repair/*_test.go— classify, collect, apply, theYAML node editor via the apply tests, atomic write), the CLI
(
cmd/conduit/root/pipelines/repair_test.go), MCP(
cmd/conduit/internal/mcp/tools_repair_test.go), and the catalogconsistency test inherited from
#2613(
TestCatalog_MatchesRegisteredToolSets, green).Docs
docs/operations/mcp-server.md— tool table + a newrepair/repair_applysection; "Known limitations" corrected (llms.txt generation is no longer a
gap; repair's v1 scope documented instead).
docs/design-documents/20260712-repair-command.md— status line updated to"implemented".
docs/architecture-decision-records/20260713-repair-edits-file-not-store.md.deploy/applyeither (only the auto-generated, non-committed
conduit docsoutput) —repair'sDocs()/Usage()/Example()follow the same pattern, so it'scovered the same way those commands are.
Not done / explicitly out of scope
(plugin-name did-you-mean, connector
/type, missing required fields, IDduplicate) — unchanged, still human-
Suggestion-only,nilFix.--deploy-after---applyconvenience flag — two explicit steps bydesign (see the new ADR's consequences section).
llms.txt) — the pieces now all exist onmain(this PR,#2608,#2588) but the scripted run itself hasn't been executed.Coverage floor / benchi regression gate / property-fuzz tests are by
Phase 1 per CLAUDE.md's Process Maturity table, not live yet — none
claimed here.