fix(mcp): stop task plan writes from silently erasing prior content (#2977)

Co-authored-by: nova28 <17953305+nova28@users.noreply.github.com>
Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>
Co-authored-by: Carlos Florencio <carlosmflorencio@gmail.com>

apps/backend/internal/mcp/handlers/handlers.go

@@ -3873,17 +3873,19 @@ func (h *Handlers) handleCreateTaskPlan(ctx context.Context, msg *ws.Message) (*
 		createdBy = "agent"
 	}
 
+	guard := h.evaluatePlanWriteGuard(ctx, req.TaskID, req.Content)
 	plan, err := h.planService.CreatePlan(ctx, service.CreatePlanRequest{
-		TaskID:    req.TaskID,
-		Title:     req.Title,
-		Content:   req.Content,
-		CreatedBy: createdBy,
+		TaskID:           req.TaskID,
+		Title:            req.Title,
+		Content:          req.Content,
+		CreatedBy:        createdBy,
+		ForceNewRevision: guard.forceNewRevision,
 	})
 	if err != nil {
 		return planws.CreateError(msg, err)
 	}
 
-	return ws.NewResponse(msg.ID, msg.Action, dto.TaskPlanFromModel(plan))
+	return ws.NewResponse(msg.ID, msg.Action, planWritePayload(dto.TaskPlanFromModel(plan), guard))
 }
 
 // handleGetTaskPlan retrieves a task plan.
@@ -3925,17 +3927,19 @@ func (h *Handlers) handleUpdateTaskPlan(ctx context.Context, msg *ws.Message) (*
 		createdBy = "agent"
 	}
 
+	guard := h.evaluatePlanWriteGuard(ctx, req.TaskID, req.Content)
 	plan, err := h.planService.UpdatePlan(ctx, service.UpdatePlanRequest{
-		TaskID:    req.TaskID,
-		Title:     req.Title,
-		Content:   req.Content,
-		CreatedBy: createdBy,
+		TaskID:           req.TaskID,
+		Title:            req.Title,
+		Content:          req.Content,
+		CreatedBy:        createdBy,
+		ForceNewRevision: guard.forceNewRevision,
 	})
 	if err != nil {
 		return planws.UpdateError(msg, err)
 	}
 
-	return ws.NewResponse(msg.ID, msg.Action, dto.TaskPlanFromModel(plan))
+	return ws.NewResponse(msg.ID, msg.Action, planWritePayload(dto.TaskPlanFromModel(plan), guard))
 }
 
 // handleDeleteTaskPlan deletes a task plan.

apps/backend/internal/mcp/handlers/task_plan_guard.go

