sync: upstream just-bash@2.14.4 (manual resolve)#7
Conversation
…labs#206) The awk lexer emitted a NEWLINE token unconditionally when it saw `\n`, even when the previous token was one of the continuation-allowing tokens POSIX awk specifies (`,`, `{`, `&&`, `||`, `?`, `:`, `do`, `else`, `if`, `while`). Common multi-line idioms like printf "%s=%d\n", $1, $2 (comma at end-of-line followed by indented args on the next line) parsed as two separate statements with a stray NEWLINE in the middle, surfacing as "Unexpected token: NEWLINE" — even though gawk, mawk, and the BSD one-true-awk all accept this form. The lexer already tracks `lastTokenType` as an instance property, so the fix is a small inject in `nextToken`: when the next character is `\n` and `lastTokenType` is in `CONTINUES_ACROSS_NEWLINE`, swallow the newline and recurse to the next real token instead of emitting one. Adds eight regression tests covering each continuation-allowing token plus the TSV → SQL INSERT printf idiom that motivated the fix.
`parseArgs` allowed `-` as the first non-flag arg but then fell into
the generic else branch that sets `result.scriptFile = arg`, so
`execute()` tried to open a file literally named `-` and returned
`python3: can't open file '-': [Errno 2] No such file or directory`
(exit 2). This breaks every CPython-conventional invocation that
passes the program on stdin via the explicit `-` sentinel:
python3 - # interactive / piped stdin
python3 - <<'EOF' ... EOF # heredoc
echo 'print(1)' | python3 - # piped stdin
Per `man python3`: `-` means "read program from standard input."
Carry `-` through `parseArgs` as a sentinel (still capturing trailing
scriptArgs), and add a branch in `execute()` that routes the sentinel
to `ctx.stdin` with `argv[0] = "-"` (matching CPython). Empty stdin
runs an empty program and exits 0, matching CPython's non-interactive
behavior when no program is supplied.
Adds four regression tests covering the dash form via pipe, the dash
form via heredoc, trailing-arg propagation into `sys.argv`, and the
empty-stdin case.
Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…#190) The sqlite3 command was broken when consumed via the published npm tarball: `findWorkerPath()` falls through to `<chunks>/worker.js` (the python3 worker, which `build:worker` placed there), causing every sqlite3 invocation to fail with "Expected ArrayBuffer for the first argument" on protocol mismatch. The bug was invisible in this repo because `tsc` emits `dist/commands/sqlite3/worker.js` locally before the buggy fallback is reached, but the `files` allowlist excluded that path from the tarball. Mirror the js-exec worker pattern: bundle the sqlite3 worker, place it at `dist/commands/sqlite3/worker.js`, `dist/bin/chunks/sqlite3-worker.js`, and `dist/bundle/chunks/sqlite3-worker.js`, and ship the first one in `files`. Update `findWorkerPath()` to prefer the uniquely-named `sqlite3-worker.js` chunk and gate the bare `<currentDir>/worker.js` lookup on the directory actually being `commands/sqlite3` so the silent fallback to a wrong-command worker can't return. Add a unit test that pins the resolution order and would have caught the original bug. Also wire sqlite3 into `check-worker-sync.js` so the bundled worker stays in sync with `worker.ts`, and exclude the generated artifact from biome / banned-patterns lints (matching the existing python3 / js-exec treatment). Verified end-to-end by `npm pack` + install in a clean consumer: sqlite3 now executes successfully without any post-install patching. Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Resolutions: - packages/just-bash/package.json: keep upstream's build:worker (full src/->dist + bin/chunks + bundle/chunks copy chain, matching the python3/js-exec pattern); dedupe duplicate dist/commands/sqlite3/worker.js files entry. - packages/just-bash/src/commands/awk/lexer.ts: take upstream's CONTINUES_ACROSS_NEWLINE check (strict superset of HEAD's COMMA-only case; comment block already described the broader behavior).
|
|
![flowglad-review-service[bot]](https://avatars.githubusercontent.com/u/178508461?s=60&v=4){kind=small}
Review SummaryThe PR is a clean upstream sync (2.14.3 → 2.14.4) with well-handled conflict resolutions. The core feature additions — awk newline continuation, sqlite3 dot-commands, worker resolution fix, python3 Two must-fix findings:
One should-fix finding:
3 comments posted · Model: |
sqlite3: dot-command errors are now appended to stdout (not stderr) so
they interleave with SQL results in script order, matching how inline
SQL errors are reported by the result loop. Avoids the surprise of a
dot-command error appearing on a different channel than a SQL error in
the same script. The -bail path still emits to stderr.
dns-pin: PinnedAddress now carries every preflight-validated address
(addresses: { address, family }[]) and patchedLookup filters by the
requested family at call time. Previously a single (address, family)
pair was pinned, so an IPv6-first dual-stack hostname would trigger
ENOTFOUND when undici later asked for IPv4 explicitly. ENOTFOUND now
fires only when no pinned address satisfies the requested family.
fetch: checkAllowed returns the full validated list instead of just
addresses[0], so both families survive into the pinning context.
Each address still passes the same isPrivateIp gate, so pinning more
addresses doesn't widen the threat surface.
| @@ -13,7 +13,7 @@ | |||
|
|
|||
There was a problem hiding this comment.
[MUST FIX] packages/just-bash/src/commands/sqlite3/sqlite3.ts:614 — When dotError is set and there is SQL that runs (the non-bail, non-empty-SQL path), the dot-command error is appended to stdout after the SQL results (line ~749 in context: stdout += \${dotError}\n`). But the -bailearly-return path (line 614:return { stdout: "", stderr: `${dotError}\n`, exitCode: 1 }) puts the dot-command error in **stderr**. This means the channel where dotErroris reported is different depending on whether-bail` is active:
- With
-bail:stderrcontains the error,stdoutis empty. - Without
-bail(non-empty SQL):stderris empty, the error is instdoutafter the SQL results.
A caller that inspects stderr to detect errors (the normal pattern) will miss dot-command errors in the non-bail case entirely, seeing only exitCode: 1 with empty stderr. The test fixtures updated in this PR already codify the stdout-based path (expect(result.stdout).toBe("Error: ...\n") with expect(result.stderr).toBe("")), which entrenches the inconsistency.
Suggestion: route dotError to stderr consistently (both bail and non-bail), matching the error channel already used for SQL errors surfaced by the worker (those go to stderr: \Error: ...`\nunder-bail, or to stdoutfor non-bail inline errors). The test expectations insqlite3.dot-commands.test.tsthat were updated in this PR to usestdoutshould be reverted to usestderr`.
Reviewed at 96ab94b
Review SummaryThis PR is a well-structured upstream sync of just-bash@2.14.4 with three Flowglad patches (sqlite3 worker, awk comma-continuation, jq control chars). The major upstream changes include: OSC-8 XSS fixes in the website terminal, DNS rebinding mitigation via One must-fix finding: the All other areas reviewed are correct: DNS-pinning (AsyncLocalStorage scoping, family filtering, dual-stack, ENOTFOUND on mismatch), rg
1 comment posted · Model: |
3 participants
Summary
Manual upstream sync to
just-bash@2.14.4after the scheduledSync upstreamaction failed:gh label create→ 403 againstvercel-labs/just-bash(fixed in ci(sync-upstream): pin gh CLI to fork repo #6).GITHUB_TOKENbecause the merge contains.github/workflows/release.ymlchanges — a platform-level restriction that can't be granted via YAMLpermissions:. Pushed manually via my own creds here so we can land this tag's sync today; the workflow-permission limitation is a separate decision (PAT/App vs. excluding workflow files from sync).Conflict resolutions
packages/just-bash/package.jsonbuild:workerscript: took upstream's version. It follows the samesrc/→dist/...+dist/bin/chunks/...+dist/bundle/chunks/...copy chain as the python3 and js-exec workers, and thesrc/commands/sqlite3/worker.jsartifact is already gitignored. Flowglad's prior version (--outfile=dist/commands/sqlite3/worker.jsonly) wouldn't populate the bundled chunks paths.filesarray: deduped — both sides added"dist/commands/sqlite3/worker.js", the auto-merge produced two copies. Kept one.packages/just-bash/src/commands/awk/lexer.ts—nextTokennewline handling: took upstream's version.CONTINUES_ACROSS_NEWLINE(defined upstream at line 148) is a strict superset of HEAD's COMMA-only check, and the existing comment block at the call site already described this broader behavior (commas,{,&&,||,?,:,do,else,if,while).Other notable merge content
release.yml: upstream pinned the three third-party actions to full commit SHAs (security hardening). Auto-merge cleanly preserved flowglad's Node 24 + OIDC customizations alongside the SHA pins.src/network/dns-pin*,src/commands/rg/rg-parser-threads.test.ts,src/commands/sqlite3/sqlite3.worker-resolution.test.ts+sqlite3.writeback-edge-cases.test.ts,src/commands/xan/moonblade-parser.test.ts. No flowglad-side changes touched these areas.Validation
pnpm install --frozen-lockfile— lockfile up to datepnpm --filter just-bash build— cleanpnpm --filter just-bash typecheck— cleanpnpm --filter just-bash exec vitest run src/commands/sqlite3/sqlite3.test.ts src/commands/python3/python3.optin.test.ts— 23/23 passedgit add -f packages/just-bash/distcommitted (worker chunks land as expected, including newsqlite3-worker.jsindist/bin/chunks/anddist/bundle/chunks/)After merge
Cut the next Flowglad package-root tag with
pnpm flowglad:tag --tag v2.14.4-fgp.<n> --push.🤖 Generated with Claude Code
Need help on this PR? Tag
@codesmithwith what you need.