fix: preserve stopped MCPs and reject ambiguous update recovery

CHANGELOG.md

@@ -4,4 +4,7 @@
 
 ### Fixed
 
+- 自更新恢复在改写文件前严格验证恢复记录、State、提交和二进制配对；拒绝缺失、重复或矛盾证据，保留可恢复现场。
+- 编辑已停止的外部 MCP 不再意外启动连接；启动/停止、配置编辑和应用重启保持一致的启停状态。
+
 - 登录密码显隐按钮改为输入框内的眼睛图标，统一输入框宽高，支持键盘焦点、44px 触控区域、中英俄文无障碍标签及浅色/深色主题。

docs/en-US/developer-guide.md

@@ -250,3 +250,9 @@ High-value tests:
 - One-click update: `internal/update/` (status, apply, rollback), HTTP surface in
   `internal/handler/update.go`, CLI switches in `cmd/server/update_cli.go`, thin shell in
   `upgrade.sh`
+
+### Recovery evidence and MCP activation edits
+
+Self-update recovery validates JSON field names/types, duplicate keys, commit/hash relationships and the actual binary files before resuming filesystem changes. The plain-text legacy build marker remains supported, but any accompanying State is validated too. An ambiguous, incomplete or unsupported record is refused with the files and recovery evidence preserved. Do not delete a pending record just to bypass the refusal: first reconcile the source commit, live binary and previous binary/backup.
+
+Editing an existing external MCP without activation fields preserves its current enabled/stopped state. Explicit `disabled` or `external_mcp_enable` fields change activation; `disabled: true` takes precedence. A new server defaults to enabled. Start/Stop also persist the corresponding disabled state, so editing or restarting the application does not silently enable a stopped server.

docs/feature/self-update-recovery-fixes.md