@@ -0,0 +1,166 @@
+package handlers
+
+import (
+	"context"
+	"fmt"
+	"unicode/utf8"
+
+	"github.com/kandev/kandev/internal/task/dto"
+)
+
+const (
+	// planTruncationMinPriorChars is the floor below which a shrinking write
+	// isn't worth flagging. A 300-char stub plan losing half its content is
+	// not a review-round-costing event; both real WO-38 incidents were on
+	// 40k+ char plans.
+	planTruncationMinPriorChars = 2000
+
+	// planTruncationMaxRetainRatio: this workflow's normal write is an
+	// append ("appends its own section"), so a plan's content grows
+	// monotonically across a build. A write that keeps less than half of the
+	// prior document is anomalous by construction, not a normal edit. Both
+	// real WO-38 incidents retained 22.8% and 23.9% of the prior plan — this
+	// line catches both with wide margin.
+	planTruncationMaxRetainRatio = 0.5
+)
+
+// planTruncationDetected reports whether newContent looks like an accidental
+// destructive truncation of priorContent, rather than a deliberate edit or a
+// legitimate (smaller) prune.
+//
+// Measured in runes, not bytes: len() on a Go string counts UTF-8 bytes, so a
+// script change (e.g. an ASCII plan rewritten in CJK) can retain a small
+// fraction of the document's characters while retaining most of its bytes —
+// silently defeating this guard on exactly the kind of loss it exists to
+// catch. Kandev ships zh-cn/zh-hk/zh-tw/pt-pt locales, so non-ASCII plan
+// content is not hypothetical.
+func planTruncationDetected(priorContent, newContent string) bool {
+	priorLen := utf8.RuneCountInString(priorContent)
+	if priorLen < planTruncationMinPriorChars {
+		return false
+	}
+	return float64(utf8.RuneCountInString(newContent)) < float64(priorLen)*planTruncationMaxRetainRatio
+}
+
+// planTruncationWarning renders the agent-facing warning appended to a plan
+// write's tool result when planTruncationDetected reports true. It states
+// plainly that the write replaced the entire document — update_task_plan_kandev
+// and create_task_plan_kandev have no partial-update mode — and names the
+// prior revision number when it is known.
+//
+// priorRevisionNumber of 0 means the prior revision could not be established
+// (the revision-history lookup itself failed after truncation was already
+// detected off the plan content). Revision numbering starts at 1
+// (NextTaskPlanRevisionNumber), so 0 is never a real revision — in that case
+// the "in plan revision N" clause is omitted rather than naming a revision
+// that cannot exist.
+//
+// It deliberately does NOT tell the caller to "recover" the content by
+// calling an MCP tool: none of the four registered plan tools can read a
+// past revision (get_task_plan_kandev returns the current, now-truncated,
+// HEAD). Telling an agent to "recover it" here would send it to the only
+// tool it has, get back the truncated document, and fall back to
+// reconstructing the plan from memory — the exact WO-38 failure this guard
+// exists to stop. Instead it says where the content lives (revision
+// history, Kandev UI only) and that the caller cannot fetch it itself, so
+// the caller stops and surfaces the loss instead of guessing.
+func planTruncationWarning(priorContent, newContent string, priorRevisionNumber int) string {
+	priorLen := utf8.RuneCountInString(priorContent)
+	newLen := utf8.RuneCountInString(newContent)
+	// dropped is always >= 0: the only caller, evaluatePlanWriteGuard, invokes this
+	// after planTruncationDetected has already confirmed newLen < priorLen.
+	dropped := priorLen - newLen
+	droppedPct := float64(dropped) / float64(priorLen) * 100
+
+	preservedIn := "the task's plan revision history"
+	if priorRevisionNumber > 0 {
+		preservedIn = fmt.Sprintf("plan revision %d, in the task's plan revision history", priorRevisionNumber)
+	}
+
+	return fmt.Sprintf(
+		"WARNING: this write replaced %d chars with %d (dropped %d chars, %.0f%%). "+
+			"Plan writes REPLACE THE ENTIRE DOCUMENT — there is no partial update or append "+
+			"mode. The pre-write content is preserved in %s — recoverable from the Kandev UI, "+
+			"but NOT fetchable through the MCP plan tools (get_task_plan_kandev returns the "+
+			"current, now-truncated, content, not that revision). If this drop was not "+
+			"intentional, stop and surface the loss rather than rewriting the plan from memory.",
+		priorLen, newLen, dropped, droppedPct, preservedIn,
+	)
+}
+
+// planWriteGuardResult carries the truncation-guard outcome for a plan
+// create/update: whether the underlying revision write must be forced to
+// append rather than coalesce, and the warning text (if any) to surface in
+// the tool response.
+type planWriteGuardResult struct {
+	forceNewRevision bool
+	warning          string
+	priorRevision    int
+}
+
+// evaluatePlanWriteGuard compares a task's current plan content against an
+// incoming write and decides whether it looks like an accidental
+// whole-document truncation. It covers both create_task_plan_kandev (which
+// upserts, so a create over an existing plan is the same destructive write
+// through a different door) and update_task_plan_kandev.
+//
+// A failure to fetch the current plan is non-fatal: the write proceeds
+// without a truncation warning, but it forces a new revision so a later
+// successful write cannot coalesce into an unknown prior revision. A failure
+// to list the revision history is handled differently:
+// truncation has already been detected from the plan content at that point,
+// so the guard still forces a new revision and still warns — it renders the
+// warning without a specific revision number (see planTruncationWarning)
+// instead of silently dropping the warning, because clearing
+// forceNewRevision here would let this destructive write coalesce into, and
+// overwrite, the only surviving copy of the pre-truncation content.
+func (h *Handlers) evaluatePlanWriteGuard(ctx context.Context, taskID, newContent string) planWriteGuardResult {
+	existing, err := h.planService.GetPlan(ctx, taskID)
+	if err != nil {
+		return planWriteGuardResult{forceNewRevision: true}
+	}
+	if existing == nil {
+		return planWriteGuardResult{}
+	}
+	if !planTruncationDetected(existing.Content, newContent) {
+		return planWriteGuardResult{}
+	}
+
+	priorRevisionNumber := 0
+	if revisions, revErr := h.planService.ListRevisions(ctx, taskID); revErr == nil && len(revisions) > 0 {
+		priorRevisionNumber = revisions[0].RevisionNumber
+	}
+
+	return planWriteGuardResult{
+		forceNewRevision: true,
+		warning:          planTruncationWarning(existing.Content, newContent, priorRevisionNumber),
+		priorRevision:    priorRevisionNumber,
+	}
+}
+
+// planWriteResponse extends the standard plan DTO with a truncation warning
+// for the MCP write actions only. It deliberately does not touch
+// dto.TaskPlanDTO itself — the browser plan editor (which has a visible diff
+// and revision history, and uses TaskPlanDTO as-is) is unaffected.
+// json.Marshal promotes an embedded pointer struct's exported fields to the
+// top level, so a non-truncating write still marshals to the identical shape
+// callers see today.
+type planWriteResponse struct {
+	*dto.TaskPlanDTO
+	PlanWriteWarning    string `json:"plan_write_warning,omitempty"`
+	PriorRevisionNumber int    `json:"prior_revision_number,omitempty"`
+}
+
+// planWritePayload wraps plan in a planWriteResponse when guard carries a
+// warning, otherwise returns plan unwrapped so an unaffected write's
+// response shape is unchanged.
+func planWritePayload(plan *dto.TaskPlanDTO, guard planWriteGuardResult) interface{} {
+	if guard.warning == "" {
+		return plan
+	}
+	return planWriteResponse{
+		TaskPlanDTO:         plan,
+		PlanWriteWarning:    guard.warning,
+		PriorRevisionNumber: guard.priorRevision,
+	}
+}

apps/backend/internal/mcp/handlers/task_plan_guard_test.go

