Conversation
Signed-off-by: Christoph Knittel <ck@cca.io>
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
https://github.com/rescript-lang/rescript/blob/261ca867a2950c48d22a8b050e16d2601c4850bb/.github/workflows/ci.yml#L596
This job is not actually duplicated by the earlier Run rewatch tests step: that step passes rewatch/target/release/rescript, causing suite.sh to inject the repository-built compiler and runtime, while this job installs the pkg.pr.new artifact and runs through node_modules/.bin/rescript. The remaining npm/pnpm jobs exercise only help and basic builds, so package-only regressions in the JavaScript launcher—especially watch-mode signal forwarding—or in packaged resolution for format, clean, and other subcommands will no longer be caught on any supported platform. Keep a package-mode run for those behaviors, even if the native full-suite matrix remains the primary Rewatch test.
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
rescript
@rescript/belt
@rescript/darwin-arm64
@rescript/darwin-x64
@rescript/linux-arm64
@rescript/linux-x64
@rescript/runtime
@rescript/win32-x64
commit: |
|
Developer playground preview: https://rescript-lang.github.io/rescript/dev-playground/?version=pr-8670 |
Signed-off-by: Christoph Knittel <ck@cca.io>
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: bb1a80b4b7
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| } else { | ||
| assert.ok(watcher.kill("SIGINT"), "could not signal packaged CLI"); | ||
| } |
There was a problem hiding this comment.
On every non-Windows runner, this path only sends SIGINT, but Rewatch's cleanup_before_watch_exit does not delete lib/watch.lock, and main.rs never calls drop_lock for LockKind::Watch. The watcher therefore exits while the lock still exists, causing the subsequent !existsSync(watchLock) poll to time out and fail this npm installation job on macOS and Linux. Remove the lock and await the process on all platforms, or stop asserting that signal-driven shutdown removes it.
AGENTS.md reference: rewatch/AGENTS.md:L220-L220
Useful? React with 👍 / 👎.
This branch has not been deployed
1 participant
Rewatch’s full integration suite currently runs twice on each supported platform: during the compiler build and again after the npm packages are assembled. The second run executes the same integration suite through the packaged CLI. The separate npm and pnpm installation tests already check that the packaged CLI and platform binaries can build a project.
Keep the earlier run so Rewatch changes get feedback sooner. Remove the late full-suite job; the separate npm and pnpm installation tests continue to verify that the packaged CLI and platform binaries can build a project.