Skip to content

feat(cli): conduit run --dev hot-reload watcher (PR2 of §4)#2616

Merged
devarismeroxa merged 3 commits into
mainfrom
feat/run-dev-hot-reload
Jul 13, 2026
Merged

feat(cli): conduit run --dev hot-reload watcher (PR2 of §4)#2616
devarismeroxa merged 3 commits into
mainfrom
feat/run-dev-hot-reload

Conversation

@devarismeroxa

Copy link
Copy Markdown
Contributor

What

conduit run --dev — the developer surface over the live in-place hot-reload engine (PR1, #2613). Watches the pipelines directory and applies config changes to running pipelines on save: a processor / name / description edit applies in place (no restart, positions continuous); a connector / DLQ / topology edit drain-restarts, each labeled. Completes execution-plan §4 (+ §3). Design: docs/design-documents/20260712-pipeline-dev-hot-reload.md (merged #2601). Risk tier 2 — a watcher over the already-Tier-1-reviewed engine.

Key points

  • Watcher lives in the runtime (pkg/conduit/dev), started after ProvisionService.Init, tied to the serve context (Ctrl-C cancels it — invariant 7). --dev sets Config.Dev.
  • --dev is the operator authorization for the applies its watcher drives (interactive, local, watched) — it does not enable the process-level API gate; remote agents stay independently gated.
  • Ensure-running: a new file, or a pipeline left stopped by a prior failed apply, is started (unless the config declares it stopped).
  • Failure modes: parse/validate error keeps the running pipeline untouched; deleted watched file leaves the pipeline running + logs (never auto-deletes); atomic-save (rename) tolerated by watching the directory.
  • --json events; conduit pipelines dev [dir] thin alias. fsnotify promoted to a direct dep (already transitively present).

Tests

Unit (event→file→pipeline mapping, debounce/coalesce, parse-error tolerance, ensure-running variants, file-delete keep-running) plus an integration test with real fsnotify + a generator→processor→file pipeline: edit the processor → applied in place; edit a source → labeled restart; syntax error → pipeline keeps running. Build/test (-race)/lint green; operator runbook added; no llms drift.

Process note

Authored by a subagent; finalized (runbook + full verification) by the main session after it stalled at the finish line. Pending independent review before merge.

🤖 Generated with Claude Code

https://claude.ai/code/session_01B3cLokwNJYLLBpdyt3cgGr

@devarismeroxa
devarismeroxa requested a review from a team as a code owner July 13, 2026 18:59
devarismeroxa added a commit that referenced this pull request Jul 13, 2026
…, Tier 1) (#2617)

* fix(processor,provisioning): live in-place apply commits running-processor config

The in-place hot-reload path (PR1, #2613) never worked end-to-end: applyInPlace
committed the new processor config through processor.Service.Update, whose
running-instance guard (ErrProcessorRunning) refused it, so ApplyPlanLive returned
the error and lifecycle.ReconfigureProcessor — the actual live swap — was never
reached. PR1's mock-based tests returned success for the mocked Update and hid it;
PR2's real-engine integration test exposed it (found in the #2616 review).

Fix (Option A): add processor.Service.UpdateWhileRunning — Update minus the
running guard — and route provisioning's updateProcessorAction through it. This is
safe for both provisioning paths: the restart path applies it on a stopped
pipeline (processor not running, so equivalent to Update), and the in-place path
immediately swaps the live node via ReconfigureProcessor to match the committed
config. Update keeps its guard for direct API/orchestrator callers, who do NOT
swap the node (verified: pkg/orchestrator still uses Update).

Regression test: TestService_UpdateWhileRunning_BypassesRunningGuard — Update
refuses a running instance (ErrProcessorRunning); UpdateWhileRunning commits the
config. The provisioning apply/import tests now assert the engine calls
UpdateWhileRunning. The full real-engine end-to-end assertion lives in PR2's
tightened integration test.

Risk tier 1 (data path). Failure-mode analysis in the PR. Fixes the §4 in-place
hot-reload blocker; unblocks PR2 (#2616).

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

* docs: fix stale godoc reference (Update -> UpdateWhileRunning) in updateProcessorAction

Review NIT on #2617.

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

---------

Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
devarismeroxa added a commit that referenced this pull request Jul 13, 2026
…e mode, sync running flag (#2619)

Completes ROADMAP §4 live in-place processor hot-reload end to end.

- MakeRunnableProcessorForReconfigure: guard-free runnable build for the live
  swap (the second running-guard that silently forced a restart every time).
- TeardownForReconfigure: tears down a swapped processor without clearing the
  shared Instance.running flag, so a swap never marks the still-running instance
  stopped (would have disarmed the Update/Delete guards). Found by independent
  review; fixed with real-runnable regression tests.
- Diff.AppliedMode: ApplyPlanLive reports the true applied path (provisioned/
  in_place/restart) so callers stop mislabeling an in-place->restart fallback.
- Instance.running is now atomic.Bool (CompareAndSwap reservation), fixing a
  pre-existing cross-goroutine data race and a test-and-set TOCTOU.

Tier-1: human sign-off given; independent review (2 findings, both fixed and
re-verified); CI green; regression tests verified fail-without-fix.

Unblocks #2616 (PR2, `conduit run --dev`).
devarismeroxa and others added 2 commits July 13, 2026 14:35
The developer-facing surface over the live in-place hot-reload engine (PR1,
#2613). `conduit run --dev` watches the pipelines directory and applies config
changes to running pipelines on save — a processor/name/description edit applies
in place (no restart), a connector/DLQ/topology edit drain-restarts, each labeled.

- The watcher lives in the runtime (pkg/conduit/dev), started after
  ProvisionService.Init and tied to the serve context so Ctrl-C cancels it
  (invariant 7). --dev sets Config.Dev.
- Per-change flow: debounce -> parse all pipelines in the file + Enrich/Validate ->
  Plan (skip empty) -> ApplyPlanLive(..., allowRestartOnRunning=true) -> ensure-
  running. --dev is itself the operator authorization for the applies it drives;
  it does NOT enable the process-level API gate.
- Ensure-running: a brand-new file or a pipeline left stopped by a prior failed
  apply is started (unless the config declares it stopped).
- Parse/validate error keeps the running pipeline untouched; a deleted watched
  file leaves its pipeline running and logs (never auto-deletes); atomic-save
  (rename) tolerated by watching the directory.
- --json structured events per apply; `conduit pipelines dev [dir]` thin alias.
- fsnotify promoted to a direct dependency (was transitive) — no new module, just
  a direct import of an already-present dep.

Tests: watcher event->file->pipeline mapping, debounce/coalesce, parse-error
tolerance, ensure-running variants, file-delete keep-running, plus an integration
test with real fsnotify + a generator->processor->file pipeline (edit the
processor -> applied in place; edit a source -> labeled restart; syntax error ->
pipeline keeps running). Operator runbook added.

Advances execution-plan §4 (completes the hot-reload arc) + §3. Risk tier 2 (a
watcher over the already-reviewed engine). Authored by a subagent; finalized
(runbook + verify) by the main session after it stalled at the finish line.
Pending independent review before merge.

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

Addresses two findings from PR2's independent review.

BLOCKER 2 (shutdown race): debouncer.run returned on ctx cancellation
without waiting for the apply goroutine it had spawned. consume's wg only
tracks run, so Watcher.Run could return — and the runtime tear the engine
and DB down — while an apply (a pipeline restart or in-place swap) was
still executing. run now owns the apply goroutine's lifetime and blocks
until it completes on the shutdown path. Regression test added
(TestDebouncer_ContextCancel_WaitsForInFlightApply), verified to fail
without the fix.

MINOR 3 (mislabeled mode): the reported mode was derived from the
pre-apply plan (LiveEligible), which is wrong when the engine falls back
in-place -> restart. The watcher now reports provisioning.Diff.AppliedMode
— the engine's ground truth — via modeFromApplied, keeping the plan-derived
value only as a fallback when the engine reports no mode. Regression test
added (TestApplyPipeline_Mode_EngineOutcomeOverridesPlanGuess).

Also tightens the full-engine integration test to assert a real in-place
apply (mode:in_place + no restart) with zero tolerance for the prior
"known issue" error branch, now that the engine live-swap works end to end.

Advances ROADMAP §4 (hot-reload / dev). Depends on the engine
live-reconfigure fix (guard #2 + Diff.AppliedMode).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01B3cLokwNJYLLBpdyt3cgGr
@devarismeroxa
devarismeroxa force-pushed the feat/run-dev-hot-reload branch from 52f98fb to 5991a48 Compare July 13, 2026 21:36
…tdown

Hardening from PR2's independent review (no blocker; robustness for a
server-side feature that never takes down live pipelines).

- Panic containment (MEDIUM): applyFile runs arbitrary, rapidly-edited user
  YAML through parse/enrich/validate/apply on a plain goroutine. An unrecovered
  panic there would crash the whole server and take every running pipeline down
  — the opposite of this package's invariant that a bad edit never touches a
  running pipeline. applyFile now recovers and reports the panic as an error
  event, keeping the server and all live pipelines up. Regression test
  TestApplyFile_RecoversFromPanic (verified: without the recover the panic
  propagates and fails the test).

- Shutdown teardown (LOW/MEDIUM): consume's per-path debouncer goroutines only
  exit on their context, but consume could also return when the fsnotify
  channels close (not just on parent-ctx cancel), leaving those goroutines
  blocked and wg.Wait hanging forever. consume now derives a child context and
  cancels it before wg.Wait, so debouncers are torn down no matter why consume
  returns; a closed fsErrors channel now returns instead of busy-spinning.
  Regression test TestConsume_ChannelClose_TearsDownAndReturns (verified: without
  the child context it hangs and times out).

- Shutdown hygiene (LOW): the debounce timer case no longer kicks off a fresh
  (possibly restart-class) apply once ctx is cancelled — it loops back so the
  ctx.Done case returns.

Advances ROADMAP §4. Part of PR2 (#2616).

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

Copy link
Copy Markdown
Contributor Author

Independent review + hardening

Fresh-context review of this PR (rebased on the merged engine PR #2619) confirmed both prior blockers are genuinely fixed and found no merge-blocker:

  • In-place works — the integration test asserts a real mode:in_place with zero restart against the real engine + real fsnotify, no failure-tolerant branch.
  • Shutdown race fixeddebouncer.run waits out the in-flight apply on cancel (buffered done, balanced receive, no deadlock); consume's wg guarantees no apply outlives Watcher.Run. Verified race-free.
  • Confirmed --dev does not set the process-level --api.allow-live-restart-apply gate; modeFromApplied fallback semantics correct against the real ApplyMode zero value.

Addressed the review's hardening findings in ff74831:

  • MEDIUM — panic containment: applyFile runs rapidly-edited user YAML on a plain goroutine; an unrecovered panic would crash the server and take down every live pipeline. It now recovers and reports the panic as an error event. Regression test TestApplyFile_RecoversFromPanic.
  • LOW/MEDIUM — shutdown teardown: consume now derives a child context cancelled before wg.Wait, so per-path debouncers are torn down no matter why it returns (parent-ctx cancel OR channel close); a closed fsErrors returns instead of busy-spinning. Regression test TestConsume_ChannelClose_TearsDownAndReturns (hangs without the fix).
  • LOW — shutdown hygiene: the debounce timer no longer starts a fresh apply once ctx is cancelled.

Deferred (non-blocking, noted for follow-up): unbounded debouncer growth per distinct filename over a very long session; --dev.json naming (scoped streaming surface) vs the repo-standard --json; renderHuman not printing ConfigPath.

@devarismeroxa
devarismeroxa merged commit d5fcefb into main Jul 13, 2026
6 checks passed
@devarismeroxa
devarismeroxa deleted the feat/run-dev-hot-reload branch July 13, 2026 21:56
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