feat(diagnostics): echo safe model-derived error detail

docs/docs/explanation/security-model.md

@@ -88,11 +88,11 @@ Static filtering is useful for deployment-wide availability, but it is not dynam
 
 Detailed causes exist only on the trusted side of the public boundary. Internal packages and host authorizers, resolvers, and handlers can hold or log those causes. A direct call to `authz/rego.Authorize` can return an ordinary error that identifies an undefined or non-Boolean decision, or carries an OPA evaluation or builtin failure.
 
-`codemode.Server.Execute` removes those trusted causes. It returns the documented public sentinel for execution, policy, handler, resource, and internal failures. Request cancellation returns `context.Canceled`. A deadline returns `ErrResourceLimit` and preserves `context.DeadlineExceeded` for `errors.Is`; no other execution cause remains wrapped at the root API.
+`codemode.Server.Execute` removes those trusted causes. It returns the documented public sentinel for execution, policy, handler, resource, and internal failures. Request cancellation returns `context.Canceled`. A deadline returns `ErrResourceLimit` and preserves `context.DeadlineExceeded` for `errors.Is`. Root `Error()` strings stay exactly coarse. Approved model-derived parser, resolver, and binding detail may travel with the sentinel for MCP formatting, but it is not part of the root error text.
 
-The MCP adapter narrows the boundary again. It emits only the fixed error texts in the [MCP tool reference](../reference/mcp-tools.md#errors). Resolver and custom-service details and recovered panic values become coarse responses. SDK input-schema errors are different: they occur before trusted subject resolution and can identify malformed client-owned fields or values.
+The MCP adapter narrows the boundary again. It emits the nine fixed error texts in the [MCP tool reference](../reference/mcp-tools.md#errors), plus two stable prefixes that may append approved CodeMode execution detail: `invalid program: ...` for parse and resolve positions and messages, and `invalid capability arguments: ...` for binding diagnostics. Resolver and custom-service details and recovered panic values become coarse responses. SDK input-schema errors are different: they occur before trusted subject resolution and can identify malformed client-owned fields or values.
 
-This projection prevents trusted diagnostic detail from becoming model-visible. In particular, MCP responses do not expose budget values, filtered capability identities, unknown requested names, argument names or values, source locations or text, Rego decision paths or rule names, handler messages, credentials, panic values, or stack details.
+This projection prevents host-derived diagnostic detail from becoming model-visible. MCP responses do not expose budget values, filtered capability identities, unknown requested names, host-derived argument values, Rego decision paths or rule names, handler messages, credentials, panic values, or stack details. The only MCP exceptions are parse or resolve positions and messages and binding argument diagnostics produced by the program that the service executed. With the shipped `*codemode.Server`, that program is the submitted `source`.
 
 If a host needs detailed diagnostics, its trusted authorizer, resolver, or handler must record them before returning. CodeMode cannot recover a discarded cause after the root or MCP projection. Apply the host's normal access controls and redaction rules to those logs.
 

docs/docs/reference/mcp-tools.md

@@ -237,7 +237,7 @@ There is no compatibility suffix, alias, or alternate signature field.
 
 ## `execute`
 
-Execute one Starlark program that defines `def main():` with zero arguments, calls only names confirmed through `search_api` and `describe_api` inside `main`, and returns `main`'s final result.
+Execute one Starlark program that defines `def main():` with zero arguments, calls only names confirmed through `search_api` and `describe_api` inside `main`, and returns `main`'s final result. Starlark is not Python: `sum`, `import`, `while`, and f-strings are unavailable; `load` is disabled; `print` is discarded.
 
 ### Input
 
@@ -254,7 +254,7 @@ Execute one Starlark program that defines `def main():` with zero arguments, cal
 }
 ```
 
-The source must define `def main():` as a function with zero arguments. Top-level source loading cannot make native calls; calls are accepted only while `main` runs, and only for names confirmed through `search_api` and `describe_api`. Module loading is disabled.
+The source must define `def main():` as a function with zero arguments. Starlark is not Python: `sum`, `import`, `while`, and f-strings are unavailable. Top-level source loading cannot make native calls; calls are accepted only while `main` runs, and only for names confirmed through `search_api` and `describe_api`. Module loading is disabled. `print` output is discarded.
 
 Capabilities are available by dotted name. The sample native call is `records.lookup(key="alpha", limit=2)`. Native calls accept keyword arguments only. Duplicate keyword syntax is rejected by the Starlark parser as `invalid program` before authorization or handler dispatch. Positional, unknown, missing, incorrectly typed, and out-of-range arguments reach binding and map to `invalid capability arguments`. For the sample, `key` is required and `limit` can be omitted, `None`, or an integer in the signed 64-bit range.
 
@@ -324,12 +324,13 @@ Only the final converted value from the worker process is exposed in the success
 
 ## Authoring and recovery
 
-The listed descriptions above are the model-facing contract. Recovery uses the same fixed coarse errors on this page. When recording or reporting a failed call, keep the coarse text and the recovery action; do not echo the failed source, arguments, credentials, or unknown requested name.
+The listed descriptions above are the model-facing contract. Recovery uses the nine fixed texts and two stable prefixes on this page. When recording or reporting a failed call, keep the error text and the recovery action; do not echo credentials, unknown requested names, or host-derived handler or policy text.
 
 - Search with a short literal substring over enabled names and summaries. If the result is empty, retry with a shorter term.
 - After `capability not found`, search again and pass `describe_api` an exact returned `name`, without whitespace or case changes.
-- After `invalid capability arguments`, compare the call with the published `signature` and `input` field shapes.
-- After `invalid program`, check the program against these requirements:
+- After `invalid capability arguments`, use any suffix after the stable prefix to identify the rejected argument, then compare the call with the published `signature` and `input` field shapes.
+- After `invalid program`, use any suffix after the stable prefix. A parse or resolve suffix includes a `<codemode>:line:col:` position in the submitted source. Check the program against these requirements:
+  - Write Starlark, not Python: `sum`, `import`, `while`, and f-strings are unavailable.
   - Define `main` with zero arguments.
   - Call only names confirmed through `search_api` and `describe_api`, and call them only inside `main`.
   - Return the final value from `main`.
@@ -342,14 +343,14 @@ The listed descriptions above are the model-facing contract. Recovery uses the s
 
 ## Errors
 
-After a well-formed call reaches the adapter, a resolver or service failure becomes a successful MCP protocol response with `isError` set and one of the eleven fixed text values below. The adapter removes resolver and custom-service details and recovered panic values. It does not expose budget values, filtered capability identities, unknown requested names, argument names or values, source locations or text, Rego decision paths or rule names, handler messages, credentials, panic values, or stack details.
+After a well-formed call reaches the adapter, a resolver or service failure becomes a successful MCP protocol response with `isError` set. Nine texts are fixed. Two classes keep a stable prefix and may append model-derived detail: `invalid program: ...` and `invalid capability arguments: ...`. The adapter removes resolver and custom-service details and recovered panic values. It does not expose budget values, filtered capability identities, unknown requested names, host-derived argument values, Rego decision paths or rule names, handler messages, credentials, panic values, or stack details. Parse and resolve suffixes may include a source position in the submitted program. Binding suffixes may include an argument name from the submitted call.
 
 | Text | Meaning |
 | --- | --- |
 | `unauthenticated` | The resolver failed or returned an empty subject ID. |
 | `capability not found` | `describe_api` did not find an enabled exact name. |
-| `invalid program` | Source, including duplicate keyword syntax, entry point, runtime behavior, or final-value conversion was invalid. |
-| `invalid capability arguments` | A native call failed binding: positional, unknown, missing, incorrectly typed, or out-of-range arguments. |
+| `invalid program` | Fixed prefix. Source, including duplicate keyword syntax, entry point, runtime behavior, or final-value conversion was invalid. A parse or resolve failure may append `<codemode>:line:col: message`. |
+| `invalid capability arguments` | Fixed prefix. A native call failed binding: positional, unknown, missing, incorrectly typed, or out-of-range arguments. A binding failure may append the model-derived argument diagnostic. |
 | `permission denied` | Policy returned a recognized denial. |
 | `authorization policy failure` | Policy evaluation failed. |
 | `resource limit exceeded` | A discovery, execution, depth, per-value, or aggregate intermediate-value budget was exceeded. |

internal/execution/classify_test.go

@@ -0,0 +1,25 @@
+package execution
+
+import (
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+	"go.starlark.net/syntax"
+)
+
+// TestClassifyRuntimeErrorOmitsParserInternalError proves parser bug text stays coarse.
+func TestClassifyRuntimeErrorOmitsParserInternalError(t *testing.T) {
+	filename := "<codemode>"
+	err := classifyRuntimeError(&executionState{}, syntax.Error{
+		Pos: syntax.MakePosition(&filename, 1, 1),
+		Msg: "internal error: parser panic",
+	})
+
+	require.ErrorIs(t, err, ErrInvalidProgram)
+	assert.Equal(t, ErrInvalidProgram.Error(), err.Error())
+	_, ok := SafeDetail(err)
+	assert.False(t, ok)
+	assert.NotContains(t, err.Error(), "internal error:")
+	assert.NotContains(t, err.Error(), "parser panic")
+}

internal/execution/detail.go

@@ -0,0 +1,44 @@
+package execution
+
+import "errors"
+
+// safeDetailError attaches one model-derived diagnostic suffix without changing the coarse error text.
+type safeDetailError struct {
+	// cause is the coarse classified sentinel.
+	cause error
+
+	// detail is the model-derived suffix excluded from Error.
+	detail string
+}
+
+// Error returns only the coarse cause text.
+func (err *safeDetailError) Error() string {
+	return err.cause.Error()
+}
+
+// Unwrap returns the coarse cause.
+func (err *safeDetailError) Unwrap() error {
+	return err.cause
+}
+
+// WithSafeDetail attaches detail to cause without changing cause.Error.
+//
+// Empty detail returns cause unchanged. Callers must pass only model-derived
+// suffixes; host-derived text must not be attached.
+func WithSafeDetail(cause error, detail string) error {
+	if detail == "" {
+		return cause
+	}
+	return &safeDetailError{cause: cause, detail: detail}
+}
+
+// SafeDetail reports the model-derived suffix attached to err, if any.
+//
+// Extraction follows the error chain with [errors.As].
+func SafeDetail(err error) (string, bool) {
+	var wrapped *safeDetailError
+	if !errors.As(err, &wrapped) {
+		return "", false
+	}
+	return wrapped.detail, true
+}

internal/execution/detail_test.go

@@ -0,0 +1,118 @@
+package execution_test
+
+import (
+	"fmt"
+	"sync/atomic"
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+
+	"github.com/meigma/codemode/internal/execution"
+)
+
+// TestWithSafeDetailPreservesCoarseCause proves the wrapper never changes Error or [errors.Is].
+func TestWithSafeDetailPreservesCoarseCause(t *testing.T) {
+	t.Run("empty detail returns cause", func(t *testing.T) {
+		got := execution.WithSafeDetail(execution.ErrInvalidProgram, "")
+
+		assert.Equal(t, execution.ErrInvalidProgram, got)
+		_, ok := execution.SafeDetail(got)
+		assert.False(t, ok)
+	})
+
+	t.Run("Error stays coarse", func(t *testing.T) {
+		const detail = "<codemode>:1:1: got '='"
+		got := execution.WithSafeDetail(execution.ErrInvalidProgram, detail)
+
+		require.ErrorIs(t, got, execution.ErrInvalidProgram)
+		assert.Equal(t, execution.ErrInvalidProgram.Error(), got.Error())
+		gotDetail, ok := execution.SafeDetail(got)
+		require.True(t, ok)
+		assert.Equal(t, detail, gotDetail)
+	})
+
+	t.Run("extracts through genuine wrapper", func(t *testing.T) {
+		const detail = `unknown argument "keu"`
+		got := fmt.Errorf("worker: %w", execution.WithSafeDetail(execution.ErrInvalidArguments, detail))
+
+		require.ErrorIs(t, got, execution.ErrInvalidArguments)
+		gotDetail, ok := execution.SafeDetail(got)
+		require.True(t, ok)
+		assert.Equal(t, detail, gotDetail)
+	})
+}
+
+// TestExecuteAttachesApprovedProgramDiagnostics proves parse and resolve suffixes stay model-derived.
+func TestExecuteAttachesApprovedProgramDiagnostics(t *testing.T) {
+	engine := buildEngine(t)
+	tests := []struct {
+		// name identifies the invalid program.
+		name string
+
+		// source is the submitted Starlark program.
+		source string
+
+		// contains is the required model-derived suffix fragment.
+		contains string
+	}{
+		{
+			name:     "ordinary syntax",
+			source:   "def main():\n    return =\n",
+			contains: "<codemode>:",
+		},
+		{
+			name:     "undefined sum",
+			source:   "def main():\n    return sum([1, 2])\n",
+			contains: "undefined: sum",
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			_, err := engine.Execute(tt.source, echoNativeCall(), defaultExecutionLimits())
+
+			require.ErrorIs(t, err, execution.ErrInvalidProgram)
+			assert.Equal(t, execution.ErrInvalidProgram.Error(), err.Error())
+			detail, ok := execution.SafeDetail(err)
+			require.True(t, ok)
+			assert.Contains(t, detail, tt.contains)
+			assert.Contains(t, detail, "<codemode>:")
+			assert.NotContains(t, err.Error(), detail)
+		})
+	}
+}
+
+// TestExecuteAttachesOneBindingDiagnostic proves BindShape suffixes omit the coarse prefix.
+func TestExecuteAttachesOneBindingDiagnostic(t *testing.T) {
+	var nativeCalls atomic.Int64
+	_, err := buildEngine(t).Execute(
+		`def main(): return records.lookup(keu="alpha")`,
+		countingNativeCall(&nativeCalls),
+		defaultExecutionLimits(),
+	)
+
+	require.ErrorIs(t, err, execution.ErrInvalidArguments)
+	assert.Equal(t, execution.ErrInvalidArguments.Error(), err.Error())
+	detail, ok := execution.SafeDetail(err)
+	require.True(t, ok)
+	assert.Equal(t, `unknown argument "keu"`, detail)
+	assert.NotContains(t, detail, execution.ErrInvalidArguments.Error())
+	assert.Zero(t, nativeCalls.Load())
+}
+
+// TestExecuteKeepsGenericRuntimeErrorsCoarse proves evaluator messages stay hidden.
+func TestExecuteKeepsGenericRuntimeErrorsCoarse(t *testing.T) {
+	_, err := buildEngine(t).Execute(
+		"def main():\n    fail(\"db password rejected\")\n",
+		echoNativeCall(),
+		defaultExecutionLimits(),
+	)
+
+	require.ErrorIs(t, err, execution.ErrInvalidProgram)
+	assert.Equal(t, execution.ErrInvalidProgram.Error(), err.Error())
+	_, ok := execution.SafeDetail(err)
+	assert.False(t, ok)
+	assert.NotContains(t, err.Error(), "db password rejected")
+	assert.NotContains(t, err.Error(), "fail")
+}

internal/execution/execute.go

@@ -3,7 +3,9 @@ package execution
 import (
 	"errors"
 	"fmt"
+	"strings"
 
+	"go.starlark.net/resolve"
 	"go.starlark.net/starlark"
 	"go.starlark.net/syntax"
 
@@ -176,7 +178,7 @@ func callCapability(
 	canonical, bindingErr := binding.BindShape(input, args, kwargs)
 	if bindingErr != nil {
 		if errors.Is(bindingErr, binding.ErrInvalidArguments) {
-			return nil, fmt.Errorf("%w: %w", ErrInvalidArguments, bindingErr)
+			return nil, invalidArgumentDetail(bindingErr)
 		}
 		return nil, fmt.Errorf("%w: %w", ErrInternal, bindingErr)
 	}
@@ -188,22 +190,69 @@ func classifyRuntimeError(state *executionState, err error) error {
 	if state.stepLimited {
 		return ErrResourceLimit
 	}
+	if detail, ok := programDetail(err); ok {
+		return WithSafeDetail(ErrInvalidProgram, detail)
+	}
+	cause := unwrapEvalError(err)
 	switch {
-	case errors.Is(err, ErrInvalidArguments):
-		return ErrInvalidArguments
-	case errors.Is(err, ErrPermissionDenied):
+	case errors.Is(cause, ErrInvalidArguments):
+		return classifiedSafeDetail(ErrInvalidArguments, cause)
+	case errors.Is(cause, ErrPermissionDenied):
 		return ErrPermissionDenied
-	case errors.Is(err, ErrPolicyFailure):
+	case errors.Is(cause, ErrPolicyFailure):
 		return ErrPolicyFailure
-	case errors.Is(err, ErrResourceLimit):
+	case errors.Is(cause, ErrResourceLimit):
 		return ErrResourceLimit
-	case errors.Is(err, ErrCapabilityFailure):
+	case errors.Is(cause, ErrCapabilityFailure):
 		return ErrCapabilityFailure
-	case errors.Is(err, ErrInternal):
+	case errors.Is(cause, ErrInternal):
 		return ErrInternal
-	case errors.Is(err, ErrInvalidProgram):
-		return ErrInvalidProgram
+	case errors.Is(cause, ErrInvalidProgram):
+		return classifiedSafeDetail(ErrInvalidProgram, cause)
 	default:
-		return fmt.Errorf("%w: %w", ErrInvalidProgram, err)
+		return ErrInvalidProgram
+	}
+}
+
+// unwrapEvalError returns the evaluator cause without reading EvalError.Msg.
+func unwrapEvalError(err error) error {
+	evalErr, ok := err.(*starlark.EvalError) //nolint:errorlint // Exact type excludes unrelated wrappers.
+	if !ok {
+		return err
+	}
+	return evalErr.Unwrap()
+}
+
+// classifiedSafeDetail reattaches an approved suffix to sentinel, or returns sentinel unchanged.
+func classifiedSafeDetail(sentinel error, err error) error {
+	detail, ok := SafeDetail(err)
+	if !ok {
+		return sentinel
+	}
+	return WithSafeDetail(sentinel, detail)
+}
+
+// programDetail reports a model-derived parse or resolve suffix for a direct evaluator error.
+func programDetail(err error) (string, bool) {
+	if syntaxErr, ok := err.(syntax.Error); ok { //nolint:errorlint // Exact type is the provenance boundary.
+		if strings.HasPrefix(syntaxErr.Msg, "internal error:") {
+			return "", false
+		}
+		return syntaxErr.Error(), true
+	}
+	list, ok := err.(resolve.ErrorList) //nolint:errorlint // Exact type is the provenance boundary.
+	if !ok || len(list) == 0 {
+		return "", false
+	}
+	return list[0].Error(), true
+}
+
+// invalidArgumentDetail attaches the binding suffix after the exact sentinel prefix, or stays coarse.
+func invalidArgumentDetail(err error) error {
+	prefix := binding.ErrInvalidArguments.Error() + ": "
+	suffix, ok := strings.CutPrefix(err.Error(), prefix)
+	if !ok {
+		return ErrInvalidArguments
 	}
+	return WithSafeDetail(ErrInvalidArguments, suffix)
 }