@@ -0,0 +1,63 @@
+# Self-update recovery and MCP edit fixes
+
+## Intake (F0)
+
+PR #346 was merged at 72cd8d62e36e536a6b18e9e10ea73d8d16294044 on explicit user direction to merge first and repair the reviewed defects afterwards. This is corrective work on branch `codex/self-update-recovery-fixes`; the checkout was clean. No release or deployment is requested.
+
+Workflow gate: P10 feedback/iteration, implementation allowed. The frozen review at a63070d and executable PRR-002/PRR-007 counterexamples define the upstream contract; there are no missing product decisions. Acceptance requires preserved MCP stop state and refusal of ambiguous recovery before filesystem mutation.
+
+Maintainability gate: `apply.go` has 1099 lines and combines recovery orchestration and filesystem IO; `external_mcp.go` has 508 lines. Risk is high for recovery. Allowed slice: narrow fix with a separate pure JSON/evidence validation module, small call-site changes, and focused regression tests. Broad refactoring is not required. Existing update/handler tests and reviewer reproductions cover the affected boundary.
+
+## Requirements (F1, confirmed by the user's repair request)
+
+- R1 / PRR-007: editing an existing MCP without an explicit enable/disable field preserves its runtime activation state. Explicit activation fields and Start/Stop remain supported; a new server defaults to enabled. Environment expansion remains isolated from persisted raw values.
+- R2 / PRR-002: incomplete, null, duplicate-key, malformed, or contradictory recovery evidence cannot authorize a second swap, wrong rollback binary, or deletion of recovery records. Valid current records and the documented legacy marker retain safe recovery.
+- Non-goals: redesigning update UX, changing release policy, or unrelated refactoring. The two nonblocking review observations are outside this corrective slice unless required by the evidence validation boundary.
+
+## Design (F2)
+
+Recovery remains an internal on-disk contract, with no new HTTP fields. Decode exact JSON field names and types, reject duplicate keys at every depth and null scalars, and reject unknown fields rather than interpreting a newer format as current evidence. The one documented plain-text legacy build marker remains supported by its existing conservative path.
+
+For JSON build records require a target commit and exactly one proof of the old binary (SHA256 or explicit absence), validate hash/commit shapes, and reconcile the record with any persisted State for the same update or its immediate predecessor. Before restoring protected content or building, verify the live/previous files match the indicated pre-swap or post-swap phase. A successful State cannot be reinterpreted as an owed build.
+
+Rollback records require an explicit restore_binary decision; reconcile it, the live/previous hashes and the nested State, then compare with the persisted State when it still exists. Missing persisted State is allowed after the final cleanup crash window, but the self-contained record must remain coherent. Invalid evidence returns an existing state_unreadable/bad_state error and preserves recovery files.
+
+MCP request decoding retains field presence separately from bool values. For existing entries, omission preserves the manager's activation state; explicit disabled or external_mcp_enable fields determine it. New entries default to enabled. Keep raw configuration persistence and environment-expanded connection snapshots separate as today.
+
+Compatibility: properly written current journals remain readable, legacy plain-text recovery remains guarded; hand-edited or unsupported JSON requires manual correction instead of guessed mutation. Rollback of this patch restores old behavior and is not recommended while invalid journals remain.
+
+## Implementation plan (F3)
+
+1. Add recovery evidence parser/validators in a separate internal/update module; add narrow calls in build/rollback readers and pre-mutation paths. Keep producer formats unchanged.
+2. Preserve explicit field presence in the MCP request without changing its JSON shape; normalize enable state against the existing manager configuration.
+3. Promote the two failing review reproductions into portable regression tests; add duplicate/null/type/contradiction matrices and valid retry controls. Assert no build/swap, unchanged binaries and preserved journals for refusals.
+4. Run update/handler targeted suites and race checks, full Go tests, build, JS and shell checks; update changelog and verification record. Open a corrective PR and integrate the verified patch as part of the authorized merge-and-fix task, respecting repository checks.
+
+Docs: this phase record plus concise behavior notes in deployment/developer documentation if needed. No screenshots or production external services are required; tests use local Git and harmless command/HTTP markers. No version bump, tag or release publication.
+
+## Implementation (F4)
+
+Implemented strict recovery decoding in recovery_evidence.go, with exact field names, nested duplicate/type/null checks, commit/hash validation and State reconciliation. Build recovery verifies pre-move versus post-move binary evidence before content restore and rechecks files after compilation. Fresh rollback and legacy-marker recovery also read State strictly. Self-contained rollback journals validate absence/hash coherence even after State cleanup, and actual absence is checked before any cleanup.
+
+MCP edits distinguish omitted activation fields from explicit booleans. Omitted fields preserve the manager state; Start/Stop and persisted disabled are synchronized so restart retains the requested state. Existing environment snapshot expansion is preserved.
+
+Tests added for malformed build/rollback journals, all independent review counterexamples, valid partial binary rename and final cleanup retries, and MCP stop/edit/start plus config reload. Targeted reproductions pass; original full Go, build, five-package race, and 277 JS checks passed before final evidence guard refinements. Final exact-patch validation follows in F5. No production external service was contacted.
+
+### F4 follow-up: activation round-trip consistency
+
+Final GET-to-edit inspection found that manager Start/Stop retained a stale disabled flag even after changing ExternalMCPEnable. Synchronize these two existing fields in the manager as well as persistence, and test the actual GET payload after Start followed by the UI-shaped edit. Maintainability assessment: external_manager.go is 1665 lines (high risk); this narrow fix adds only three assignments beside existing activation assignments, without new control flow or broad refactoring. The additional handler/MCP race suites validate this consistency change.
+
+## Verification and user documentation (F5)
+
+- Recovery code at fab7e603: `go test -race ./internal/update ./internal/handler` passed (84.580s / 16.692s). This includes malformed/null/duplicate/contradictory records, legacy and fresh State entry points, actual absence, pre-merge phase rejection and valid crash controls.
+- Final activation consistency change at ff50af02: `go test -race ./internal/handler ./internal/mcp -count=1` passed (18.658s / 3.281s), including Stop/edit/reload/Start/GET/edit behavior. Independent targeted handler tests also passed.
+- Full Go suite, `go build ./...`, 277 JS tests and shell syntax passed during implementation. A frozen-code full Go/build rerun and exact-head GitHub CI are tracked in F6 before integration.
+- Independent fresh-context code review at ff50af02 found no remaining blockers. Six independent recovery counterexamples were promoted to permanent tests and passed. No acceptance relies solely on test status: refusals assert retained journals/binary data, and valid controls prove continued recovery.
+- English and Chinese developer guides describe refusal/reconciliation behavior and MCP activation field precedence. No public request shape or release version changed.
+- Limits: local validation is macOS arm64; Windows/Linux results come from CI. No production restart/deployment smoke or exhaustive disk fault testing was performed.
+
+## Review and integration record (F6)
+
+Corrective PR: https://github.com/AIPentest/CyberStrikeAI/pull/347 . Code snapshot ff50af028f87465954911533ac07b420c92767b8 passed independent review; subsequent changes are documentation/changelog only. PRR-002 and PRR-007 are closed by the recorded implementation and regression evidence. The Unreleased changelog records both user-visible fixes. Final integration requires exact-head cross-platform CI success and unchanged reviewed production code; no tag or deployment is part of this task.
+
+The earlier nonblocking version-display crash window and recovery-path symlink observation from PR #346 remain follow-up items, outside the two requested blocker fixes. No broader reliability guarantee is inferred from these tests.