@@ -0,0 +1,451 @@
+package handlers
+
+import (
+	"context"
+	"encoding/json"
+	"errors"
+	"path/filepath"
+	"strings"
+	"testing"
+	"time"
+
+	"github.com/jmoiron/sqlx"
+	"github.com/kandev/kandev/internal/common/logger"
+	"github.com/kandev/kandev/internal/db"
+	"github.com/kandev/kandev/internal/events/bus"
+	"github.com/kandev/kandev/internal/task/models"
+	"github.com/kandev/kandev/internal/task/repository/sqlite"
+	"github.com/kandev/kandev/internal/task/service"
+	v1 "github.com/kandev/kandev/pkg/api/v1"
+	ws "github.com/kandev/kandev/pkg/websocket"
+)
+
+func mustMarshalPlanPayload(t *testing.T, v map[string]any) string {
+	t.Helper()
+	data, err := json.Marshal(v)
+	if err != nil {
+		t.Fatalf("marshal payload: %v", err)
+	}
+	return string(data)
+}
+
+func revisionWithNumber(t *testing.T, revisions []*models.TaskPlanRevision, number int) *models.TaskPlanRevision {
+	t.Helper()
+	for _, rev := range revisions {
+		if rev.RevisionNumber == number {
+			return rev
+		}
+	}
+	t.Fatalf("no revision with number %d among %d revisions", number, len(revisions))
+	return nil
+}
+
+// TestMCPPlanTruncationGuard_WarnsAndPreservesHistory pins the defect this
+// card fixes: a write that drops the majority of a substantial plan today
+// returns plain success with no signal that anything shrank (WO-38, task
+// 809498b3, measured two incidents dropping 76-77% of a 40k+ char plan).
+//
+// This asserts against a response shape (plan_write_warning,
+// prior_revision_number) that does not exist yet, so it fails first.
+func TestMCPPlanTruncationGuard_WarnsAndPreservesHistory(t *testing.T) {
+	h := newMCPPlanTestHandlers(t)
+	ctx := context.Background()
+
+	large := strings.Repeat("x", 40000)
+	small := strings.Repeat("y", 10000) // ~25% retained, matching the WO-38 magnitude
+
+	createOut, err := h.handleCreateTaskPlan(ctx, mcpPlanMsg(t, ws.ActionMCPCreateTaskPlan,
+		mustMarshalPlanPayload(t, map[string]any{
+			"task_id": mcpPlanTaskID,
+			"title":   "Ship it",
+			"content": large,
+		})))
+	if err != nil {
+		t.Fatalf("handleCreateTaskPlan: %v", err)
+	}
+	created := decodeMCPPlanPayload(t, createOut)
+	if warning, ok := created["plan_write_warning"]; ok {
+		t.Errorf("unexpected warning on initial create (no prior plan to truncate): %v", warning)
+	}
+
+	updateOut, err := h.handleUpdateTaskPlan(ctx, mcpPlanMsg(t, ws.ActionMCPUpdateTaskPlan,
+		mustMarshalPlanPayload(t, map[string]any{
+			"task_id": mcpPlanTaskID,
+			"content": small,
+		})))
+	if err != nil {
+		t.Fatalf("handleUpdateTaskPlan: %v", err)
+	}
+	updated := decodeMCPPlanPayload(t, updateOut)
+	if updated["content"] != small {
+		t.Errorf("content = %v, want the new (truncated) content", updated["content"])
+	}
+
+	warning, _ := updated["plan_write_warning"].(string)
+	if warning == "" {
+		t.Fatal("expected a truncation warning naming the character drop, got none")
+	}
+	if !strings.Contains(warning, "40000") || !strings.Contains(warning, "10000") {
+		t.Errorf("warning does not name the character drop (40000 -> 10000): %q", warning)
+	}
+	if !strings.Contains(strings.ToLower(warning), "entire") && !strings.Contains(strings.ToLower(warning), "whole document") {
+		t.Errorf("warning does not explain the write replaced the whole document: %q", warning)
+	}
+
+	priorRev, ok := updated["prior_revision_number"].(float64)
+	if !ok || int(priorRev) != 1 {
+		t.Errorf("prior_revision_number = %v, want 1", updated["prior_revision_number"])
+	}
+
+	// The truncating write must NOT coalesce: revision 1 must survive as its
+	// own row with the full pre-truncation content, not be overwritten
+	// in-place by mergeRevisionInTx.
+	revisions, err := h.planService.ListRevisions(ctx, mcpPlanTaskID)
+	if err != nil {
+		t.Fatalf("ListRevisions: %v", err)
+	}
+	if len(revisions) != 2 {
+		t.Fatalf("expected 2 separate revisions (no coalesce), got %d", len(revisions))
+	}
+	rev1 := revisionWithNumber(t, revisions, 1)
+	fullRev1, err := h.planService.GetRevision(ctx, rev1.ID)
+	if err != nil {
+		t.Fatalf("GetRevision(rev1): %v", err)
+	}
+	if fullRev1.Content != large {
+		t.Errorf("revision 1 content was mutated by the truncating write; got len=%d, want len=%d",
+			len(fullRev1.Content), len(large))
+	}
+}
+
+// TestMCPPlanTruncationGuard_SmallDropsAreQuiet pins the two false-positive
+// guards from the threshold: a small plan (under the 2,000-char floor) and a
+// modest, legitimate shrink (well above the 50% retain line) must not warn.
+func TestMCPPlanTruncationGuard_SmallDropsAreQuiet(t *testing.T) {
+	h := newMCPPlanTestHandlers(t)
+	ctx := context.Background()
+
+	t.Run("small plan under the floor", func(t *testing.T) {
+		_, err := h.handleCreateTaskPlan(ctx, mcpPlanMsg(t, ws.ActionMCPCreateTaskPlan,
+			mustMarshalPlanPayload(t, map[string]any{
+				"task_id": mcpPlanTaskID,
+				"content": "a short plan",
+			})))
+		if err != nil {
+			t.Fatalf("handleCreateTaskPlan: %v", err)
+		}
+		out, err := h.handleUpdateTaskPlan(ctx, mcpPlanMsg(t, ws.ActionMCPUpdateTaskPlan,
+			mustMarshalPlanPayload(t, map[string]any{
+				"task_id": mcpPlanTaskID,
+				"content": "x",
+			})))
+		if err != nil {
+			t.Fatalf("handleUpdateTaskPlan: %v", err)
+		}
+		updated := decodeMCPPlanPayload(t, out)
+		if warning, ok := updated["plan_write_warning"]; ok {
+			t.Errorf("unexpected warning for a sub-floor plan: %v", warning)
+		}
+	})
+
+	t.Run("legitimate prune retains more than half", func(t *testing.T) {
+		large := strings.Repeat("x", 40000)
+		retained := strings.Repeat("y", 25000) // 62.5% retained, above the 50% line
+		_, err := h.handleCreateTaskPlan(ctx, mcpPlanMsg(t, ws.ActionMCPCreateTaskPlan,
+			mustMarshalPlanPayload(t, map[string]any{
+				"task_id": mcpPlanlessID,
+				"content": large,
+			})))
+		if err != nil {
+			t.Fatalf("handleCreateTaskPlan: %v", err)
+		}
+		out, err := h.handleUpdateTaskPlan(ctx, mcpPlanMsg(t, ws.ActionMCPUpdateTaskPlan,
+			mustMarshalPlanPayload(t, map[string]any{
+				"task_id": mcpPlanlessID,
+				"content": retained,
+			})))
+		if err != nil {
+			t.Fatalf("handleUpdateTaskPlan: %v", err)
+		}
+		updated := decodeMCPPlanPayload(t, out)
+		if warning, ok := updated["plan_write_warning"]; ok {
+			t.Errorf("unexpected warning for a legitimate >50%% retain: %v", warning)
+		}
+	})
+}
+
+// TestMCPPlanTruncationGuard_NonASCIIUsesCharacterCount pins Review round 1
+// Finding 1: len() on a Go string counts UTF-8 bytes, not characters. A plan
+// rewritten from ASCII into a CJK script can drop 80% of its characters
+// while retaining 60% of its bytes, so a byte-counting guard stays silent on
+// a loss larger than either real WO-38 incident. This asserts the guard is
+// measured in runes: it must fire on this drop even though the byte ratio
+// alone would not cross the threshold.
+func TestMCPPlanTruncationGuard_NonASCIIUsesCharacterCount(t *testing.T) {
+	h := newMCPPlanTestHandlers(t)
+	ctx := context.Background()
+
+	// 4000 ASCII runes == 4000 bytes.
+	large := strings.Repeat("x", 4000)
+	// 800 CJK runes, each 3 UTF-8 bytes == 2400 bytes: 20% of characters
+	// retained, but 60% of bytes retained — the byte ratio alone would not
+	// cross the 50% line, but the character ratio must.
+	small := strings.Repeat("好", 800)
+
+	_, err := h.handleCreateTaskPlan(ctx, mcpPlanMsg(t, ws.ActionMCPCreateTaskPlan,
+		mustMarshalPlanPayload(t, map[string]any{
+			"task_id": mcpPlanTaskID,
+			"content": large,
+		})))
+	if err != nil {
+		t.Fatalf("handleCreateTaskPlan: %v", err)
+	}
+
+	out, err := h.handleUpdateTaskPlan(ctx, mcpPlanMsg(t, ws.ActionMCPUpdateTaskPlan,
+		mustMarshalPlanPayload(t, map[string]any{
+			"task_id": mcpPlanTaskID,
+			"content": small,
+		})))
+	if err != nil {
+		t.Fatalf("handleUpdateTaskPlan: %v", err)
+	}
+	updated := decodeMCPPlanPayload(t, out)
+
+	warning, _ := updated["plan_write_warning"].(string)
+	if warning == "" {
+		t.Fatal("expected a truncation warning for an 80 percent character drop disguised as a 60 percent byte retain, got none")
+	}
+	if !strings.Contains(warning, "4000") || !strings.Contains(warning, "800") {
+		t.Errorf("warning does not name the character counts (4000 -> 800): %q", warning)
+	}
+}
+
+// TestMCPPlanTruncationGuard_CreateOverExistingPlanWarns pins the deliberate
+// scope extension in handleCreateTaskPlan: CreatePlan upserts, so a create
+// call over an existing large plan is the same destructive write as update,
+// through a different door, and must be guarded identically.
+func TestMCPPlanTruncationGuard_CreateOverExistingPlanWarns(t *testing.T) {
+	h := newMCPPlanTestHandlers(t)
+	ctx := context.Background()
+
+	large := strings.Repeat("x", 40000)
+	small := strings.Repeat("y", 10000)
+
+	_, err := h.handleCreateTaskPlan(ctx, mcpPlanMsg(t, ws.ActionMCPCreateTaskPlan,
+		mustMarshalPlanPayload(t, map[string]any{
+			"task_id": mcpPlanlessID,
+			"content": large,
+		})))
+	if err != nil {
+		t.Fatalf("handleCreateTaskPlan (initial): %v", err)
+	}
+
+	out, err := h.handleCreateTaskPlan(ctx, mcpPlanMsg(t, ws.ActionMCPCreateTaskPlan,
+		mustMarshalPlanPayload(t, map[string]any{
+			"task_id": mcpPlanlessID,
+			"content": small,
+		})))
+	if err != nil {
+		t.Fatalf("handleCreateTaskPlan (overwrite): %v", err)
+	}
+	updated := decodeMCPPlanPayload(t, out)
+
+	warning, _ := updated["plan_write_warning"].(string)
+	if warning == "" {
+		t.Fatal("expected a truncation warning when create_task_plan_kandev overwrites an existing large plan, got none")
+	}
+}
+
+// failingRevisionsRepo wraps the real sqlite repository but forces
+// ListTaskPlanRevisions to always fail, simulating a transient lookup
+// failure (e.g. SQLITE_BUSY) that happens after GetPlan has already
+// succeeded.
+type failingRevisionsRepo struct {
+	*sqlite.Repository
+	failGetPlan bool
+}
+
+func (r *failingRevisionsRepo) ListTaskPlanRevisions(context.Context, string, int) ([]*models.TaskPlanRevision, error) {
+	return nil, errors.New("simulated revision lookup failure")
+}
+
+func (r *failingRevisionsRepo) GetTaskPlan(ctx context.Context, taskID string) (*models.TaskPlan, error) {
+	if r.failGetPlan {
+		r.failGetPlan = false
+		return nil, errors.New("simulated plan lookup failure")
+	}
+	return r.Repository.GetTaskPlan(ctx, taskID)
+}
+
+// newMCPPlanTestHandlersWithFailingRevisionLookup builds Handlers like
+// newMCPPlanTestHandlers, but backed by a plan service whose ListRevisions
+// call always fails.
+func newMCPPlanTestHandlersWithFailingRevisionLookup(t *testing.T) (*Handlers, *failingRevisionsRepo) {
+	t.Helper()
+	dbConn, err := db.OpenSQLite(filepath.Join(t.TempDir(), "test.db"))
+	if err != nil {
+		t.Fatalf("OpenSQLite: %v", err)
+	}
+	sqlxDB := sqlx.NewDb(dbConn, "sqlite3")
+	t.Cleanup(func() {
+		if err := sqlxDB.Close(); err != nil {
+			t.Errorf("close database: %v", err)
+		}
+	})
+	repo, err := sqlite.NewWithDB(sqlxDB, sqlxDB, nil)
+	if err != nil {
+		t.Fatalf("sqlite.NewWithDB: %v", err)
+	}
+	t.Cleanup(func() {
+		if err := repo.Close(); err != nil {
+			t.Errorf("close repository: %v", err)
+		}
+	})
+
+	log, err := logger.NewLogger(logger.LoggingConfig{Level: "error", Format: "json", OutputPath: "stdout"})
+	if err != nil {
+		t.Fatalf("NewLogger: %v", err)
+	}
+	eventBus := bus.NewMemoryEventBus(log)
+	t.Cleanup(func() { eventBus.Close() })
+
+	ctx := context.Background()
+	if err := repo.CreateWorkspace(ctx, &models.Workspace{ID: mcpPlanWS, Name: "Plan WS"}); err != nil {
+		t.Fatalf("CreateWorkspace: %v", err)
+	}
+	if err := repo.CreateWorkflow(ctx, &models.Workflow{ID: mcpPlanWF, WorkspaceID: mcpPlanWS, Name: "WF"}); err != nil {
+		t.Fatalf("CreateWorkflow: %v", err)
+	}
+	now := time.Now().UTC()
+	task := &models.Task{
+		ID:          mcpPlanTaskID,
+		WorkspaceID: mcpPlanWS,
+		WorkflowID:  mcpPlanWF,
+		Title:       "Plan target",
+		State:       v1.TaskStateCreated,
+		Priority:    "medium",
+		CreatedAt:   now,
+		UpdatedAt:   now,
+	}
+	if err := repo.CreateTask(ctx, task); err != nil {
+		t.Fatalf("CreateTask: %v", err)
+	}
+
+	wrapped := &failingRevisionsRepo{Repository: repo}
+	return &Handlers{planService: service.NewPlanService(wrapped, eventBus, log), logger: log}, wrapped
+}
+
+// TestMCPPlanTruncationGuard_RevisionLookupFailureOmitsRevisionNumber pins
+// Review round 2 Finding 3: revision numbering starts at 1
+// (NextTaskPlanRevisionNumber), so "plan revision 0" can never be a real
+// revision. When ListRevisions fails after GetPlan has already detected a
+// truncating write, evaluatePlanWriteGuard must still force a new revision
+// and still warn — silently returning an empty guard here would let the
+// coalescing path overwrite the only surviving copy of the pre-truncation
+// content — but the rendered warning must not claim the content lives in
+// revision 0.
+func TestMCPPlanTruncationGuard_RevisionLookupFailureOmitsRevisionNumber(t *testing.T) {
+	h, _ := newMCPPlanTestHandlersWithFailingRevisionLookup(t)
+	ctx := context.Background()
+
+	large := strings.Repeat("x", 40000)
+	small := strings.Repeat("y", 10000)
+
+	_, err := h.handleCreateTaskPlan(ctx, mcpPlanMsg(t, ws.ActionMCPCreateTaskPlan,
+		mustMarshalPlanPayload(t, map[string]any{
+			"task_id": mcpPlanTaskID,
+			"content": large,
+		})))
+	if err != nil {
+		t.Fatalf("handleCreateTaskPlan: %v", err)
+	}
+
+	// Check the guard directly before the write mutates the stored plan, so
+	// "existing" below is still the large content.
+	guard := h.evaluatePlanWriteGuard(ctx, mcpPlanTaskID, small)
+	if !guard.forceNewRevision {
+		t.Error("forceNewRevision must stay true even when the revision lookup fails, " +
+			"otherwise a truncating write can coalesce into the only surviving revision")
+	}
+	if guard.warning == "" {
+		t.Fatal("expected a truncation warning even when the revision lookup fails, got none")
+	}
+	if strings.Contains(guard.warning, "revision 0") {
+		t.Errorf("warning names a nonexistent revision 0 (revision numbering starts at 1): %q", guard.warning)
+	}
+
+	out, err := h.handleUpdateTaskPlan(ctx, mcpPlanMsg(t, ws.ActionMCPUpdateTaskPlan,
+		mustMarshalPlanPayload(t, map[string]any{
+			"task_id": mcpPlanTaskID,
+			"content": small,
+		})))
+	if err != nil {
+		t.Fatalf("handleUpdateTaskPlan: %v", err)
+	}
+	updated := decodeMCPPlanPayload(t, out)
+
+	warning, _ := updated["plan_write_warning"].(string)
+	if warning == "" {
+		t.Fatal("expected a truncation warning even when the revision lookup fails, got none")
+	}
+	if strings.Contains(warning, "revision 0") {
+		t.Errorf("warning names a nonexistent revision 0 (revision numbering starts at 1): %q", warning)
+	}
+	if !strings.Contains(warning, "40000") || !strings.Contains(warning, "10000") {
+		t.Errorf("warning does not name the character drop (40000 -> 10000): %q", warning)
+	}
+
+	if _, ok := updated["prior_revision_number"]; ok {
+		t.Errorf("prior_revision_number should be omitted when the revision number is unknown, got %v",
+			updated["prior_revision_number"])
+	}
+}
+
+func TestMCPPlanTruncationGuard_PlanLookupFailurePreservesHistory(t *testing.T) {
+	h, repo := newMCPPlanTestHandlersWithFailingRevisionLookup(t)
+	ctx := context.Background()
+
+	large := strings.Repeat("x", 40000)
+	small := strings.Repeat("y", 10000)
+
+	_, err := h.handleCreateTaskPlan(ctx, mcpPlanMsg(t, ws.ActionMCPCreateTaskPlan,
+		mustMarshalPlanPayload(t, map[string]any{
+			"task_id": mcpPlanTaskID,
+			"content": large,
+		})))
+	if err != nil {
+		t.Fatalf("handleCreateTaskPlan: %v", err)
+	}
+
+	repo.failGetPlan = true
+
+	out, err := h.handleUpdateTaskPlan(ctx, mcpPlanMsg(t, ws.ActionMCPUpdateTaskPlan,
+		mustMarshalPlanPayload(t, map[string]any{
+			"task_id": mcpPlanTaskID,
+			"content": small,
+		})))
+	if err != nil {
+		t.Fatalf("handleUpdateTaskPlan: %v", err)
+	}
+	updated := decodeMCPPlanPayload(t, out)
+	if warning, ok := updated["plan_write_warning"]; ok {
+		t.Errorf("unexpected warning when the guard could not read the prior plan: %v", warning)
+	}
+
+	revisions, err := repo.Repository.ListTaskPlanRevisions(ctx, mcpPlanTaskID, 0)
+	if err != nil {
+		t.Fatalf("ListTaskPlanRevisions: %v", err)
+	}
+	if len(revisions) != 2 {
+		t.Fatalf("expected 2 revisions after a guarded read failure, got %d", len(revisions))
+	}
+	rev1 := revisionWithNumber(t, revisions, 1)
+	fullRev1, err := repo.GetTaskPlanRevision(ctx, rev1.ID)
+	if err != nil {
+		t.Fatalf("GetTaskPlanRevision(rev1): %v", err)
+	}
+	if fullRev1.Content != large {
+		t.Errorf("revision 1 content was mutated after a guarded read failure; got len=%d, want len=%d",
+			len(fullRev1.Content), len(large))
+	}
+}

