feat: improve command failure diagnostics

Include command index, source line, operation, path, and category in
script errors, while escaping control characters and newlines in
diagnostics. Add coverage for syntax, selection, file, and edit
failures.

Update the README, interface specification, and agent instructions with
current in-memory editing semantics and workspace-root requirements for
translation and patch application.

Author: yusing

AGENT_INSTRUCTIONS.md

@@ -1,30 +1,104 @@
 # Editing with hpatch
 
-Inspect the relevant source, then send a compact script to `hpatch translate`. Pass its
-stdout directly to the native `apply_patch` tool inside the same wrapper operation:
+Inspect the relevant source before selecting it. Run `hpatch translate` from the
+workspace root with paths relative to that root, then pass its stdout directly to the
+native `apply_patch` tool inside one model tool call or equivalent host orchestration
+boundary:
 
 ```text
-translated = exec("hpatch translate", stdin=SCRIPT)
-if translated failed: return its error
-apply_patch(translated.stdout)
+translated = exec("hpatch translate", stdin=SCRIPT, cwd=WORKSPACE_ROOT)
+if translated failed:
+    return its diagnostic
+
+applied = native_apply_patch(translated.stdout)
+if applied failed:
+    return its diagnostic
+
+reread the intended workspace paths
+run focused validation
 ```
 
-Keep the translated patch internal. Do not invoke a shell executable named
-`apply_patch`, return the patch to the model, or ask the model to repeat it. The native
-tool call is what applies the change and lets the harness display its diff.
+The boundary requirement means the translated patch stays internal: do not invoke a
+shell executable named `apply_patch`, return the patch to the model, or ask the model
+to repeat it. Use a non-PTY stdin facility when the wrapper provides one. Keep stdout
+patch-only and propagate diagnostics from either stage.
+
+The available native `apply_patch` resolves relative paths from its fixed workspace
+root and has no working-directory override. The working directory used by
+`hpatch translate` is not embedded in its textual patch or transferred to the native
+tool. Therefore:
+
+- use workspace-relative paths and normally run translation from the workspace root;
+- do not expect an absolute path or translator working directory to patch outside it;
+- for an artifact ultimately required in a system temporary directory, edit and
+  validate it in an ignored workspace-local staging directory, then relocate the
+  completed artifact;
+- treat an opaque successful native result such as `{}` as no operation summary:
+  reread the intended files and validate behavior;
+- treat an absent, malformed, unrelated, or unknown/future native result as
+  unconfirmed rather than inventing hpatch semantics.
 
 ```text
 in PATH                     select an existing file
 new PATH                    select a new empty file at cursor 0:0
 mv PATH                     move the selected file
-rm                          remove the selected file
-sel LINE START:END          select inclusive one-based columns
-tsel LINE OCCURRENCE "TEXT" select a literal; -1 means the last occurrence
-rsel START:END              select inclusive complete lines
+rm                          remove the selected file and clear active state
+sel LINE START:END          select inclusive one-based Unicode columns
+tsel LINE OCCURRENCE "TEXT" select a nonempty one-line literal; -1 is last
+rsel START:END              select inclusive complete logical lines
 type "TEXT"                 replace the selection or insert at the cursor
 del                         delete the selection
 dup                         duplicate the selection
 ```
 
-Commands run sequentially and may switch files with multiple `in` commands. `type` and
-`tsel` operands are JSON strings.
+Commands run sequentially against the current in-memory contents and may switch files
+with multiple `in` commands. Returning to a file resets its cursor to `0:0` but
+retains pending edits. Every selection replaces the prior cursor or selection.
+`type` and `del` leave the cursor immediately after the inserted text or at the
+deleted selection's start. `dup` selects the new copy.
+
+`rsel` owns each selected line's terminator when present. Replacing complete lines
+with `type` therefore requires the replacement string to include any desired final
+newline. Earlier edits immediately affect later line numbers:
+
+```text
+in example.go
+rsel 2:3
+type "replacement line\n"
+tsel 3 1 "later"
+type "updated"
+```
+
+Here the original lines 2–3, including line 3's terminator, are replaced. The later
+`tsel` observes the resulting current line 3, not the original file.
+
+`type` and `tsel` operands are JSON strings. Generate operands with a JSON encoder
+instead of hand-escaping multiline text, quotes, backslashes, template literals, or
+Unicode. A `type` value may contain line terminators; a `tsel` value may not.
+
+Additional examples:
+
+```text
+new note.txt
+type "foo"
+type " "
+type "bar\n"
+
+in config.txt
+tsel 4 -1 "old"
+type "new"
+
+in logs.txt
+tsel 8 1 "debug"
+del
+
+in block.txt
+rsel 2:4
+dup
+
+in old.txt
+mv current.txt
+
+in obsolete.txt
+rm
+```

