fix: make walkthrough MCP failures actionable (#2209)

2026-08-03 16:34:04 +01:00
parent d014fd56fe
commit c95d44193c
14 changed files with 684 additions and 7 deletions
@@ -258,6 +258,8 @@ Every long-running goroutine must have a single owner with explicit start and st

 You may still list the column in the `CREATE TABLE` so fresh DBs get it inline, but the migration is the source of truth for evolution and must stand alone. New columns also need: the struct field in `models/`, the DTO field + `ToAPI` in `pkg/api/v1/`, and every `CreateX`/`UpdateX`/bulk write in the repo that should set it.

+Built-in prompt content refreshes are seed-data migrations, not schema migrations. Match only known historical content hashes after applying the same normalization as the embedded prompt loader, require `created_at == updated_at` to preserve user edits, and use a conditional update over the original row values to avoid racing concurrent edits. Keep these refreshes with prompt seeding rather than `runMigrations()`.
+
 ## Code-quality limits

 Enforced by `apps/backend/.golangci.yml` (errors on new code only):
@@ -1,5 +1,10 @@
 Please create an agent-authored walkthrough of the current changes using `show_walkthrough_kandev`.

+Required tool shape:
+- `steps` is an ordered array.
+- Every step requires `file` (repository-relative path), `line` (positive 1-based integer), and `text` (markdown explanation).
+- Optional step fields are `repo`, `title`, and `line_end`.
+
 Walkthrough requirements:
 - Inspect the changed files yourself instead of relying on UI-provided paths or diff context.
 - If this is a PR task or PR review task, compare the PR head against the PR base branch.
@@ -11,6 +11,9 @@ Available tools:
 - get_task_plan_kandev: Retrieve the current plan for a task (includes any user edits). Required params: task_id.
 - update_task_plan_kandev: Update an existing plan. Required params: task_id, content (markdown). Optional: title.
 - delete_task_plan_kandev: Delete a task plan. Required params: task_id.
+- show_walkthrough_kandev: Show and store a code walkthrough for the current task. Required param: `steps` (ordered array; every step requires `file`, `line`, and `text`). Optional: task_id, title; each step may include repo, title, and line_end.
+- get_walkthrough_kandev: Retrieve the stored walkthrough for a task. Optional: task_id (defaults to the current task).
+- delete_walkthrough_kandev: Delete the stored walkthrough for a task. Optional: task_id (defaults to the current task).
 - list_workspaces_kandev: List all workspaces.
 - list_workflows_kandev: List workflows in a workspace. Required params: workspace_id.
 - list_tasks_kandev: List tasks in a workflow. Required params: workflow_id.
@@ -8,6 +8,7 @@ import (
 	"strings"
 	"testing"

+	promptcfg "github.com/kandev/kandev/config/prompts"
 	"github.com/kandev/kandev/internal/sysprompt"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
@@ -425,3 +426,71 @@ func TestAskUserQuestionDocs_MatchSchema(t *testing.T) {
 		}
 	}
 }
+
+func extractWalkthroughStepRequiredFields(t *testing.T, s *Server) []string {
+	t.Helper()
+
+	tool, ok := s.mcpServer.ListTools()["show_walkthrough_kandev"]
+	require.True(t, ok, "show_walkthrough_kandev not registered on this server")
+
+	raw, err := json.Marshal(tool.Tool.InputSchema)
+	require.NoError(t, err)
+
+	var parsed map[string]any
+	require.NoError(t, json.Unmarshal(raw, &parsed))
+	properties, ok := parsed["properties"].(map[string]any)
+	require.True(t, ok, "walkthrough schema must expose properties")
+	topLevelRequired, ok := parsed["required"].([]any)
+	require.True(t, ok, "walkthrough schema must declare top-level required fields")
+	require.Contains(t, topLevelRequired, "steps")
+	steps, ok := properties["steps"].(map[string]any)
+	require.True(t, ok, "walkthrough schema must expose steps")
+	require.Equal(t, "array", steps["type"])
+	items, ok := steps["items"].(map[string]any)
+	require.True(t, ok, "walkthrough steps must declare an item schema")
+	requiredRaw, ok := items["required"].([]any)
+	require.True(t, ok, "walkthrough step schema must declare required fields")
+
+	required := make([]string, 0, len(requiredRaw))
+	for _, value := range requiredRaw {
+		field, ok := value.(string)
+		require.True(t, ok, "walkthrough required field must be a string")
+		required = append(required, field)
+	}
+	sort.Strings(required)
+	return required
+}
+
+func TestWalkthroughDocs_MatchSchema(t *testing.T) {
+	log := newTestLogger(t)
+	backend := NewChannelBackendClient(log)
+	t.Cleanup(backend.Close)
+
+	s := New(backend, "test-session", "test-task", 10005, log, "", false, ModeTask)
+	require.NotNil(t, s)
+	requiredFields := extractWalkthroughStepRequiredFields(t, s)
+
+	context := sysprompt.KandevContext()
+	for _, toolName := range []string{
+		"show_walkthrough_kandev",
+		"get_walkthrough_kandev",
+		"delete_walkthrough_kandev",
+	} {
+		assert.Contains(t, context, toolName,
+			"KandevContext must advertise task-mode walkthrough tool %q", toolName)
+	}
+
+	for name, prompt := range map[string]string{
+		"KandevContext":      context,
+		"ChangesWalkthrough": promptcfg.Get("changes-walkthrough"),
+	} {
+		assert.Contains(t, prompt, "`steps`",
+			"%s must identify the walkthrough steps parameter", name)
+		assert.Contains(t, prompt, "ordered array",
+			"%s must say walkthrough steps preserve order", name)
+		for _, field := range requiredFields {
+			assert.Contains(t, prompt, fmt.Sprintf("`%s`", field),
+				"%s must identify required walkthrough step field %q", name, field)
+		}
+	}
+}
@@ -5,10 +5,13 @@ import (
 	"errors"
 	"fmt"
 	"net/url"
+	"sort"
+	"strconv"
 	"strings"

 	"github.com/mark3labs/mcp-go/mcp"
 	jsonschema "github.com/santhosh-tekuri/jsonschema/v6"
+	"github.com/santhosh-tekuri/jsonschema/v6/kind"
 	"go.uber.org/zap"
 )