apps/backend/internal/mcp/server/compact_results_test.go

@@ -81,6 +81,48 @@ func TestPlanAck_ReportsStoredSizeNotSentSize(t *testing.T) {
 	assert.Contains(t, text, "6 bytes")
 }
 
+// TestUpdateTaskPlan_SurfacesTruncationWarning pins that planWriteAck relays
+// a plan_write_warning field from the backend response into the tool result
+// text an agent actually reads. The ws handler is the layer that computes
+// this warning (internal/mcp/handlers/task_plan_guard.go); this test only
+// pins that the ack layer above it doesn't drop the field on the floor.
+func TestUpdateTaskPlan_SurfacesTruncationWarning(t *testing.T) {
+	backend := &testBackend{response: map[string]interface{}{
+		"task_id":               "task-A",
+		"title":                 "Plan",
+		"content":               "y",
+		"plan_write_warning":    "WARNING: this write replaced 40000 chars with 1 (dropped 39999 chars, 100%). Plan revision 3 has the prior content.",
+		"prior_revision_number": float64(3),
+	}}
+	s := newTaskModeServer(t, backend, "task-A")
+
+	text := planText(t, callTool(t, s, "update_task_plan_kandev", map[string]interface{}{
+		"content": "y",
+	}))
+
+	assert.Contains(t, text, "Plan updated successfully")
+	assert.Contains(t, text, "WARNING")
+	assert.Contains(t, text, "40000 chars with 1")
+	assert.Contains(t, text, "revision 3")
+}
+
+// TestUpdateTaskPlan_NoWarningWhenAbsent pins that the ack stays exactly the
+// pre-existing shape when the backend reports no truncation.
+func TestUpdateTaskPlan_NoWarningWhenAbsent(t *testing.T) {
+	backend := &testBackend{response: map[string]interface{}{
+		"task_id": "task-A",
+		"title":   "Plan",
+		"content": "step two",
+	}}
+	s := newTaskModeServer(t, backend, "task-A")
+
+	text := planText(t, callTool(t, s, "update_task_plan_kandev", map[string]interface{}{
+		"content": "step two",
+	}))
+
+	assert.NotContains(t, text, "WARNING")
+}
+
 func TestCreateTask_ResponseOmitsDescriptionEcho(t *testing.T) {
 	backend := &testBackend{response: map[string]interface{}{
 		"id":          "task-new",

apps/backend/internal/mcp/server/handlers.go

@@ -1028,8 +1028,11 @@ func planWriteAck(action string, result map[string]interface{}, sentContent stri
 	if updatedAt := stringField(result, "updated_at"); updatedAt != "" {
 		ack += ", updated_at=" + updatedAt
 	}
-	return mcp.NewToolResultText(ack +
-		". Plan content is omitted from this response; read it back with get_task_plan_kandev if needed.")
+	ack += ". Plan content is omitted from this response; read it back with get_task_plan_kandev if needed."
+	if warning := stringField(result, "plan_write_warning"); warning != "" {
+		ack += "\n\n" + warning
+	}
+	return mcp.NewToolResultText(ack)
 }
 
 func (s *Server) createTaskPlanHandler() server.ToolHandlerFunc {

apps/backend/internal/mcp/server/server.go

@@ -1609,9 +1609,9 @@ This tool is available only to autopilot child tasks. It sends a durable questio
 func (s *Server) registerPlanTools() {
 	s.mcpServer.AddTool(
 		mcp.NewTool("create_task_plan_kandev",
-			mcp.WithDescription("Create or save a task plan. task_id addresses the plan's task: pass your own task ID for your current task, or another task's ID to write that task's plan (allowed only within your reach — same workspace / task tree; a task outside it is rejected, never silently redirected to your own)."),
+			mcp.WithDescription("Create or save a task plan. task_id addresses the plan's task: pass your own task ID for your current task, or another task's ID to write that task's plan (allowed only within your reach — same workspace / task tree; a task outside it is rejected, never silently redirected to your own). If a plan already exists for this task, this REPLACES ITS ENTIRE CONTENT — same as update_task_plan_kandev through a different door. Read it first with get_task_plan_kandev if you need to preserve any of it."),
 			mcp.WithString("task_id", mcp.Description("The task ID to create a plan for. Defaults to your current task when omitted; pass another task's ID to target it directly.")),
-			mcp.WithString("content", mcp.Required(), mcp.Description("The plan content in markdown format")),
+			mcp.WithString("content", mcp.Required(), mcp.Description("The full plan content in markdown format. This REPLACES any existing plan whole — there is no partial update or append mode. To preserve prior content, call get_task_plan_kandev first and include its content plus your additions in this call.")),
 			mcp.WithString("title", mcp.Description("Optional title for the plan (default: 'Plan')")),
 		),
 		s.wrapHandler("create_task_plan_kandev", s.createTaskPlanHandler()),
@@ -1625,9 +1625,9 @@ func (s *Server) registerPlanTools() {
 	)
 	s.mcpServer.AddTool(
 		mcp.NewTool("update_task_plan_kandev",
-			mcp.WithDescription("Update an existing task plan. task_id selects the task whose plan to modify: your own task by default, or another task's ID to update that task's plan (allowed only within your reach — same workspace / task tree; a task outside it is rejected, never silently redirected to your own)."),
+			mcp.WithDescription("Update an existing task plan. task_id selects the task whose plan to modify: your own task by default, or another task's ID to update that task's plan (allowed only within your reach — same workspace / task tree; a task outside it is rejected, never silently redirected to your own). This REPLACES THE ENTIRE PLAN — there is no partial update, append, or section-patch mode. The correct sequence is: call get_task_plan_kandev, then send this call with the full document (prior content plus your changes), never just the new section."),
 			mcp.WithString("task_id", mcp.Description("The task ID to update the plan for. Defaults to your current task when omitted; pass another task's ID to target it directly.")),
-			mcp.WithString("content", mcp.Required(), mcp.Description("The updated plan content in markdown format")),
+			mcp.WithString("content", mcp.Required(), mcp.Description("The full plan content in markdown format that REPLACES the entire existing plan. Sending only a new section instead of the whole document will silently delete everything else. Read the current plan with get_task_plan_kandev first and include its content here plus your additions.")),
 			mcp.WithString("title", mcp.Description("Optional new title for the plan")),
 		),
 		s.wrapHandler("update_task_plan_kandev", s.updateTaskPlanHandler()),

apps/backend/internal/task/service/plan_service.go

@@ -101,6 +101,13 @@ type CreatePlanRequest struct {
 	CreatedBy  string // "agent" | "user"
 	AuthorKind string // optional explicit override
 	AuthorName string // optional; display snapshot
+	// ForceNewRevision skips coalescing for this write even if it would
+	// otherwise qualify (same author within the coalesce window). Callers set
+	// this when the write looks like an accidental whole-document truncation:
+	// coalescing would merge the destructive content into the prior revision
+	// row in place, destroying the only recovery path. Zero value (false)
+	// preserves today's coalescing behavior for every existing caller.
+	ForceNewRevision bool
 }
 
 // CreatePlan upserts a plan and appends or coalesces a revision.
@@ -113,12 +120,13 @@ func (s *PlanService) CreatePlan(ctx context.Context, req CreatePlanRequest) (*m
 
 // UpdatePlanRequest mirrors CreatePlanRequest; kept as a distinct type for API clarity.
 type UpdatePlanRequest struct {
-	TaskID     string
-	Title      string
-	Content    string
-	CreatedBy  string
-	AuthorKind string
-	AuthorName string
+	TaskID           string
+	Title            string
+	Content          string
+	CreatedBy        string
+	AuthorKind       string
+	AuthorName       string
+	ForceNewRevision bool
 }
 
 // UpdatePlan updates an existing plan (errors if missing).
@@ -145,12 +153,13 @@ func (s *PlanService) UpdatePlan(ctx context.Context, req UpdatePlanRequest) (*m
 		createdBy = existing.CreatedBy
 	}
 	return s.upsertPlan(ctx, CreatePlanRequest{
-		TaskID:     req.TaskID,
-		Title:      title,
-		Content:    req.Content,
-		CreatedBy:  createdBy,
-		AuthorKind: req.AuthorKind,
-		AuthorName: req.AuthorName,
+		TaskID:           req.TaskID,
+		Title:            title,
+		Content:          req.Content,
+		CreatedBy:        createdBy,
+		AuthorKind:       req.AuthorKind,
+		AuthorName:       req.AuthorName,
+		ForceNewRevision: req.ForceNewRevision,
 	})
 }
 
@@ -209,7 +218,7 @@ func (s *PlanService) upsertPlan(ctx context.Context, req CreatePlanRequest) (*m
 		return nil, err
 	}
 	now := time.Now().UTC()
-	coalesce := s.canCoalesce(latest, authorKind, authorName, now)
+	coalesce := !req.ForceNewRevision && s.canCoalesce(latest, authorKind, authorName, now)
 
 	rev := &models.TaskPlanRevision{
 		TaskID:     req.TaskID,

apps/backend/internal/task/service/plan_service_test.go

@@ -439,6 +439,40 @@ func TestPlanService_AppendsWhenWindowExpired(t *testing.T) {
 	}
 }
 
+func TestPlanService_ForceNewRevisionPreventsCoalesce(t *testing.T) {
+	svc, _, repo := createTestPlanService(t)
+	ctx := context.Background()
+	seedTask(t, ctx, repo, "task-force")
+	svc.coalesceWindow = 10 * time.Minute // same generous window as the coalesce test
+
+	_, err := svc.CreatePlan(ctx, CreatePlanRequest{
+		TaskID: "task-force", Content: "v1",
+		AuthorKind: "agent", AuthorName: "Claude",
+	})
+	if err != nil {
+		t.Fatalf("initial create: %v", err)
+	}
+	// Same author, same window — would coalesce by default (see
+	// TestPlanService_CoalescesWithinWindow) but must not when a truncating
+	// write forces a new revision so the pre-write content survives.
+	_, err = svc.UpdatePlan(ctx, UpdatePlanRequest{
+		TaskID: "task-force", Content: "v2",
+		AuthorKind: "agent", AuthorName: "Claude",
+		ForceNewRevision: true,
+	})
+	if err != nil {
+		t.Fatalf("forced update: %v", err)
+	}
+
+	list, err := svc.ListRevisions(ctx, "task-force")
+	if err != nil {
+		t.Fatalf("ListRevisions: %v", err)
+	}
+	if len(list) != 2 {
+		t.Fatalf("expected 2 separate revisions (forced), got %d", len(list))
+	}
+}
+
 func TestPlanService_AuthorSwitchBreaksCoalesce(t *testing.T) {
 	svc, _, repo := createTestPlanService(t)
 	ctx := context.Background()