README.md

@@ -38,9 +38,11 @@ type " world\n"
 EOF
 ```
 
-The second command writes an `Add File` patch to stdout. Errors go to stderr and return
-a nonzero status. Normal mode preserves existing file permission bits and creates files
-from `new` with mode `0644`.
+The second command writes an `Add File` patch to stdout. Successful translation keeps
+stdout patch-only. Errors return nonzero and go to stderr; evaluation diagnostics
+identify the command index, source line, operation, relevant path, and failure category.
+Normal mode preserves existing file permission bits and creates files from `new` with
+mode `0644`.
 
 Translate mode emits LF-only logical-line patches. OpenAI `apply_patch` cannot preserve
 CRLF bytes when such a patch is applied, so applying translated output to a CRLF file
@@ -80,9 +82,15 @@ dup
 `in` and `new` select a file at the cursor position before its first character.
 `type` inserts at the cursor or replaces a selection, then advances the cursor. Lines
 and inclusive columns are one-based and count Unicode code points. String operands use
-JSON syntax. Commands execute sequentially, including across repeated `in` commands.
-Paths use normal host filesystem semantics: relative paths resolve from the current
-directory and absolute paths remain absolute.
+JSON syntax; generate nontrivial operands with a JSON encoder. Commands execute
+sequentially against current in-memory content, including across repeated `in`
+commands. `rsel` includes each selected line's terminator when present, so complete-line
+replacement text must include any desired final newline.
+
+Paths use normal host filesystem semantics during translation: relative paths resolve
+from hpatch's current directory and absolute paths remain absolute. The translated text
+contains paths but no current-directory metadata. A downstream patch tool independently
+chooses its application root, so its root must be aligned with the paths hpatch emits.
 
 The complete behavior and failure contract is in
 [`doc/spec/interface.md`](doc/spec/interface.md). A shorter instruction sheet for

doc/spec/interface.md

@@ -86,10 +86,14 @@ Numbers are base-ten integers. Text lines, selection columns, and inclusive endp
 are one-based. Cursor `0:0` denotes the position before the first code point of a file;
 it is editor state, not a command operand. `OCCURRENCE` is nonzero: positive values
 count exact, non-overlapping literal matches from the start; negative values count
-them from the end. JSON operands use standard JSON string decoding. Paths are nonempty
-filesystem paths normalized with the host OS path rules. Relative paths resolve from
-the process working directory; absolute paths remain absolute. Trailing operands and
-unknown commands are invalid.
+them from the end. JSON operands use standard JSON string decoding. `type` strings may
+contain line terminators; `tsel` strings must stay within one logical line.
+
+Paths are nonempty filesystem paths normalized with the host OS path rules. During
+translation, relative paths resolve from the hpatch process working directory and
+absolute paths remain absolute. Emitted patch paths do not carry that process working
+directory as metadata. A downstream patch consumer resolves them according to its own
+application-root contract. Trailing operands and unknown commands are invalid.
 
 Acceptance:
 
@@ -150,12 +154,17 @@ points, including one code point per tab; the line terminator is not selectable.
 The literal must be nonempty and cannot contain a line terminator. Matches do not
 overlap. `-1` selects the last match.
 
-`rsel` selects an inclusive range of complete current logical lines. It records a
-linewise selection so duplication creates another complete adjacent line range,
-including when the selected final line has no terminator.
+`rsel` selects an inclusive range of complete current logical lines. For every
+selected line it owns the line terminator when one is present. It records a linewise
+selection so duplication creates another complete adjacent line range, including when
+the selected final line has no terminator. Replacing an `rsel` selection with `type`
+therefore removes the selected final terminator unless the replacement string supplies
+one.
 
 Every selection replaces the previous cursor or selection. Commands observe current
-contents and current line numbering after earlier edit actions.
+contents and current line numbering after earlier edit actions. `type` leaves the
+cursor immediately after inserted text, `del` leaves it at the removed selection's
+start, and `dup` selects the duplicated copy.
 
 Acceptance:
 
@@ -243,8 +252,12 @@ does not modify source files. Normal mode continues to preserve existing line en
 outside explicitly inserted strings. Applying translated output to a non-LF file may
 normalize that file to LF; this is a declared format limitation, not byte equivalence.
 
-Failures emit one concise diagnostic to stderr, prefixed with `hpatch:` and, for script
-errors, the script line number. They return nonzero and emit no stdout.
+Failures emit one concise diagnostic to stderr, prefixed with `hpatch:`. Script
+diagnostics identify the one-based nonblank command index, one-based source line,
+operation, relevant operand or selected path when one exists, and a failure category
+(`syntax`, `file`, `selection`, or `edit`). Control bytes in every diagnostic are escaped
+and embedded newlines are folded so one failure remains one logical line. Failures
+return nonzero and emit no stdout.
 
 Acceptance:
 

hpatch_test.go

@@ -266,6 +266,66 @@ func TestEvaluationFailuresDoNotMutateOrEmitPatch(t *testing.T) {
 	}
 }
 
+func TestFailureDiagnosticsIdentifyCommandContext(t *testing.T) {
+	tests := []struct {
+		name   string
+		script string
+		want   string
+	}{
+		{
+			name:   "selection failure includes selected path",
+			script: "in file.txt\ntsel 1 2 \"aa\"",
+			want:   "hpatch: command 2, source line 2, operation \"tsel\", path \"file.txt\", category selection: occurrence 2 of \"aa\" not found on line 1\n",
+		},
+		{
+			name:   "file failure includes operand path",
+			script: "new file.txt",
+			want:   "hpatch: command 1, source line 1, operation \"new\", path \"file.txt\", category file: destination file.txt already exists\n",
+		},
+		{
+			name:   "malformed command is syntax",
+			script: "in file.txt\nsel 1 2",
+			want:   "hpatch: command 2, source line 2, operation \"sel\", category syntax: unknown or malformed command\n",
+		},
+		{
+			name:   "unknown future command is syntax",
+			script: "\nin file.txt\nsplice 1:2",
+			want:   "hpatch: command 2, source line 3, operation \"splice\", category syntax: unknown or malformed command\n",
+		},
+		{
+			name:   "tab-separated malformed command identifies token",
+			script: "in file.txt\nsplice\t1:2",
+			want:   "hpatch: command 2, source line 2, operation \"splice\", category syntax: unknown or malformed command\n",
+		},
+		{
+			name:   "control byte in malformed operation is escaped",
+			script: "in file.txt\n\x1b[31msplice 1:2",
+			want:   "hpatch: command 2, source line 2, operation \"\\x1b[31msplice\", category syntax: unknown or malformed command\n",
+		},
+		{
+			name:   "control byte in path message is escaped",
+			script: "new \x1b[31mfile.txt",
+			want:   "hpatch: command 1, source line 1, operation \"new\", path \"\\x1b[31mfile.txt\", category file: destination \\x1b[31mfile.txt already exists\n",
+		},
+		{
+			name:   "edit without file omits path",
+			script: "type \"x\"",
+			want:   "hpatch: command 1, source line 1, operation \"type\", category edit: type requires an active file\n",
+		},
+	}
+	for _, test := range tests {
+		t.Run(test.name, func(t *testing.T) {
+			root := t.TempDir()
+			writeTestFile(t, root, "file.txt", "aaa\n", 0o644)
+			writeTestFile(t, root, "\x1b[31mfile.txt", "occupied\n", 0o644)
+			stdout, stderr, exitCode := runForTest(root, []string{"translate"}, test.script)
+			if exitCode != 1 || stdout != "" || stderr != test.want {
+				t.Fatalf("Run() = exit %d, stdout %q, stderr %q; want stderr %q", exitCode, stdout, stderr, test.want)
+			}
+		})
+	}
+}
+
 func TestInvalidUTF8IsRejected(t *testing.T) {
 	root := t.TempDir()
 	if err := os.WriteFile(filepath.Join(root, "binary.txt"), []byte{0xff}, 0o644); err != nil {

program.go

@@ -2,6 +2,7 @@ package hpatch
 
 import (
 	"encoding/json"
+	"errors"
 	"fmt"
 	"path/filepath"
 	"regexp"
@@ -31,12 +32,30 @@ type program struct {
 }
 
 type commandError struct {
-	Line    int
-	Message string
+	Command   int
+	Line      int
+	Operation string
+	Path      string
+	Category  string
+	Message   string
 }
 
 func (e *commandError) Error() string {
-	return fmt.Sprintf("line %d: %s", e.Line, e.Message)
+	var context []string
+	if e.Command != 0 {
+		context = append(context, fmt.Sprintf("command %d", e.Command))
+	}
+	context = append(context, fmt.Sprintf("source line %d", e.Line))
+	if e.Operation != "" {
+		context = append(context, fmt.Sprintf("operation %q", e.Operation))
+	}
+	if e.Path != "" {
+		context = append(context, fmt.Sprintf("path %q", e.Path))
+	}
+	if e.Category != "" {
+		context = append(context, "category "+e.Category)
+	}
+	return fmt.Sprintf("%s: %s", strings.Join(context, ", "), e.Message)
 }
 
 func parse(source string) (*program, error) {
@@ -47,9 +66,20 @@ func parse(source string) (*program, error) {
 		if strings.TrimSpace(line) == "" {
 			continue
 		}
+		commandIndex := len(program.instructions) + 1
 		command, err := parseInstruction(lineNumber, line)
 		if err != nil {
-			return nil, err
+			message := err.Error()
+			if sourceError, ok := errors.AsType[*commandError](err); ok {
+				message = sourceError.Message
+			}
+			return nil, &commandError{
+				Command:   commandIndex,
+				Line:      lineNumber,
+				Operation: strings.Fields(line)[0],
+				Category:  "syntax",
+				Message:   message,
+			}
 		}
 		program.instructions = append(program.instructions, command)
 	}

run.go

@@ -7,7 +7,9 @@ import (
 	"io/fs"
 	"os"
 	"path/filepath"
+	"strconv"
 	"strings"
+	"unicode"
 	"unicode/utf8"
 )
 
@@ -112,13 +114,29 @@ func resolveFilesystemPath(workingDirectory, path string) string {
 	return filepath.Join(workingDirectory, path)
 }
 
+func sanitizeDiagnostic(message string) string {
+	var sanitized strings.Builder
+	for _, character := range message {
+		switch {
+		case character == '\n':
+			sanitized.WriteString("; ")
+		case unicode.IsControl(character):
+			escaped := strconv.QuoteRune(character)
+			sanitized.WriteString(escaped[1 : len(escaped)-1])
+		default:
+			sanitized.WriteRune(character)
+		}
+	}
+	return sanitized.String()
+}
+
 func fail(stderr io.Writer, message string) int {
-	message = strings.ReplaceAll(message, "\n", "; ")
+	message = sanitizeDiagnostic(message)
 	_, _ = fmt.Fprintf(stderr, "hpatch: %s\n", message)
 	return 1
 }
 
 func warn(stderr io.Writer, message string) {
-	message = strings.ReplaceAll(message, "\n", "; ")
+	message = sanitizeDiagnostic(message)
 	_, _ = fmt.Fprintf(stderr, "hpatch: warning: %s\n", message)
 }