@@ -108,8 +111,22 @@ func sanitizedToolArgumentError(toolName string, err error) error {
 		failure = validationErr
 		keyword = "schema"
 	}
-	return fmt.Errorf("invalid arguments for %s: validation failed at %s (keyword: %s)",
-		toolName, validationInstancePath(failure.InstanceLocation), keyword)
+	return fmt.Errorf("invalid arguments for %s: validation failed at %s (keyword: %s%s)",
+		toolName, validationInstancePath(failure.InstanceLocation), keyword, missingRequiredProperties(failure))
+}
+
+func missingRequiredProperties(err *jsonschema.ValidationError) string {
+	required, ok := err.ErrorKind.(*kind.Required)
+	if !ok || len(required.Missing) == 0 {
+		return ""
+	}
+
+	missing := make([]string, len(required.Missing))
+	for i, property := range required.Missing {
+		missing[i] = strconv.Quote(property)
+	}
+	sort.Strings(missing)
+	return "; missing: " + strings.Join(missing, ", ")
 }

 func firstKeywordFailure(err *jsonschema.ValidationError) (*jsonschema.ValidationError, string) {
@@ -33,6 +33,81 @@ func TestToolArgumentValidationRejectsUnknownTopLevelArgument(t *testing.T) {
 		content.Text)
 }