docs/zh-CN/developer-guide.md

@@ -290,3 +290,9 @@ i18n 规范见 [前端国际化方案](frontend-i18n.md)。
 - SQLite 迁移兼容。
 
 这些地方比普通 getter/setter 更容易出现真实用户故障。
+
+### 恢复证据与 MCP 编辑启停状态
+
+自更新恢复在改写文件前验证 JSON 字段名称/类型、重复键、提交与哈希配对及实际二进制文件。旧版纯文本待编译标记仍受支持，但关联 State 也必须通过验证。记录不完整、矛盾或格式不受支持时会拒绝恢复并保留文件与恢复证据。不要仅为绕过报错而删除标记，应先核对源码提交、当前二进制和旧二进制/备份的配对。
+
+编辑已有外部 MCP 时，省略启停字段会保留当前启用/停止状态。显式 `disabled` 或 `external_mcp_enable` 可改变启停状态，`disabled: true` 优先；新建 MCP 默认启用。启动/停止按钮同时持久化对应的 disabled 状态，避免编辑或应用重启悄悄启用已停止的服务。

internal/handler/external_mcp.go

@@ -179,13 +179,10 @@ func (h *ExternalMCPHandler) AddOrUpdateExternalMCP(c *gin.Context) {
 
 	cfg := req.Config
 
-	// 官方 disabled 字段 → ExternalMCPEnable 取反
-	if cfg.Disabled {
-		cfg.ExternalMCPEnable = false
-	} else if !cfg.ExternalMCPEnable {
-		// 用户未显式设置 external_mcp_enable，官方配置默认就是启用的
-		cfg.ExternalMCPEnable = true
-	}
+	// Editing metadata does not change activation unless the request says so.
+	previous, exists := h.manager.GetConfigs()[name]
+	cfg.ExternalMCPEnable = req.activation(previous.ExternalMCPEnable, exists)
+	cfg.Disabled = !cfg.ExternalMCPEnable
 
 	// 展开 ${VAR} 环境变量
 	config.ExpandConfigEnv(&cfg)
@@ -272,6 +269,7 @@ func (h *ExternalMCPHandler) StartExternalMCP(c *gin.Context) {
 	}
 	cfg := h.config.ExternalMCP.Servers[name]
 	cfg.ExternalMCPEnable = true
+	cfg.Disabled = false
 	h.config.ExternalMCP.Servers[name] = cfg
 
 	// 保存到配置文件
@@ -326,6 +324,7 @@ func (h *ExternalMCPHandler) StopExternalMCP(c *gin.Context) {
 	}
 	cfg := h.config.ExternalMCP.Servers[name]
 	cfg.ExternalMCPEnable = false
+	cfg.Disabled = true
 	h.config.ExternalMCP.Servers[name] = cfg
 
 	// 保存到配置文件
@@ -496,7 +495,9 @@ func setStringArrayInMap(mapNode *yaml.Node, key string, values []string) {
 
 // AddOrUpdateExternalMCPRequest 添加或更新外部MCP请求
 type AddOrUpdateExternalMCPRequest struct {
-	Config config.ExternalMCPServerConfig `json:"config"`
+	Config   config.ExternalMCPServerConfig `json:"config"`
+	enable   *bool
+	disabled *bool
 }
 
 // ExternalMCPResponse 外部MCP响应

internal/handler/external_mcp_activation_test.go

@@ -0,0 +1,129 @@
+package handler
+
+import (
+	"bytes"
+	"encoding/json"
+	"net/http"
+	"net/http/httptest"
+	"sync/atomic"
+	"testing"
+	"time"
+
+	"cyberstrike-ai/internal/config"
+)
+
+func TestMCPEditPreservesStoppedState(t *testing.T) {
+	router, h, path := setupTestRouter()
+	defer cleanupTestConfig(path)
+	defer h.manager.StopAll()
+	var calls atomic.Int64
+	called := make(chan struct{}, 16)
+	endpoint := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		calls.Add(1)
+		select {
+		case called <- struct{}{}:
+		default:
+		}
+		http.Error(w, "test endpoint", 503)
+	}))
+	defer endpoint.Close()
+	cfg := config.ExternalMCPServerConfig{Type: "http", URL: endpoint.URL, Description: "before"}
+	h.config.ExternalMCP.Servers["stopped"] = cfg
+	if err := h.manager.AddOrUpdateConfig("stopped", cfg); err != nil {
+		t.Fatal(err)
+	}
+	request := func(method, url string, body []byte) *httptest.ResponseRecorder {
+		t.Helper()
+		w := httptest.NewRecorder()
+		r := httptest.NewRequest(method, url, bytes.NewReader(body))
+		r.Header.Set("Content-Type", "application/json")
+		router.ServeHTTP(w, r)
+		if w.Code != 200 {
+			t.Fatalf("%s %s: %d %s", method, url, w.Code, w.Body)
+		}
+		return w
+	}
+	request("POST", "/api/external-mcp/stopped/stop", nil)
+	// Shape emitted by the edit UI: enable omitted, descriptive metadata changed.
+	body, _ := json.Marshal(map[string]any{"config": map[string]any{"type": "http", "url": endpoint.URL, "description": "after"}})
+	request("PUT", "/api/external-mcp/stopped", body)
+	if h.manager.GetConfigs()["stopped"].ExternalMCPEnable {
+		t.Fatal("metadata edit enabled a stopped MCP")
+	}
+	persisted, err := config.Load(path)
+	if err != nil {
+		t.Fatal(err)
+	}
+	if persisted.ExternalMCP.Servers["stopped"].ExternalMCPEnable {
+		t.Fatal("stop state lost on config reload")
+	}
+	select {
+	case <-called:
+		t.Fatal("metadata edit initiated a connection")
+	case <-time.After(100 * time.Millisecond):
+	}
+	if calls.Load() != 0 {
+		t.Fatal("stopped edit started external work")
+	}
+	request("POST", "/api/external-mcp/stopped/start", nil)
+	select {
+	case <-called:
+	case <-time.After(3 * time.Second):
+		t.Fatal("explicit Start did not connect")
+	}
+	if !h.manager.GetConfigs()["stopped"].ExternalMCPEnable {
+		t.Fatal("explicit Start stayed disabled")
+	}
+	response := request("GET", "/api/external-mcp/stopped", nil)
+	var server ExternalMCPResponse
+	if err := json.Unmarshal(response.Body.Bytes(), &server); err != nil {
+		t.Fatal(err)
+	}
+	if server.Config.Disabled {
+		t.Fatal("Start left a stale disabled field in the edit response")
+	}
+	// Round-trip the actual GET payload just as the UI does, removing enable.
+	encoded, _ := json.Marshal(server.Config)
+	var edit map[string]any
+	_ = json.Unmarshal(encoded, &edit)
+	delete(edit, "external_mcp_enable")
+	edit["description"] = "after explicit start"
+	body, _ = json.Marshal(map[string]any{"config": edit})
+	request("PUT", "/api/external-mcp/stopped", body)
+	if !h.manager.GetConfigs()["stopped"].ExternalMCPEnable {
+		t.Fatal("edit after Start stopped the MCP")
+	}
+	persisted, err = config.Load(path)
+	if err != nil {
+		t.Fatal(err)
+	}
+	if !persisted.ExternalMCP.Servers["stopped"].ExternalMCPEnable {
+		t.Fatal("explicit Start not persisted")
+	}
+}
+
+func TestMCPRequestActivationPresence(t *testing.T) {
+	for _, tc := range []struct {
+		name, fields           string
+		previous, exists, want bool
+	}{
+		{"new default", "", false, false, true},
+		{"stopped edit", "", false, true, false},
+		{"running edit", "", true, true, true},
+		{"explicit enable", `,"external_mcp_enable":true`, false, true, true},
+		{"explicit stop", `,"external_mcp_enable":false`, true, true, false},
+		{"explicit disabled", `,"disabled":true`, true, true, false},
+		{"explicit enabled official", `,"disabled":false`, false, true, true},
+		{"disabled wins", `,"disabled":true,"external_mcp_enable":true`, true, true, false},
+	} {
+		t.Run(tc.name, func(t *testing.T) {
+			var r AddOrUpdateExternalMCPRequest
+			if err := json.Unmarshal([]byte(`{"config":{"description":"edit"`+tc.fields+`}}`), &r); err != nil {
+				t.Fatal(err)
+			}
+			if got := r.activation(tc.previous, tc.exists); got != tc.want {
+				t.Fatalf("activation=%v want=%v", got, tc.want)
+			}
+		})
+	}
+}

