* fix(cli): do not report success when sync push drops a script metadata change
`sync push` skipped every added `.script.yaml` / `.script.json` / `.script.lock`
on the assumption that the sibling content file in the same group carried the
deploy. When the content file was not in the changeset — e.g. filtered out by
`excludes` — nothing was sent to the remote, yet the push still printed
"Done! All N changes pushed" and exited 0, so CI gating on the exit code went
green on a deploy that never happened.
Route those changes through `handleScriptMetadata` (as the "edited" branch
already does), which resolves the content file from disk and deploys it. The
deploy stays idempotent via `alreadySynced`, and a metadata file with no content
file now fails the push instead of being counted as pushed.
Fixes WIN-2254
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* fix(cli): treat only the module entry point as script metadata
`handleScriptMetadata` / `findContentFile` matched any `script.yaml`,
`script.json` or `script.lock` anywhere under a `__mod/` tree, so a module file
nested deeper (e.g. `f/foo__mod/config/script.yaml`) was mistaken for the
script's own metadata. Gate on `isModuleEntryPoint`, which requires the file to
sit directly under `__mod/`.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* fix(cli): reject non-metadata paths in findContentFile
Every candidate-path replacement in findContentFile is a no-op on a path that
is neither flat `*.script.{yaml,json,lock}` nor a module entry point, so the
input resolved to itself and the caller got a "more than one candidate found"
list of 25 copies of the same path. Reject those inputs up front.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* fix(cli): report an unpushable script as a failure instead of aborting the push
Throwing out of the apply loop for a metadata file with no script file left the
push partially applied: every change queued behind it was dropped, including
ones with nothing wrong. Collect these into a failed list, log each one, keep
applying the rest, and report `N of M changes pushed; K failed` with a non-zero
exit (`success: false` plus a `failed` array under --json-output).
Only the content-resolution failure is soft, via MissingScriptContentFileError.
A deploy the remote rejected still aborts, since it says nothing about whether
the remaining changes are safe to apply.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* fix(cli): let stdout drain before sync push returns a failure exit code
process.exit does not wait for a pending piped stdout write, so a --json-output
push with enough changes was cut off at the 64KiB pipe buffer, handing CI
consumers unparseable JSON. Set process.exitCode instead and return normally,
matching how main.ts already reports failures.
Reproduced with a 1904-change push: process.exit truncated the result at exactly
65536 bytes; process.exitCode emits all 432KiB and still exits 1.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* fix(cli): treat an ambiguous script file as a recoverable push failure
findContentFile has two ways to fail to pair a metadata file with a script
file, and only the "none found" one threw the class sync push catches. Two
script files for the same name (a .ts and a .py both being in exts) therefore
still aborted the whole push, dropping every change queued behind it — the
failure mode failedChanges exists to prevent, newly reachable now that an added
.script.yaml reaches findContentFile at all.
Both branches now throw the same class, renamed to
UnresolvableScriptContentFileError since it no longer only covers a missing
file, and the ambiguous case gets a message that names the clashing files.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
* fix(cli): separate unit tests from integration tests and fix test cleanup
- Rename 14 non-backend test files to *_unit.test.ts convention
- Add UNIT_ONLY env var guard in setup.ts to skip cargo build/backend startup
- Add test:unit and test:integration scripts to package.json
- Use setsid on Linux for process group management so stop() kills both
cargo and the windmill child process
- Fix exit handler to kill process group instead of just the direct child
- Add cleanupStaleTestResources() to drop orphaned windmill_test_* databases
and kill orphaned backend processes on startup
- Rewrite TESTING.md with current bun-based instructions
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
* fix(cli): fix process group approach - kill by db name instead of setsid
The setsid approach didn't work because setsid forks, making the PID
we get from Bun.spawn ephemeral. Instead, kill orphaned windmill child
processes by matching our unique database name in /proc/pid/environ.
Also add afterAll hook in setup.ts so full async cleanup (process kill
+ database drop) runs when all tests complete normally, not just on
SIGINT/SIGTERM.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
* fix(cli): address PR review feedback
- Remove duplicate cleanupStaleTestResources() call in getTestBackend()
(already called in setup.ts)
- Add regex guard on database names before SQL interpolation
- Extract shared killWindmillProcessesByEnvMatch() helper to deduplicate
process-killing logic
- Remove redundant test:integration script (test already runs everything)
- Flip setup.ts to if/else pattern for readability
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Ruben Fiszel
centdix