+func TestToolArgumentValidationNamesMissingWalkthroughStepProperty(t *testing.T) {
+	const rejectedValue = "private sibling value"
+	backend := &testBackend{}
+	s := newTaskModeServer(t, backend, "task-current")
+
+	result := callTool(t, s, "show_walkthrough_kandev", map[string]interface{}{
+		"steps": []interface{}{
+			map[string]interface{}{
+				"file":  "example.go",
+				"line":  1,
+				"title": rejectedValue,
+			},
+		},
+	})
+
+	assert.True(t, result.IsError)
+	assert.Empty(t, backend.lastAction)
+	require.NotEmpty(t, result.Content)
+	content, ok := result.Content[0].(mcp.TextContent)
+	require.True(t, ok)
+	assert.Contains(t, content.Text, "/steps/0")
+	assert.Contains(t, content.Text, "keyword: required")
+	assert.Contains(t, content.Text, `missing: "text"`)
+	assert.NotContains(t, content.Text, rejectedValue)
+}
+
+func TestToolArgumentValidationNamesEveryMissingWalkthroughStepProperty(t *testing.T) {
+	backend := &testBackend{}
+	s := newTaskModeServer(t, backend, "task-current")
+
+	result := callTool(t, s, "show_walkthrough_kandev", map[string]interface{}{
+		"steps": []interface{}{
+			map[string]interface{}{"line": 1},
+		},
+	})
+
+	assert.True(t, result.IsError)
+	assert.Empty(t, backend.lastAction)
+	require.NotEmpty(t, result.Content)
+	content, ok := result.Content[0].(mcp.TextContent)
+	require.True(t, ok)
+	assert.Contains(t, content.Text, `missing: "file", "text"`)
+}
+
+func TestToolArgumentValidationSortsMissingProperties(t *testing.T) {
+	const toolName = "unordered_required_properties_tool"
+	backend := &testBackend{}
+	s := newTaskModeServer(t, backend, "task-current")
+	s.mcpServer.AddTool(
+		mcp.NewToolWithRawSchema(
+			toolName,
+			"Validates deterministic missing-property diagnostics.",
+			json.RawMessage(`{
+				"type": "object",
+				"properties": {
+					"alpha": {"type": "string"},
+					"zeta": {"type": "string"}
+				},
+				"required": ["zeta", "alpha"]
+			}`),
+		),
+		s.wrapHandler(toolName, s.listWorkspacesHandler()),
+	)
+	s.rebuildToolArgumentValidators()
+
+	result := callTool(t, s, toolName, map[string]interface{}{})
+
+	assert.True(t, result.IsError)
+	assert.Empty(t, backend.lastAction)
+	require.NotEmpty(t, result.Content)
+	content, ok := result.Content[0].(mcp.TextContent)
+	require.True(t, ok)
+	assert.Contains(t, content.Text, `missing: "alpha", "zeta"`)
+}
+
 func TestToolArgumentValidation(t *testing.T) {
 	t.Run("accepts an empty object for a parameterless tool", func(t *testing.T) {
 		backend := &testBackend{
@@ -2,6 +2,8 @@ package store

 import (
 	"context"
+	"crypto/sha256"
+	"database/sql"
 	"errors"
 	"fmt"
 	"strings"
@@ -14,6 +16,13 @@ import (
 	"github.com/kandev/kandev/internal/prompts/models"
 )

+const (
+	builtinChangesWalkthroughPromptID = "builtin-changes-walkthrough"
+	// Historical prompt hashes use the same TrimSpace normalization as promptcfg.Get.
+	changesWalkthroughV1SHA256 = "23a82694ef3b6d0220da2879c1c351cf5ee4926c2bc54a52fa4f7d5182bcb111"
+	changesWalkthroughV2SHA256 = "7a28dc81df4bff75b4fb8d66d6b9118febe5daf7f4b570e3b8ef8c74ac3e3146"
+)
+
 type sqliteRepository struct {
 	db     *sqlx.DB // writer
 	ro     *sqlx.DB // reader
@@ -181,10 +190,58 @@ func (r *sqliteRepository) seedBuiltinPrompts() error {
 		if err != nil {
 			return fmt.Errorf("failed to upsert built-in prompt %s: %w", prompt.ID, err)
 		}
+		if prompt.ID == builtinChangesWalkthroughPromptID {
+			if err := r.refreshLegacyChangesWalkthroughPrompt(prompt); err != nil {
+				return err
+			}
+		}
 	}
 	return nil
 }

+// refreshLegacyChangesWalkthroughPrompt updates exact, untouched revisions
+// shipped before the required step shape was documented. User edits change
+// updated_at, and the conditional update prevents racing a concurrent edit.
+func (r *sqliteRepository) refreshLegacyChangesWalkthroughPrompt(current *models.Prompt) error {
+	var storedContent string
+	var builtinInt int
+	var createdAt, updatedAt time.Time
+	err := r.db.QueryRow(r.db.Rebind(`
+		SELECT content, builtin, created_at, updated_at
+		FROM custom_prompts
+		WHERE id = ?
+	`), current.ID).Scan(&storedContent, &builtinInt, &createdAt, &updatedAt)
+	if errors.Is(err, sql.ErrNoRows) {
+		return nil
+	}
+	if err != nil {
+		return fmt.Errorf("read stored changes walkthrough prompt: %w", err)
+	}
+	if builtinInt != 1 || !createdAt.Equal(updatedAt) || !isLegacyChangesWalkthroughPrompt(storedContent) {
+		return nil
+	}
+
+	_, err = r.db.Exec(r.db.Rebind(`
+		UPDATE custom_prompts
+		SET content = ?
+		WHERE id = ? AND builtin = 1 AND content = ? AND created_at = ? AND updated_at = ?
+	`), current.Content, current.ID, storedContent, createdAt, updatedAt)
+	if err != nil {
+		return fmt.Errorf("refresh stored changes walkthrough prompt: %w", err)
+	}
+	return nil
+}
+
+func isLegacyChangesWalkthroughPrompt(content string) bool {
+	hash := fmt.Sprintf("%x", sha256.Sum256([]byte(content)))
+	switch hash {
+	case changesWalkthroughV1SHA256, changesWalkthroughV2SHA256:
+		return true
+	default:
+		return false
+	}
+}
+
 // getBuiltinPrompts returns the predefined built-in prompts loaded from embedded markdown files.
 func (r *sqliteRepository) getBuiltinPrompts() []*models.Prompt {
 	now := time.Now().UTC()
@@ -193,6 +250,6 @@ func (r *sqliteRepository) getBuiltinPrompts() []*models.Prompt {
 		{ID: "builtin-open-pr", Name: "open-pr", Builtin: true, CreatedAt: now, UpdatedAt: now, Content: promptcfg.Get("open-pr")},
 		{ID: "builtin-merge-base", Name: "merge-base", Builtin: true, CreatedAt: now, UpdatedAt: now, Content: promptcfg.Get("merge-base")},
 		{ID: "builtin-ci-auto-fix", Name: "ci-auto-fix", Builtin: true, CreatedAt: now, UpdatedAt: now, Content: promptcfg.Get("ci-auto-fix")},
-		{ID: "builtin-changes-walkthrough", Name: "changes-walkthrough", Builtin: true, CreatedAt: now, UpdatedAt: now, Content: promptcfg.Get("changes-walkthrough")},
+		{ID: builtinChangesWalkthroughPromptID, Name: "changes-walkthrough", Builtin: true, CreatedAt: now, UpdatedAt: now, Content: promptcfg.Get("changes-walkthrough")},
 	}
 }
@@ -2,12 +2,14 @@ package store

 import (
 	"context"
+	"os"
 	"path/filepath"
 	"strings"
 	"testing"
 	"time"

 	"github.com/jmoiron/sqlx"
+	promptcfg "github.com/kandev/kandev/config/prompts"
 	"github.com/kandev/kandev/internal/db"
 	"github.com/kandev/kandev/internal/prompts/models"
 )
@@ -35,6 +37,52 @@ func createTestRepo(t *testing.T) (*sqliteRepository, func()) {
 	return repo, cleanup
 }

+func createUnseededPromptDB(t *testing.T) *sqlx.DB {
+	t.Helper()
+	dbConn, err := db.OpenSQLite(filepath.Join(t.TempDir(), "test.db"))
+	if err != nil {
+		t.Fatalf("failed to open sqlite db: %v", err)
+	}
+	sqlxDB := sqlx.NewDb(dbConn, "sqlite3")
+	t.Cleanup(func() {
+		if err := sqlxDB.Close(); err != nil {
+			t.Errorf("failed to close sqlite db: %v", err)
+		}
+	})
+	if _, err := sqlxDB.Exec(`
+		CREATE TABLE custom_prompts (
+			id TEXT PRIMARY KEY,
+			name TEXT NOT NULL UNIQUE,
+			content TEXT NOT NULL,
+			builtin INTEGER NOT NULL DEFAULT 0,
+			created_at TIMESTAMP NOT NULL,
+			updated_at TIMESTAMP NOT NULL
+		);
+	`); err != nil {
+		t.Fatalf("create schema: %v", err)
+	}
+	return sqlxDB
+}
+
+func seedStoredWalkthroughPrompt(t *testing.T, sqlxDB *sqlx.DB, content string, createdAt, updatedAt time.Time) {
+	t.Helper()
+	if _, err := sqlxDB.Exec(
+		`INSERT INTO custom_prompts (id, name, content, builtin, created_at, updated_at) VALUES (?, ?, ?, 1, ?, ?)`,
+		"builtin-changes-walkthrough", "changes-walkthrough", content, createdAt, updatedAt,
+	); err != nil {
+		t.Fatalf("seed stored walkthrough prompt: %v", err)
+	}
+}
+
+func readPromptFixture(t *testing.T, name string) string {
+	t.Helper()
+	content, err := os.ReadFile(filepath.Join("testdata", name))
+	if err != nil {
+		t.Fatalf("read prompt fixture: %v", err)
+	}
+	return string(content)
+}
+
 func TestSQLiteRepository_CRUD(t *testing.T) {
 	repo, cleanup := createTestRepo(t)
 	defer cleanup()
@@ -222,3 +270,67 @@ func TestSQLiteRepository_BuiltinSeedIgnoresUserPromptNameConflict(t *testing.T)
 		t.Fatalf("expected user prompt to remain canonical, got %+v", got)
 	}
 }
+
+func TestSQLiteRepository_RefreshesUnmodifiedLegacyChangesWalkthroughPrompt(t *testing.T) {
+	for _, fixture := range []string{
+		"changes-walkthrough-v1.md",
+		"changes-walkthrough-v2.md",
+	} {
+		t.Run(fixture, func(t *testing.T) {
+			sqlxDB := createUnseededPromptDB(t)
+			legacyContent := strings.TrimSpace(readPromptFixture(t, fixture))
+			now := time.Now().UTC()
+			seedStoredWalkthroughPrompt(t, sqlxDB, legacyContent, now, now)
+
+			repo, err := newSQLiteRepositoryWithDB(sqlxDB, sqlxDB)
+			if err != nil {
+				t.Fatalf("initialize repository: %v", err)
+			}
+			got, err := repo.GetPromptByName(context.Background(), "changes-walkthrough")
+			if err != nil {
+				t.Fatalf("get refreshed prompt: %v", err)
+			}
+			if got.Content != promptcfg.Get("changes-walkthrough") {
+				t.Fatalf("stored legacy prompt was not refreshed")
+			}
+		})
+	}
+}
+
+func TestSQLiteRepository_PreservesEditedLegacyChangesWalkthroughPrompt(t *testing.T) {
+	sqlxDB := createUnseededPromptDB(t)
+	legacyContent := readPromptFixture(t, "changes-walkthrough-v2.md")
+	createdAt := time.Now().UTC().Add(-time.Hour)
+	seedStoredWalkthroughPrompt(t, sqlxDB, legacyContent, createdAt, createdAt.Add(time.Minute))
+
+	repo, err := newSQLiteRepositoryWithDB(sqlxDB, sqlxDB)
+	if err != nil {
+		t.Fatalf("initialize repository: %v", err)
+	}
+	got, err := repo.GetPromptByName(context.Background(), "changes-walkthrough")
+	if err != nil {
+		t.Fatalf("get edited prompt: %v", err)
+	}
+	if got.Content != legacyContent {
+		t.Fatalf("user-edited built-in prompt was overwritten")
+	}
+}
+
+func TestSQLiteRepository_PreservesUntouchedUnrecognizedChangesWalkthroughPrompt(t *testing.T) {
+	sqlxDB := createUnseededPromptDB(t)
+	customContent := "this is not a recognized legacy revision"
+	now := time.Now().UTC()
+	seedStoredWalkthroughPrompt(t, sqlxDB, customContent, now, now)
+
+	repo, err := newSQLiteRepositoryWithDB(sqlxDB, sqlxDB)
+	if err != nil {
+		t.Fatalf("initialize repository: %v", err)
+	}
+	got, err := repo.GetPromptByName(context.Background(), "changes-walkthrough")
+	if err != nil {
+		t.Fatalf("get unrecognized prompt: %v", err)
+	}
+	if got.Content != customContent {
+		t.Fatalf("unrecognized untouched content was overwritten")
+	}
+}
@@ -0,0 +1,12 @@
+Please create an agent-authored walkthrough of the current changes using `show_walkthrough_kandev`.
+
+Walkthrough requirements:
+- Use only files listed below or files you verify exist in this task's local worktree/review diff.
+- For PR-only files, do not assume the PR head is checked out locally; anchor to the review diff when available, and avoid editor-only/current-worktree claims.
+- Anchor steps to changed lines or changed line ranges whenever possible.
+- Use `line_end` whenever a logical explanation spans multiple lines; prefer one range step over adjacent single-line steps.
+- Keep each step concise and direct. Do not include a `Justification:` preamble.
+- If a good local/review anchor is unavailable, omit that step instead of referencing a remote-only path.
+
+Available changed files:
+{{changed_files}}
@@ -0,0 +1,14 @@
+Please create an agent-authored walkthrough of the current changes using `show_walkthrough_kandev`.
+
+Walkthrough requirements:
+- Inspect the changed files yourself instead of relying on UI-provided paths or diff context.
+- If this is a PR task or PR review task, compare the PR head against the PR base branch.
+- Otherwise, compare against the task/repository base using the local task state (`git status`, `git diff`, `git diff --cached`, and cumulative diff context when available).
+- For PR-only files, do not assume the PR head is checked out locally; anchor to the review diff when available, and avoid editor-only/current-worktree claims.
+- The first walkthrough step must contextualize the whole change before explaining individual hunks.
+- Anchor the first walkthrough step to the most representative changed line or changed line range available.
+- Include an `ELI5:` line in the first walkthrough step text that explains the change in the simplest terms possible.
+- Anchor steps to changed lines or changed line ranges whenever possible.
+- Use `line_end` whenever a logical explanation spans multiple lines; prefer one range step over adjacent single-line steps.
+- Keep each step concise and direct. Do not include a `Justification:` preamble.
+- If a good local/review anchor is unavailable, omit that step instead of referencing a remote-only path.
@@ -0,0 +1,175 @@
+---
+spec: docs/specs/integrations/mcp-tool-argument-validation.md
+created: 2026-08-03
+status: done
+---
+
+# Fix Plan: Recover Invalid Walkthrough MCP Calls
+
+## Overview
+
+Make shared MCP validation errors actionable without weakening their redaction,
+then put the load-bearing walkthrough call shape in both task context and the
+built-in walkthrough request. Bind those instructions to the registered schema
+with focused tests and update the public MCP reference.
+
+## Confirmed root cause
+
+`sanitizedToolArgumentError` reduces a JSON Schema `required` failure to its
+instance path and keyword. A walkthrough step missing `text` therefore returns
+`validation failed at /steps/0 (keyword: required)` and never reaches the
+walkthrough service's more specific `step 1 text is required` validation. The
+shared validator landed on 2026-08-01 and now intercepts this malformed call
+before the tool-specific handler.
+
+The registered `show_walkthrough_kandev` schema correctly requires `file`,
+`line`, and `text`, but the injected task context omits all walkthrough tools
+and the built-in `changes-walkthrough` prompt never states the argument shape.
+Clients or models that do not preserve nested schema detail therefore receive
+weak initial guidance followed by an error that does not identify the missing
+field.
+
+Smallest reproduction: call `show_walkthrough_kandev` with
+`steps: [{"file":"example.go","line":1,"title":"Explanation"}]`. The
+advertised schema contains `required: ["file", "line", "text"]`, but the
+returned error does not contain `text` and no backend action runs.
+
+## Backend
+
+### Actionable required-property errors
+
+- `apps/backend/internal/mcp/server/tool_argument_validation.go` — recognize
+  the validator library's `kind.Required` error and append its schema-defined
+  missing property names to the sanitized result. Preserve the current path and
+  keyword for compatibility and retain the existing generic formatting for all
+  other constraints.
+- Never format a generic validator message or rejected value. Only the missing
+  names declared by the registered schema may enter the error.
+- Keep fail-closed validation, root closure, open nested maps, mode rebuilds,
+  handler suppression, and create-task compatibility normalization unchanged.
+
+### Existing-install prompt refresh
+
+- `apps/backend/internal/prompts/store/sqlite.go` — refresh only the exact
+  loader-normalized stored content of the two historical `changes-walkthrough`
+  built-in revisions that predate the required step shape and have never been
+  saved by a user.
+- Preserve unrecognized content, edited built-ins, and user-owned name
+  conflicts. Keep the conditional write race-safe so an edit concurrent with
+  startup cannot be overwritten.
+
+## Agent prompt contract
+
+- `apps/backend/config/prompts/kandev-context.md` — list
+  `show_walkthrough_kandev`, `get_walkthrough_kandev`, and
+  `delete_walkthrough_kandev`. State that `show` requires an ordered `steps`
+  array and every step requires `file`, `line`, and `text`; keep optional
+  top-level and step fields concise.
+- `apps/backend/config/prompts/changes-walkthrough.md` — repeat the exact
+  required step shape next to the existing generation requirements. Do not add
+  task-specific paths, diff contents, or a copied sample that an agent could
+  submit literally.
+
+## Public documentation
+
+Update the MCP validation contract in
+`docs/public/automation-and-mcp.md` to state that `required` failures name
+missing schema properties while never echoing argument values. The existing
+walkthrough guide already documents that each step needs text, a file, and a
+positive line, so it needs no behavior change.
+
+Primary content type: reference.
+
+## Tests
+
+- **What:** A nested missing-required-property failure names `text`, retains
+  `/steps/0` and `required`, does not expose sibling values, and dispatches no
+  backend action.
+  **File:**
+  `apps/backend/internal/mcp/server/tool_argument_validation_test.go`.
+  **How:** Call the real registered `show_walkthrough_kandev` tool through
+  `wrapHandler` with the minimal malformed step. Write this regression first
+  and confirm it fails because the current result omits `text`.
+- **What:** Walkthrough instructions stay aligned with the live tool schema.
+  **File:** `apps/backend/internal/mcp/server/sysprompt_sync_test.go`.
+  **How:** Extract the `steps` item required fields from the registered tool,
+  assert the first-turn task context names all three walkthrough tools, and
+  assert both task context and embedded `changes-walkthrough` content mention
+  `steps` plus every required item field.
+- **What:** Public documentation remains structurally valid.
+  **File:** `docs/public/automation-and-mcp.md`.
+  **How:** Run both public-doc validation scripts.
+- **What:** Existing installations receive the corrected walkthrough request
+  without losing prompt customizations.
+  **File:** `apps/backend/internal/prompts/store/sqlite_test.go` plus historical
+  fixtures under `apps/backend/internal/prompts/store/testdata/`.
+  **How:** Initialize repositories over each shipped legacy revision in the
+  exact trimmed form stored by `promptcfg.Get` and require refresh, then prove
+  unrecognized content, a saved built-in, and a user-owned name conflict remain
+  unchanged.
+
+No browser E2E is needed: this changes agent-facing MCP diagnostics and hidden
+prompt instructions, not rendered UI behavior.
+
+## Verification Results
+
+Completed on 2026-08-03 with strict red-green coverage:
+
+- Focused RED tests failed because required-property diagnostics omitted
+  `text`, task context omitted the walkthrough tools, and neither prompt surface
+  documented the step shape.
+- The same focused tests passed after implementation, including coverage for
+  multiple missing properties and submitted-value redaction.
+- `cd apps/backend && go test ./internal/mcp/server ./internal/sysprompt` —
+  passed.
+- `node --test scripts/validate-public-docs.test.mjs` — passed, 58 tests.
+- `node scripts/validate-public-docs.mjs` — passed, 41 published pages.
+- `git diff --check` — passed.
+
+PR review remediation also passed:
+
+- Focused RED tests reproduced stale stored prompts and declaration-ordered
+  missing-property output.
+- Focused GREEN tests passed for both shipped legacy prompt revisions, edited
+  prompt preservation, canonical property sorting, the top-level `steps`
+  contract, and immediate test cleanup.
+- `cd apps/backend && go test ./internal/prompts/store ./internal/mcp/server ./internal/sysprompt` — passed.
+- `cd apps/backend && golangci-lint run ./... --new-from-rev="0ca00c2aca4430a5e4f06874ba960743de1acf9a" --timeout=5m` — passed with 0 issues.
+
+Follow-up review remediation also passed:
+
+- Focused RED coverage seeded both historical revisions in the exact trimmed
+  form produced by `promptcfg.Get`; both remained stale with the fixture-file
+  hashes.
+- Focused GREEN coverage passed after hashing the loader-normalized historical
+  contents. Equal-timestamp unrecognized content and edited content remain
+  unchanged.
+- `cd apps/backend && go test ./internal/prompts/store ./internal/mcp/server ./internal/sysprompt` — passed.
+- `cd apps/backend && golangci-lint run ./... --new-from-rev="0ca00c2aca4430a5e4f06874ba960743de1acf9a" --timeout=5m` — passed with 0 issues.
+- `node --test scripts/validate-public-docs.test.mjs` — passed, 58 tests.
+- `node scripts/validate-public-docs.mjs` — passed, 41 published pages.
+- `git diff --check` — passed.
+
+No browser E2E was run because the change affects MCP diagnostics and embedded
+agent prompts, not rendered UI behavior.
+
+## Implementation Wave
+
+- [x] [Task 01: Make walkthrough call recovery actionable](task-01-actionable-walkthrough-call-recovery.md) — done.
+
+Execution stays sequential in the primary conversation. No subagents are
+authorized.
+
+## Risks and out of scope
+
+- Calling a generic localized validator formatter could expose rejected values;
+  inspect only the typed `required` error's missing schema names.
+- Keep existing non-`required` error text stable to avoid broadening this fix.
+- Do not alter the walkthrough schema, persistence schema, unrelated
+  persistence behavior, rendering, MCP transport, authorization, or retry
+  behavior.
+- Task context receives only the walkthrough contract needed for reliable calls;
+  this work does not mirror all registered tool schemas into that prompt.
+- Exact offending ACP frames were not available, so this repair hardens both
+  documented contributors without claiming whether one client dropped `text`
+  or one model omitted it.
@@ -0,0 +1,127 @@
+---
+id: "mcp-walkthrough-call-recovery-01"
+title: "Make walkthrough call recovery actionable"
+status: done
+wave: 1
+depends_on: []
+plan: "plan.md"
+spec: "../../specs/integrations/mcp-tool-argument-validation.md"
+---
+
+# Task 01: Make Walkthrough Call Recovery Actionable
+
+## Acceptance
+
+1. A `show_walkthrough_kandev` call whose step omits `text` fails before
+   backend dispatch and names `/steps/0`, the `required` constraint, and the
+   missing `text` schema property without exposing any submitted value.
+2. First-turn task context names show/get/delete walkthrough tools, and both it
+   and the built-in walkthrough request state that every `steps` item requires
+   `file`, `line`, and `text`, with tests derived from the registered schema.
+3. Public MCP reference documents missing-property diagnostics; all focused Go
+   and public-doc checks pass.
+4. Existing installations refresh untouched historical walkthrough prompts in
+   their loader-normalized stored form to the corrected embedded content
+   without overwriting unrecognized content, user edits, or user-owned prompt
+   conflicts.
+
+## Verification
+
+Follow strict TDD, then run:
+
+```bash
+cd apps/backend
+go test ./internal/prompts/store ./internal/mcp/server ./internal/sysprompt
+cd ../..
+node --test scripts/validate-public-docs.test.mjs
+node scripts/validate-public-docs.mjs
+git diff --check
+```
+
+## Files likely touched
+
+- `apps/backend/internal/mcp/server/tool_argument_validation.go`
+- `apps/backend/internal/mcp/server/tool_argument_validation_test.go`
+- `apps/backend/internal/mcp/server/sysprompt_sync_test.go`
+- `apps/backend/internal/prompts/store/sqlite.go`
+- `apps/backend/internal/prompts/store/sqlite_test.go`
+- `apps/backend/internal/prompts/store/testdata/`
+- `apps/backend/config/prompts/kandev-context.md`
+- `apps/backend/config/prompts/changes-walkthrough.md`
+- `docs/public/automation-and-mcp.md`
+- `docs/specs/integrations/mcp-tool-argument-validation.md`
+- `docs/specs/INDEX.md`
+- `docs/plans/mcp-walkthrough-call-recovery/plan.md`
+- `docs/plans/mcp-walkthrough-call-recovery/task-01-actionable-walkthrough-call-recovery.md`
+
+## Dependencies
+
+None.
+
+## Parallelism
+
+`sequential` — validator behavior, schema-derived prompt coverage, and contract
+documentation form one regression repair.
+
+## Inputs
+
+- [MCP Tool Argument Validation spec](../../specs/integrations/mcp-tool-argument-validation.md)
+- [Fix plan](plan.md)
+- `docs/decisions/2026-08-01-validate-mcp-tool-arguments.md`
+- `sanitizedToolArgumentError`, `firstKeywordFailure`, and
+  `buildWalkthroughStepSchemaItem`
+- `TestAskUserQuestionDocs_MatchSchema` as schema-to-prompt consistency pattern
+- Built-in `changes-walkthrough` prompt and task-mode Kandev context
+
+## Risks
+
+- Do not expose rejected argument values through a generic validator message.
+- Preserve error formatting for every non-`required` validation keyword.
+- Do not change nested-map openness or add client-specific retries.
+
+## Output contract
+
+Report red/green test evidence, exact error shape, prompt and public-doc files,
+all command results, residual risks, and synchronized task/plan/spec statuses.
+
+## Results
+
+Completed on 2026-08-03.
+
+- RED: focused server tests failed on the old behavior because the validation
+  error did not name missing `text`, task context did not advertise the
+  walkthrough tools, and the embedded prompts did not state the required item
+  shape.
+- GREEN: required failures now retain the object path and keyword while adding
+  quoted schema property names, for example
+  `validation failed at /steps/0 (keyword: required; missing: "text")`.
+  Submitted argument values remain absent and invalid calls still do not reach
+  backend dispatch.
+- Task context now lists show/get/delete walkthrough tools. It and the built-in
+  walkthrough request both describe `steps` as an ordered array whose items
+  require `file`, `line`, and `text`; a schema-derived regression test guards
+  this contract.
+- Public MCP reference now documents the actionable, redacted missing-property
+  behavior.
+- `cd apps/backend && go test ./internal/mcp/server ./internal/sysprompt` —
+  passed.
+- `node --test scripts/validate-public-docs.test.mjs` — passed, 58 tests.
+- `node scripts/validate-public-docs.mjs` — passed, 41 published pages.
+- `git diff --check` — passed.
+- Browser E2E was intentionally not run because no rendered UI behavior
+  changed.
+- PR review remediation refreshes the exact loader-normalized stored content of
+  the two untouched historical prompt revisions, preserves unrecognized
+  content, edited built-ins, and user-owned conflicts, sorts missing schema
+  properties deterministically, asserts top-level `steps` requiredness, and
+  registers backend cleanup with `t.Cleanup`.
+- Remediation RED tests reproduced both stale stored revisions and unsorted
+  output; the focused GREEN regressions passed afterward.
+- `cd apps/backend && go test ./internal/prompts/store ./internal/mcp/server ./internal/sysprompt` — passed.
+- `cd apps/backend && golangci-lint run ./... --new-from-rev="0ca00c2aca4430a5e4f06874ba960743de1acf9a" --timeout=5m` — passed with 0 issues.
+- Follow-up review RED coverage proved the fixture-file hashes did not recognize
+  historical content in the exact trimmed form stored by `promptcfg.Get`.
+- Follow-up GREEN coverage uses loader-normalized historical hashes and proves
+  equal-timestamp unrecognized content remains unchanged.
+- The affected Go packages, changed-code lint, 58 public-doc tests, 41-page
+  public-doc validation, and `git diff --check` all passed again.
@@ -18,7 +18,7 @@ Kandev has several mechanisms that can act without repeated manual setup. Their

 Use workflow events for predictable transitions on existing work. Use a workspace automation when an external signal must create new work. MCP is a tool interface, not a scheduler.

-Across Kandev's task, configuration, external, and Office MCP modes, each tool call is validated against that mode's live `tools/list` schema before its handler runs. Missing required fields, wrong types, declared constraint violations, and unknown top-level fields return a tool error without performing the requested action. Nested configuration maps still accept arbitrary keys when their schema defines them as open.
+Across Kandev's task, configuration, external, and Office MCP modes, each tool call is validated against that mode's live `tools/list` schema before its handler runs. Missing required fields, wrong types, declared constraint violations, and unknown top-level fields return a tool error without performing the requested action. A missing-field error names each absent schema property, but never echoes submitted argument values. Nested configuration maps still accept arbitrary keys when their schema defines them as open.

 ## Quick path

@@ -10,17 +10,20 @@ Decision: [ADR-2026-08-01-validate-mcp-tool-arguments](../../decisions/2026-08-0

 ## Why

-Agents and external MCP clients need a failed tool call to be distinguishable from a successful operation. Silently ignoring an incorrect argument can create tasks, start agents, or mutate configuration without the caller's intended data while still reporting success.
+Agents and external MCP clients need a failed tool call to be distinguishable from a successful operation and specific enough to repair safely. Silently ignoring an incorrect argument can create tasks, start agents, or mutate configuration without the caller's intended data, while a generic constraint failure can leave an agent repeating the same malformed call.

 ## What

 - Every Kandev MCP tool validates its arguments against the schema registered for its current MCP mode before running its handler.
 - Missing required arguments, values of the wrong declared type, violated declared constraints, and unknown top-level arguments return a tool error and cause no handler or backend side effect.
+- A missing-required-property error names the missing schema property or properties as well as the invalid object location and `required` constraint, without returning argument values.
 - Parameterless tools accept omitted arguments or an empty object and reject any supplied field.
 - Validation follows mode changes: after the server changes its registered tool set, calls use the replacement tools' schemas.
 - Nested configuration objects remain open when their schema intentionally permits arbitrary keys.
 - `create_task_kandev` advertises `prompt` for the text delivered to a newly started agent. It accepts the former `description` name as an unadvertised compatibility alias, without adding a second field or explanation to the tool schema. A call containing both names fails rather than choosing one silently.
 - The backend task action continues receiving the text in its existing `description` field; this naming convergence changes only the MCP boundary.
+- Task-mode agent context lists the walkthrough tools and states that every `show_walkthrough_kandev.steps` item requires `file`, `line`, and `text`. The built-in `changes-walkthrough` request repeats that load-bearing call shape so callers do not depend on a client rendering nested JSON Schema correctly.
+- On startup, a stored built-in `changes-walkthrough` prompt that exactly matches the loader-normalized content of an untouched shipped revision is refreshed to the current embedded prompt. User-edited built-ins, unrecognized stored content, and user-owned prompts remain unchanged.

 ## API surface

@@ -30,18 +33,20 @@ The advertised MCP `tools/list` schemas do not gain repeated unknown-argument me
 2. Validate the resulting arguments against the currently registered tool schema with the root object closed to unknown fields.
 3. Invoke the handler only when validation succeeds.

-Validation failures are returned as MCP tool error results. They identify the invalid argument location and violated constraint without returning sensitive argument values.
+Validation failures are returned as MCP tool error results. They identify the invalid argument location and violated constraint without returning sensitive argument values. For the `required` constraint, the result also identifies each missing property by its schema-defined name.

 ## Failure modes

 - If tool arguments fail validation, Kandev returns an error and does not invoke the handler.
 - If a registered tool schema cannot compile, calls to that tool fail closed and Kandev logs the schema defect; test coverage prevents shipping an uncompilable built-in schema.
 - If both `prompt` and compatibility alias `description` are supplied to `create_task_kandev`, Kandev returns an error and creates no task.
+- Missing-property diagnostics expose only names declared by the registered schema, never sibling values or rejected argument contents.
+- Prompt refresh recognizes only exact stored content whose hash matches a loader-normalized historical built-in revision and that has never been saved by a user; unknown or edited content is left intact.
 - Validation errors do not echo complete prompts, credentials, configuration values, or other argument contents.

 ## Scenarios

- **GIVEN** any registered Kandev MCP tool, **WHEN** a caller omits one of its schema-required arguments, **THEN** the call returns a tool error and its handler is not invoked.
+- **GIVEN** any registered Kandev MCP tool, **WHEN** a caller omits one or more schema-required properties, **THEN** the call returns a tool error naming the invalid object location, the `required` constraint, and every missing schema property, and its handler is not invoked.
 - **GIVEN** any registered Kandev MCP tool, **WHEN** a caller supplies a value with the wrong schema type, **THEN** the call returns a tool error and its handler is not invoked.
 - **GIVEN** any registered Kandev MCP tool, **WHEN** a caller supplies an unknown top-level argument, **THEN** the call returns a tool error naming its location and its handler is not invoked.
 - **GIVEN** a parameterless tool, **WHEN** a caller omits arguments or supplies `{}`, **THEN** the handler runs normally; **WHEN** it supplies any field, **THEN** the call fails.
@@ -50,6 +55,8 @@ Validation failures are returned as MCP tool error results. They identify the in
 - **GIVEN** a caller supplies advertised `prompt` to `create_task_kandev`, **WHEN** validation runs, **THEN** the prompt is forwarded unchanged through the existing backend `description` field.
 - **GIVEN** a caller supplies only legacy `description` to `create_task_kandev`, **WHEN** validation runs, **THEN** the value is accepted as `prompt` and forwarded unchanged.
 - **GIVEN** a caller supplies both `prompt` and `description` to `create_task_kandev`, **WHEN** validation runs, **THEN** the call returns an error and creates no task.
+- **GIVEN** a task-mode agent receives Kandev's first-turn context or the built-in `changes-walkthrough` request, **WHEN** it prepares `show_walkthrough_kandev`, **THEN** the instructions identify `steps` as an ordered array whose items require `file`, `line`, and `text`; the first-turn context also names the show, get, and delete walkthrough tools.
+- **GIVEN** an existing installation with an untouched historical built-in `changes-walkthrough` prompt stored through the embedded loader, **WHEN** Kandev starts with a newer embedded prompt, **THEN** the stored built-in is refreshed; unrecognized content, a user-edited built-in, or a user-owned prompt is preserved.

 ## Out of scope

@@ -57,3 +64,5 @@ Validation failures are returned as MCP tool error results. They identify the in
 - Changing the semantic business rules enforced inside individual handlers and services.
 - Closing arbitrary-key nested configuration maps that are intentionally open.
 - Changing MCP transport, authorization, tool availability by mode, or backend action payloads.
+- Adding automatic tool-call retries or agent-client-specific schema rewriting.
+- Duplicating every registered tool schema in the first-turn task context.