internal/handler/external_mcp_request.go

@@ -0,0 +1,41 @@
+package handler
+
+import "encoding/json"
+
+// Keep omission distinct from false without changing the public request shape.
+func (r *AddOrUpdateExternalMCPRequest) UnmarshalJSON(data []byte) error {
+	type plain AddOrUpdateExternalMCPRequest
+	var decoded plain
+	if err := json.Unmarshal(data, &decoded); err != nil {
+		return err
+	}
+	var presence struct {
+		Config struct {
+			Enable   *bool `json:"external_mcp_enable"`
+			Disabled *bool `json:"disabled"`
+		} `json:"config"`
+	}
+	if err := json.Unmarshal(data, &presence); err != nil {
+		return err
+	}
+	*r = AddOrUpdateExternalMCPRequest(decoded)
+	r.enable = presence.Config.Enable
+	r.disabled = presence.Config.Disabled
+	return nil
+}
+
+func (r AddOrUpdateExternalMCPRequest) activation(previous, exists bool) bool {
+	if r.disabled != nil && *r.disabled {
+		return false
+	}
+	if r.enable != nil {
+		return *r.enable
+	}
+	if r.disabled != nil {
+		return !*r.disabled
+	}
+	if exists {
+		return previous
+	}
+	return true
+}

internal/mcp/external_manager.go

@@ -337,6 +337,7 @@ func (m *ExternalMCPManager) startClient(name string, autoReconnect bool) error
 			if !autoReconnect {
 				m.mu.Lock()
 				serverCfg.ExternalMCPEnable = true
+				serverCfg.Disabled = false
 				m.configs[name] = serverCfg
 				m.mu.Unlock()
 			}
@@ -362,6 +363,7 @@ func (m *ExternalMCPManager) startClient(name string, autoReconnect bool) error
 	// 更新配置为启用
 	m.mu.Lock()
 	serverCfg.ExternalMCPEnable = true
+	serverCfg.Disabled = false
 	m.configs[name] = serverCfg
 	// 清除之前的错误信息（重新启动时）
 	delete(m.errors, name)
@@ -443,6 +445,7 @@ func (m *ExternalMCPManager) StopClient(name string) error {
 
 	// 更新配置为禁用
 	serverCfg.ExternalMCPEnable = false
+	serverCfg.Disabled = true
 	m.configs[name] = serverCfg
 
 	m.clearReconnectState(name)