fix(jq,make,sql): CRLF line-ending bugs in cluster-id and .regret parsing - #529
Merged
Conversation
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
…sing
## jq -- TWO bugs, BOTH execution-verified end-to-end
1. capture_jq.sh/validate_jq.sh: jq.exe (Windows build) emits CRLF line
endings on -r text output. list_jq_clusters()'s output is read via
`while IFS= read -r cluster_id`, which only splits on '\n' -- every
cluster_id but the last one (command substitution strips only the
final trailing newline) carries a trailing '\r', so every subsequent
`jq -r ... select(.id == "$cluster_id")` lookup matches nothing.
Reproduced live: capture_jq.sh failed 5/6 clusters with "jq file not
found" (the 6th -- last line -- worked by accident). Fixed by
stripping the trailing \r right after reading cluster_id. Confirmed:
6/6 captured after the fix.
2. validate_jq.sh: same root issue as the rest of this session's CRLF
findings -- its own `while IFS= read -r line` over the .regret file
content doesn't strip '\r', so `[[ "$line" == "---" ]]` fails to find
the separator on a CRLF-converted .regret file.
## Verification (live execution)
- Before fix: capture_jq.sh => 1/6 captured, 5/6 "file not found"
- After fix: capture_jq.sh => 6/6 captured
- validate (LF .regret) => 6/6 PASS
- Converted all 6 .regret files to CRLF (sed 's/$/\r/', simulating a
Windows git checkout with core.autocrlf=true) => still 6/6 PASS
- Broke greet() ("Hello, " -> "Hi, ") => validate => exactly 5/6 PASS,
1/6 FAIL (jq-greet, hash + output diff both correct)
- Restored greet() => 6/6 PASS again
## make, sql -- pattern-applied, NOT execution-verified
- validate_make.sh: identical `while IFS= read -r line` + `[[ "$line" ==
"---" ]]` pattern as jq -- same fix applied (strip trailing \r after
read). No `make` binary available locally to run the full cycle.
- validate_sql.mjs: `content.split('\n')` + `line === '---'` -- same bug,
same fix family (normalize CRLF before split). No sqlite3 available
locally to run the full cycle.
Same bug family as the rest of this session: validate_nim.sh's embedded node -e parser does regretContent.split(/\n---\n/, 2) -- a regex anchored on literal \n, which does not match the '\r\n---\r\n' separator produced by a CRLF .regret file (git core.autocrlf=true, the standard Windows git setting). Fixed by normalizing CRLF -> LF right after reading the file. Pattern-applied, not execution-verified (no Nim compiler locally).
Same bug family as validate_nim.sh (this PR) and the rest of this
session: content.split('\n---\n') anchored on literal LF, breaks on a
CRLF-converted .regret file (git core.autocrlf=true). Fixed by
normalizing CRLF -> LF right after reading the file. Pattern-applied,
not execution-verified (no Dart SDK locally).
|
This was referenced Jun 28, 2026
Wolfvin
added a commit
that referenced
this pull request
Jun 29, 2026
…#555) ## scripts/capture_bash.sh, scripts/validate_bash.sh — REAL execution-verified bug invoke_entry()'s argument-passing pipeline writes args via `jq -r`, then reads them back via `mapfile -t ARGS < "$args_file"`. jq.exe (Windows build) emits CRLF line endings on -r text output (same root cause as the confirmed jq-stack bug, #519/#529), and `mapfile -t` only strips the trailing `\n`, leaving every argument contaminated with a trailing `\r` (e.g. "Hello, World!" becomes "Hello, World!\r"). This silently defeats exact string comparisons in user code -- discovered via tests/bash-stack.test.js's #315 multi-input regression test, which mutates a function to special-case input #2 ("Hello, World!") and expects validate to catch it. It didn't: the live arg never matched the literal string the mutation checked for, so the mutated branch was never taken, and validate silently PASSed a real behavioral regression. Initial fix attempt (strip \r from each array element via `${ARGS[i]%$'\r'}` after mapfile) reproducibly does NOT work when nested inside this file's `output=$( { ... } )` command-substitution structure -- confirmed via isolated minimal repro that the exact same suffix-removal pattern works at top level and inside a plain `( )` subshell, but silently no-ops here. Root cause of THAT specific bash behavior is not fully understood; worked around it by stripping \r from the file via `tr -d '\r'` (through process substitution) BEFORE mapfile reads it, which does work reliably. ## tests/bash-stack.test.js — two test-infrastructure bugs (Windows-only) 1. REPO_ROOT/SCRIPTS (computed via path.resolve()/join()) get embedded unquoted into bash -c command strings throughout this file. On native Windows these are backslash-separated paths, and bash's own command- string parser treats every unquoted '\X' as an escaped literal X -- silently dropping every backslash ("C:\Users\user\foo" becomes "C:Usersuserfoo"), which then fails to resolve as a file path at all (status 127, "No such file or directory"). Fixed by normalizing both to forward slashes once at the top (valid path separator on Windows too) -- fixes every call site at once. 2. `await import(join(SCRIPTS, 'validate.js'))` passes a raw absolute path string to dynamic import(). Node's ESM loader on Windows requires a file:// URL for absolute-path imports, not a raw "c:/..." string (ERR_UNSUPPORTED_ESM_URL_SCHEME). Fixed with pathToFileURL(). ## Verification - Full manual repro outside the test suite: fresh capture -> 2/2 PASS -> mutate slugify() to special-case input #2 only -> validate -> exactly 1/2 FAIL ("bash-slugify: INPUTS[1] hash mismatch", bash-greet still PASS) -> restore -> 2/2 PASS again. - `node --test tests/bash-stack.test.js`: 13/13 pass, 0 fail (was 13 failures before this fix, all stemming from the two test-infra bugs cascading into every test in the file). These three bugs were found while investigating why a from-scratch `npm test` run on Windows showed 91 failures despite this session's direct, isolated manual verification of the Bash stack passing cleanly. Cross-checked against GitHub Actions CI (Linux, green on main) to confirm the bulk of those 91 failures are Windows-test-runner-specific artifacts (SIGINT/SIGTERM semantics, this exact path-escaping class of bug) rather than product regressions -- but this particular one (#315 multi-input silently not caught) is a genuine product bug that happened to only surface because the test assertion exercises a multi-input scenario that this session's manual spot-checks did not happen to construct.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
jq — TWO bugs, BOTH execution-verified end-to-end
capture_jq.sh/validate_jq.sh:
jq.exe(Windows build) emits CRLF line endings on-rtext output.list_jq_clusters()'s output is read viawhile IFS= read -r cluster_id, which only splits on\n— every cluster_id but the last one (command substitution strips only the final trailing newline) carries a trailing\r, so every subsequentjq -r ... select(.id == "$cluster_id")lookup matches nothing. Reproduced live:capture_jq.shfailed 5/6 clusters with "jq file not found" (the 6th — last line — worked by accident). Fixed by stripping the trailing\rright after readingcluster_id.validate_jq.sh: same root issue as the rest of this session's CRLF findings — its own
while IFS= read -r lineover the.regretfile content doesn't strip\r, so[[ "$line" == "---" ]]fails to find the separator on a CRLF-converted.regretfile.Verification (live execution)
capture_jq.sh→ 1/6 captured, 5/6 "file not found"capture_jq.sh→ 6/6 captured.regret) → 6/6 PASS.regretfiles to CRLF (sed 's/$/\r/', simulating a Windows git checkout withcore.autocrlf=true) → still 6/6 PASSgreet()("Hello, "→"Hi, ") → validate → exactly 5/6 PASS, 1/6 FAIL (jq-greet, hash + output diff both correct)greet()→ 6/6 PASS againmake, sql — pattern-applied, NOT execution-verified
while IFS= read -r line+[[ "$line" == "---" ]]pattern as jq — same fix applied. Nomakebinary available locally to run the full cycle.content.split('\n')+line === '---'— same bug, same fix family. No sqlite3 available locally to run the full cycle.