fix(engine): complete live in-place processor reconfigure and report its true mode

[devarismeroxa]

Summary

Two engine gaps blocked ROADMAP §4 live hot-reload from working end to end — both hidden by mock-based tests, both surfaced by PR2's full-engine integration test (which runs the reconfigure path against the real processor.Service).

1. Second running-guard (the actual fix).
#2617 removed processor.Service.Update's running-guard (via UpdateWhileRunning), but MakeRunnableProcessor has its own guard: it refuses a running instance (ErrProcessorRunning) and flips i.running. lifecycle.ReconfigureProcessor called it on the still-running instance, got ErrProcessorRunning, so the live swap never happened and ApplyPlanLive silently fell back to a restart every time. New MakeRunnableProcessorForReconfigure builds the runnable without the guard and without touching i.running (the instance legitimately stays running across an open-before-teardown swap; the caller owns that lifetime). MakeRunnableProcessor is unchanged for the fresh-start path.

2. Honest applied-mode reporting.
ApplyPlanLive returned no signal of the path it took, so callers guessed the mode from the pre-apply plan (LiveEligible) — wrong whenever a live-eligible diff falls back to a restart (e.g. a parallel processor that can't be swapped live). New provisioning.ApplyMode + Diff.AppliedMode carry the ground truth, set at each ApplyPlanLive success return. AppliedMode is a runtime outcome, excluded from JSON/hash/wire (json:"-"); Plan never sets it.

Risk tier: 1 (data path — processor lifecycle + provisioning apply path)

Requires human (DeVaris) sign-off per CONTRIBUTING / CLAUDE.md — this PR does not merge on automated review alone.

Failure-mode analysis

• MakeRunnableProcessorForReconfigure is guard-free by design. Misuse (calling it for a fresh start) could produce two runnables wrapping one instance. It has exactly one call site (ReconfigureProcessor), which pairs it with an immediate open-before-teardown node swap; the fresh-start path keeps the guarded MakeRunnableProcessor. Both behaviors are pinned by tests.
• Ack/position safety unchanged (invariant 3): the swap happens at a record boundary, the new processor opens before the old tears down, and on open failure the old keeps running — no drop. No serialized-format changes.
• Diff.AppliedMode is additive and non-behavioral: changes no apply logic, only reports what happened. Excluded from the hash's hashable view (which lists its own fields) and from the proto, so no wire / stale-plan impact.
• Observability: the mislabel would have shown as mode:in_place logs alongside a concurrent pipeline started (restart) log — exactly what PR2's integration test now cross-checks.
• Rollback: revert this commit; §4 in-place reverts to always-restart (prior behavior), no data-format migration.

Tests (each verified to fail without the corresponding fix)

• TestService_MakeRunnableProcessorForReconfigure_BypassesRunningGuard — asserts MakeRunnableProcessor still refuses a running instance while the reconfigure variant builds one and leaves running untouched.
• TestApplyPlanLive_ProcessorUpdate_NotLiveReconfigurable_FallsBackToRestart — now asserts AppliedMode == restart despite a live-eligible plan (the exact mislabel bug).
• AppliedMode assertions on the in_place / restart / provisioned / empty(unknown) paths.

Adversarial self-review

Re-read the diff for error paths, concurrency, resource cleanup, and the data-integrity invariants:

• Confirmed the guard-free builder is unreachable except via the paired swap; no path flips running twice or leaks a plugin.
• Confirmed AppliedMode is set only on success returns; error/stale/refusal returns leave it Unknown, and consumers fall back rather than assert a false mode.
• Confirmed the hash is computed over an explicit hashable{PipelineID, Changes, Desired}, so the new field cannot perturb plan hashes.

Verification

go build ./... clean · golangci-lint 0 issues · pkg/processor + pkg/provisioning + pkg/lifecycle green (pkg/lifecycle has a pre-existing flaky error-injection test unrelated to this change; passes on re-run).

Advances ROADMAP §4. Unblocks #2616 (PR2).

🤖 Generated with Claude Code

https://claude.ai/code/session_01B3cLokwNJYLLBpdyt3cgGr

[devarismeroxa]

Independent review cycle (adversarial self-review + fresh-context reviewer)

A fresh-context senior review of the first commit (674b41f) caught a HIGH, merge-blocking bug that the mock-based tests were structurally blind to — worth recording since this PR is what makes the live swap execute for the first time:

Finding: RunnableProcessor embeds a shared *Instance; its Teardown clears running. applyPendingSwap tore down the swapped-out old runnable (success) / failed-new runnable (open failure) via that plain Teardown while the instance stayed running via the other runnable — leaving the live instance marked running=false and silently disarming the Update/Delete running-guards for the rest of the pipeline's life.

Fix (cc95503): RunnableProcessor.TeardownForReconfigure tears the plugin down without clearing running; applyPendingSwap uses it at both swap sites (type-assert with Teardown fallback for mocks). running is cleared only by a real pipeline stop. Two regression tests added, both verified to fail without the fix:

• TestReconfigureSwap_KeepsInstanceRunning_GuardsStayArmed (pkg/processor)
• TestProcessorNode_Reconfigure_{SuccessfulSwap,OpenFailure}_UsesReconfigureTeardown (pkg/lifecycle/stream)

Re-review of the fix: verified RESOLVED, nothing merge-blocking — reviewer independently confirmed by reversion that each test fails without its fix, that no swap path still clears running, no new leak, and the concurrency story improves (one running write removed from the Run goroutine).

Pre-existing follow-up (not introduced here, non-blocking)

Instance.running is an unsynchronized bool read by Update/Delete (API goroutine) and written by Teardown at stop (Run goroutine). This cross-goroutine access predates this PR; the fix strictly reduces the write sites. Worth a follow-up to make running a mutex/atomic — flagging rather than expanding this PR's scope.

[devarismeroxa]

Follow-up: Instance.running made atomic (commit 0222371)

At Tier-1 sign-off the pre-existing running cross-goroutine race was requested fixed before merge. Done:

• Instance.running is now atomic.Bool; MakeRunnableProcessor reserves it with CompareAndSwap (also closing a pre-existing test-and-set TOCTOU, with Store(false) rollback on build failure); guards use Load(); Teardown uses Store(false). Store.encode encodes the pointer to avoid copying the atomic (go vet copylocks).
• Regression test TestService_RunningFlag_ConcurrentUpdateAndTeardown_NoRace — independently verified to report WARNING: DATA RACE and fail under -race with the old plain bool, and pass with the atomic. go vet ./... copylocks-clean.

Independent re-review confirmed the race is fully resolved and the change safe. One remaining pre-existing item (non-blocking, unchanged by this PR): the logical check-then-act in Update/Delete (Load()==false then mutate) is not atomic against a concurrent MakeRunnableProcessor CAS; closing it needs a lock spanning the build/mutate — a larger design change tracked for a follow-up, not this PR.