Add safe try-activate rollback for plugin/config hot-reload#684
Merged
Conversation
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
⏱ Benchmark Results✅ No significant performance regressions detected. benchstat comparison (baseline → PR)
|
- Fix reloadEngine to build candidate before stopping old engine (safe try-activate: build failure leaves current engine untouched) - Add rollback: if candidate start fails, rebuild old config and restart - Add tryActivateEngine probe that builds without swapping any pointers - Add RegisteredModuleTypes/StepTypes/TriggerTypes to StdEngine - Add TryActivateResult type and SetTryActivateFunc to WorkflowUIHandler - Add POST /api/workflow/try-activate endpoint - Add POST /api/v1/admin/engine/try-activate to OpenAPI schema - Wire tryActivateEngine into server management handlers - Add unit tests: reload build failure, success, try-activate success/failure - Add engine tests: RegisteredModuleTypes/StepTypes/TriggerTypes - Add UI handler tests: try-activate endpoint coverage - Update docs: APPLICATION_LIFECYCLE.md, DEPLOYMENT_GUIDE.md, PLUGIN_DEVELOPMENT_GUIDE.md distinguish legacy vs safe reload semantics Agent-Logs-Url: https://github.com/GoCodeAlone/workflow/sessions/8b12843e-11ce-440e-b111-9201f556b02a Co-authored-by: intel352 <77607+intel352@users.noreply.github.com>
Copilot
AI
changed the title
[WIP] Add safe plugin/config hot-reload try-activate rollback
Add safe try-activate rollback for plugin/config hot-reload
May 15, 2026
Contributor
There was a problem hiding this comment.
Pull request overview
Introduces a safe try-activate/rollback contract for hot-reloading the engine and external plugins, plus a new probe endpoint that builds a candidate config without disturbing the live engine.
Changes:
reloadEnginereorders to build-candidate → stop-old → start-candidate, with automatic rollback to the previous config when the candidate fails to start.- Adds
POST /api/workflow/try-activate(and/api/v1/admin/engine/try-activate) with a newTryActivateResultresponse andStdEngine.RegisteredModule/Step/TriggerTypes()accessors. - Documents the legacy-vs-safe semantics in
APPLICATION_LIFECYCLE.md,DEPLOYMENT_GUIDE.md, andPLUGIN_DEVELOPMENT_GUIDE.md, and adds unit tests for the new code paths.
Reviewed changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| cmd/server/main.go | Implements safe reload sequence and tryActivateEngine probe; wires probe into the management handler. |
| cmd/server/main_test.go | Tests build-failure-preserves-engine, success replaces engine, and try-activate valid/invalid paths. |
| engine.go | Adds RegisteredModuleTypes/StepTypes/TriggerTypes accessors. |
| engine_test.go | Tests for the new accessors verifying contents and sort order. |
| module/api_workflow_ui.go | New TryActivateResult type, SetTryActivateFunc, route registration, and handleTryActivate handler. |
| module/api_workflow_ui_test.go | Tests handler 503/200/422 paths and ServeHTTP dispatch. |
| module/openapi_admin_schemas.go | Adds OpenAPI operation schema for the new endpoint (component schema not yet registered). |
| docs/APPLICATION_LIFECYCLE.md | Documents the three-stage safe reload and try-activate probe. |
| docs/DEPLOYMENT_GUIDE.md | Documents safe reload sequence and adds dry-run probe section. |
| docs/PLUGIN_DEVELOPMENT_GUIDE.md | Clarifies legacy vs. safe plugin reload semantics. |
Contributor
|
Addressed the three Copilot review findings in 0a4d878:
Focused verification passed:
|
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.
reloadEnginestopped the current engine before building the replacement — a build failure left the server with no running engine.ReloadPluginsimilarly had unsafe stop-then-start semantics. Neither path defined try-activate, rollback, or probe contracts.Core: safe
reloadEnginesequenceBefore: stop → build → start (build failure = degraded)
After: build candidate → stop old (only on build success) → start candidate → rollback to previous config on start failure
Try-activate probe endpoint
New
POST /api/workflow/try-activate(also registered at/api/v1/admin/engine/try-activate) builds a candidate engine without touching the active one. Returns structured result for automated update campaigns:{ "status": "build_ok", "moduleTypes": ["http.server"], "stepTypes": ["step.set"], "triggerTypes": ["http"] } { "status": "build_failed", "error": "module type \"bad.type\" not found" }StdEngine.RegisteredModuleTypes/StepTypes/TriggerTypes()expose registered type names for the probe resultWorkflowUIHandler.SetTryActivateFuncwires the probe into the management handlerTests
TestReloadEngine_BuildFailureKeepsPriorEngineActive— build failure leaves engine pointer unchangedTestReloadEngine_SuccessReplacesEngine— successful reload replaces engineTestTryActivateEngine_Valid/Invalid— probe returnsbuild_ok/build_failedwithout touching active engineTestStdEngine_RegisteredModule/Step/TriggerTypes— type enumeration and sort orderTestWorkflowUIHandler_TryActivate_*— HTTP 200/422/503 coverageDocs
APPLICATION_LIFECYCLE.md,DEPLOYMENT_GUIDE.md, andPLUGIN_DEVELOPMENT_GUIDE.mdnow explicitly distinguish the legacy unsafe unload→load pattern from the current safe try-activate contract.