Repository navigation
fix: stabilize PR CI (node-gyp from lockfile, pipe hang, flaky tests) - #21
Merged
Merged
Conversation
Native packages (node-pty, ssh2, tree-sitter-json) build with node-gyp during install. With none resolvable, bun ran `bun x node-gyp`, pulling an unpinned latest release on every CI run, outside the lockfile and the 3-day minimumReleaseAge, and that install occasionally failed. A pinned root devDependency puts node-gyp in node_modules/.bin, which bun uses for lifecycle scripts instead. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
pipe() unpipes and pauses the source once the target's stdin closes. A paused stdout never reads EOF, so the local source process never emitted `close` and pipeBetweenServers hung whenever the target exited before the source output was consumed. This made the "reports a target failure with its stderr" test time out intermittently in CI. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The backup script runs `docker exec | rclone rcat`, so both sides start concurrently and their order in the call log varies. Keep `docker ps` first and `rclone deletefile` last, but compare the pipeline pair as a set. The Better Auth schema check and the cloud queue tests import the whole auth/API module graph from a fresh registry inside the timed test/hook, which exceeded the default timeouts on a busy machine. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
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
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.
What changed
1. node-gyp now comes from the lockfile (
package.json,bun.lock)node-gyp@13.1.0is a pinned root devDependency (picked by bun with the 3-day release age).Without a resolvable node-gyp, bun's lifecycle runner puts a shim on
PATHthat runsbun x --silent node-gyp. That pulled an unpinned latest node-gyp on every CI run, outside the lockfile and the 3-dayminimumReleaseAge. One run failed withCannot find module 'exponential-backoff'from the temporary bunx install. With the pin,node_modules/.bin/node-gypis on the lifecyclePATHand bun uses it.The lockfile diff only adds node-gyp and its dependencies. A few hoisted entries (
abbrev,chownr,isexe,which,nopt,yallist) switch to the newer majors node-gyp needs. Their existing consumers keep their old versions as nested entries.bun auditreports nothing new.Dockerfiles: unchanged. They still
npm install -g node-gyp@13.0.1. The fullbun install --frozen-lockfilein the build stage now uses the lockfile copy.bun install --productionskips dev dependencies, so if it reruns native scripts it falls back to the global 13.0.1, as it does today.2.
pipeBetweenServerscould hang (packages/server/src/utils/process/remoteStream.ts)When the target's stdin closes,
Readable.pipe()unpipes and pauses the source. A paused child stdout never reads EOF, so the local source process never emitsclose, andPromise.all([from.exit, targetExit])waits forever. Whether the test hit this depended on whether the target exited before the source's output was consumed. That made "reports a target failure with its stderr" hit the 5000 ms timeout now and then. The fix unpipes and resumes the source when the target's stdin closes or the target exits, so the source drains and reports its exit.This affects production code paths too (cross-server pipes where the target exits early). It is not only a test problem.
New regression test: "drains the source when the target exits before reading it". It times out at 5000 ms without the fix and passes with it.
3. Backup script test asserted a non-guaranteed order (
backup-failure-output.test.ts)docker exec … | rclone rcat …starts both sides concurrently. The assertion still checks that the call log has the exact length (each command runs once), thatdocker pscomes first and thatrclone deletefilecomes last. Only the pipeline pair is compared as a set. The stderr, log and cleanup assertions are unchanged.4. Timeouts for two module-loading tests
better-auth-schema-compatandcloud-queue-availabilityimport the whole auth/API module graph from a fresh registry (vi.resetModules()+ dynamic import) inside the timed test orbeforeAll. That takes ~1.5 s on an idle machine and went over the 5 s test and 10 s hook defaults in 1 of 4 local full-suite runs under load. The cost is inherent, so those two now have a 60 s timeout. No logic changed.env-file-literals(already 60 s, and it needs Docker) anddb-backup-restore-injection(synchronous shell stubs with per-pid paths) showed no timing races, so they are left unchanged.Release notes
None. node-gyp 13.1.0 has the same dependency list as 13.0.2, including
exponential-backoff. The CI failure came from the transientbun xinstall, not from the release itself.Verification
remote-stream.test.ts,backup-failure-output.test.ts) ran 30 times in a row twice, once on an idle machine and once alongside a fullbun run typecheck. 60/60 passed.vitest runin apps/dokploy: everything passes exceptapplication.real.test.ts, which needs Docker and fails locally as expected.bun run typecheck: all 4 workspaces pass.bunx biome checkon the changed files: clean.bun audit --audit-level=moderate --ignore GHSA-vfj7-8cjw-p6xm: no vulnerabilities.bun install --frozen-lockfilein a copy with only manifests and the lockfile (no node_modules):node_modules/.bin/node-gypis v13.1.0, and the native builds loggyp info using node-gyp@13.1.0fromnode_modules/.bun/node-gyp@13.1.0/…. Nobun xcall.PATHresolvesnode-gypto bun's temp shim (bun x --silent node-gyp $@). With the pin it resolves tonode_modules/.bin/node-gyp.Needs a manual check
bun installstep logs should show node-gyp fromnode_modules/.bun/node-gyp@13.1.0and no/tmp/bunx-*-node-gyp@latest. On Linux, node-pty has no prebuilds, so it always builds with node-gyp. Locally (macOS) it used a prebuild.🤖 Generated with Claude Code