diff --git a/assets/functions/tools/fs_patch.sh b/assets/functions/tools/fs_patch.sh index f48b8b3..bb5e434 100755 --- a/assets/functions/tools/fs_patch.sh +++ b/assets/functions/tools/fs_patch.sh @@ -14,6 +14,11 @@ set -e # is the most common cause of "unable to apply patch" failures, especially in files with sed/jq/regex pipelines or # embedded Python with quoted strings. # - Hunks are applied in order; the first hunk that fails aborts the whole patch — later hunks are NOT attempted. +# - Hunks anchor at the FIRST exact match of their context in the file, so include enough context to make each hunk +# unique. Hunks must appear in file order. +# - Every hunk needs at least one context or removed line to anchor it; a hunk containing only additions is an error. +# - Unrecognized lines inside a hunk are hard errors: every hunk line must start with ' ' (context), '-' (removal), +# or '+' (addition). # - If you've edited this file in earlier tool calls, fs_cat it again before composing the patch. A stale view of the file # produces context lines that no longer match. # - On failure the error message names the failing hunk and shows the expected-vs-actual line. Fix that specific line and @@ -50,6 +55,14 @@ main() { # status so a failure still aborts under `set -e`. new_contents="$(patch_file "$argc_path" <(printf "%s" "$argc_contents"); rc=$?; printf x; exit "$rc")" new_contents="${new_contents%x}" + + # awk newline-terminates every printed line, so patching a file that lacks + # a final newline would silently add one. Preserve the original file's + # final-newline state instead. + if [[ -n "$(tail -c 1 "$argc_path")" ]]; then + new_contents="${new_contents%$'\n'}" + fi + printf "%s" "$new_contents" | git diff --no-index "$argc_path" - || true guard_operation "Apply changes?" diff --git a/assets/functions/utils/prompt-utils.sh b/assets/functions/utils/prompt-utils.sh index 85137c9..943afd6 100755 --- a/assets/functions/utils/prompt-utils.sh +++ b/assets/functions/utils/prompt-utils.sh @@ -528,7 +528,11 @@ guard_operation() { # + print(f"Hello {name}") patch_file() { awk ' - FNR == NR { + function isHeaderPair(i) { + return (patchLines[i] ~ /^--- / && patchLines[i+1] ~ /^\+\+\+ / && patchLines[i+2] ~ /^@@/) + } + + FILENAME == ARGV[1] { lines[FNR] = $0 next; } @@ -547,11 +551,6 @@ patch_file() { while (patchLineIndex <= totalPatchLines) { line = patchLines[patchLineIndex] - if (line ~ /^--- / || line ~ /^\+\+\+ /) { - patchLineIndex++ - continue - } - if (line ~ /^@@/) { mode = "hunk" hunkIndex++ @@ -560,7 +559,22 @@ patch_file() { } if (mode == "hunk") { - while (patchLineIndex <= totalPatchLines && line ~ /^[-+ ]|^\s*$/ && line !~ /^--- /) { + while (patchLineIndex <= totalPatchLines) { + line = patchLines[patchLineIndex] + + if (line ~ /^\\ No newline/) { + patchLineIndex++ + continue + } + + if (isHeaderPair(patchLineIndex)) { + break + } + + if (line !~ /^[-+ ]/ && line !~ /^[ \t]*$/) { + break + } + sanitizedLine = substr(line, 2) if (line !~ /^\+/) { @@ -574,13 +588,37 @@ patch_file() { } patchLineIndex++ - line = patchLines[patchLineIndex] } mode = "none" - } else { - patchLineIndex++ + continue } + + if (isHeaderPair(patchLineIndex)) { + patchLineIndex += 2 + continue + } + + if (line ~ /^\\ No newline/) { + patchLineIndex++ + continue + } + + if (hunkIndex == 0) { + # Preamble before the first hunk: tolerate prose, code fences, and lone headers. + patchLineIndex++ + continue + } + + if (line ~ /^[ \t]*$/ || line ~ /^```/) { + patchLineIndex++ + continue + } + + print "error: unrecognized line in patch (line " patchLineIndex "): " line > "/dev/stderr" + print "" > "/dev/stderr" + print "Every line inside a hunk must start with \" \" (context), \"-\" (removal), or \"+\" (addition)." > "/dev/stderr" + exit 1 } if (hunkIndex == 0) { @@ -593,6 +631,36 @@ patch_file() { } totalHunks = hunkIndex + + if (totalLines == 0) { + for (h = 1; h <= totalHunks; h++) { + if (hunkTotalOriginalLines[h] > 0) { + print "error: unable to apply patch" > "/dev/stderr" + print "" > "/dev/stderr" + print "Hunk " h " expects existing content but the file is empty." > "/dev/stderr" + exit 1 + } + } + + for (h = 1; h <= totalHunks; h++) { + for (i = 1; i <= hunkTotalUpdatedLines[h]; i++) { + print hunkUpdatedLines[h,i] + } + } + + exit 0 + } + + for (h = 1; h <= totalHunks; h++) { + if (hunkTotalOriginalLines[h] == 0) { + print "error: unable to apply patch" > "/dev/stderr" + print "" > "/dev/stderr" + print "Hunk " h " contains no context or removed lines; include at least one" > "/dev/stderr" + print "context line so the hunk can be anchored." > "/dev/stderr" + exit 1 + } + } + hunkIndex = 1 for (lineIndex = 1; lineIndex <= totalLines; lineIndex++) { @@ -603,7 +671,7 @@ patch_file() { nextLineIndex = lineIndex + 1 for (i = 2; i <= hunkTotalOriginalLines[hunkIndex]; i++) { - if (lines[nextLineIndex] != hunkOriginalLines[hunkIndex,i]) { + if (nextLineIndex > totalLines || lines[nextLineIndex] != hunkOriginalLines[hunkIndex,i]) { if (i - 1 > bestPartialLen[hunkIndex]) { bestPartialLen[hunkIndex] = i - 1 bestPartialAnchorLine[hunkIndex] = lineIndex @@ -646,9 +714,13 @@ patch_file() { print "" > "/dev/stderr" print "Closest match: anchored at file line " bestPartialAnchorLine[failingHunk] ", matched " bestPartialLen[failingHunk] " of " hunkTotalOriginalLines[failingHunk] " original lines before diverging." > "/dev/stderr" print "" > "/dev/stderr" - print "At file line " bestPartialDivergeLine[failingHunk] " (hunk original line " bestPartialHunkPos[failingHunk] "):" > "/dev/stderr" - print " expected: " bestPartialExpected[failingHunk] > "/dev/stderr" - print " actual: " bestPartialActual[failingHunk] > "/dev/stderr" + if (bestPartialDivergeLine[failingHunk] > totalLines) { + print "The hunk expects additional lines beyond the end of the file (file has " totalLines " lines)." > "/dev/stderr" + } else { + print "At file line " bestPartialDivergeLine[failingHunk] " (hunk original line " bestPartialHunkPos[failingHunk] "):" > "/dev/stderr" + print " expected: " bestPartialExpected[failingHunk] > "/dev/stderr" + print " actual: " bestPartialActual[failingHunk] > "/dev/stderr" + } } print "" > "/dev/stderr" diff --git a/src/parsers/bash.rs b/src/parsers/bash.rs index c00269c..ad9d37e 100644 --- a/src/parsers/bash.rs +++ b/src/parsers/bash.rs @@ -23,6 +23,7 @@ pub fn generate_bash_declarations( "", env::var("TERM_WIDTH").ok().and_then(|v| v.parse().ok()), )?; + let build_script = allow_empty_required_values(&build_script); fs::write(tools_file_path, &build_script) .with_context(|| format!("Failed to write built script to '{tools_file_path:?}'"))?; @@ -74,6 +75,20 @@ fn underscore(s: &str) -> String { s.replace('-', "_") } +/// argc's generated required-param check uses `-z "${!name:-}"`, which +/// conflates "not provided" with "provided but empty", so a required option +/// passed an explicit empty string (e.g. `fs_write --content=''` to create an +/// empty file) is rejected as "required arguments were not provided". The +/// JSON schema we advertise to models treats `required` as *presence*, so +/// rewrite the check to a set-ness test to keep runtime behavior consistent +/// with the schema. Applied post-build so it survives every regeneration. +fn allow_empty_required_values(build_script: &str) -> String { + build_script.replace( + r#"if [[ -z "${!name:-}" ]]; then"#, + r#"if [[ -z "${!name+x}" ]]; then"#, + ) +} + fn schema_ty(t: &str) -> JsonSchema { JsonSchema { type_value: Some(t.to_string()), @@ -147,3 +162,23 @@ fn parse_parameters_schema(flags: &[FlagOptionValue]) -> JsonSchema { required: Some(required), } } + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn rewrites_argc_required_check_to_setness_test() { + let src = "# @describe test tool\n# @option --content! The content\nmain() { :; }\n"; + let built = argc::build(src, "", None).expect("argc build failed"); + assert!( + built.contains(r#"if [[ -z "${!name:-}" ]]; then"#), + "argc changed its generated required-param template; update allow_empty_required_values()" + ); + + let fixed = allow_empty_required_values(&built); + + assert!(fixed.contains(r#"if [[ -z "${!name+x}" ]]; then"#)); + assert!(!fixed.contains(r#"if [[ -z "${!name:-}" ]]; then"#)); + } +}