fix: latent parsing bugs in fs_patch and argc

This commit is contained in:
2026-08-14 16:12:41 -06:00
parent 644d899f78
commit e1b6e3f8c6
3 changed files with 134 additions and 14 deletions
+13
View File
@@ -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?"
+83 -11
View File
@@ -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,10 +714,14 @@ 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"
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"
print "Lines must match byte-for-byte (no fuzzy matching). Check escaping, whitespace, and quoting." > "/dev/stderr"
+35
View File
@@ -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"#));
}
}