Conversation
There was a problem hiding this comment.
Pull request overview
This PR adds Deno binary compilation support to enable cross-platform binary distributions of c8ctl without requiring Node.js runtime, fixing issue #36.
Changes:
- Refactored CLI entry point to support both Node.js and Deno runtimes via a shared runtime-agnostic runner
- Replaced Buffer with Uint8Array and file-based JSON loading with import attributes for cross-runtime compatibility
- Added Deno compilation configuration and GitHub Actions workflow for automated binary builds
- Updated tests to use newer SDK API patterns (ProcessDefinitionId wrappers and config-based client creation)
Reviewed changes
Copilot reviewed 13 out of 15 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
| src/cli.ts | New runtime-agnostic CLI runner extracted from index.ts, uses dynamic imports for optimization |
| src/index.ts | Simplified Node.js entrypoint that delegates to cli.ts |
| src/deno.ts | New Deno entrypoint with polyfills and Request patching for SDK compatibility |
| src/runtime.ts | Changed from readFileSync to JSON import attributes for package version |
| src/commands/help.ts | Changed from readFileSync to JSON import attributes for package version |
| src/commands/deployments.ts | Changed Buffer to Uint8Array for cross-runtime compatibility |
| src/commands/run.ts | Changed Buffer.from() to TextEncoder for cross-runtime compatibility |
| tests/integration/*.test.ts | Updated to use ProcessDefinitionId.assumeExists() and config-based client API |
| deno.json | Added Deno compilation tasks for multiple platforms |
| .github/workflows/deno-binaries.yml | Added CI workflow for building and releasing Deno binaries |
| README.md | Added documentation for Deno binary distribution and compilation |
| .gitignore | Added dist-deno/ directory |
| package-lock.json | Version bump to 2.0.0-alpha.6 and extraneous test plugin entry |
- Refactor index.ts into cli.ts (runtime-agnostic) + slim Node entrypoint - Add Deno entrypoint with SDK Request patching for compatibility - Replace Buffer/readFileSync with Uint8Array/TextDecoder/JSON imports - Add deno.json with compile tasks for Linux and macOS binaries - Add GitHub Actions workflow for building Deno binaries on release - Add demo plugin and Deno Request patching unit tests
| constructor(input: any, init?: any) { | ||
| if (init && typeof init === 'object' && 'client' in (init as any)) { | ||
| const candidate = (init as any).client; | ||
| const isLikelyDenoHttpClient = candidate && typeof candidate === 'object' && typeof candidate.close === 'function'; |
There was a problem hiding this comment.
The test implementation differs from the source code in src/deno.ts line 31. The test uses candidate && typeof candidate === 'object' while the source uses !!candidate && typeof candidate === 'object'. While functionally equivalent in this boolean context, the test should exactly mirror the source implementation to ensure accurate testing. Update the test to use !!candidate to match src/deno.ts line 31.
| const isLikelyDenoHttpClient = candidate && typeof candidate === 'object' && typeof candidate.close === 'function'; | |
| const isLikelyDenoHttpClient = !!candidate && typeof candidate === 'object' && typeof candidate.close === 'function'; |
| @@ -0,0 +1,16 @@ | |||
| // c8ctl-plugin.ts | |||
There was a problem hiding this comment.
The file comment incorrectly identifies this as a TypeScript file (c8ctl-plugin.ts) when it's actually a JavaScript file (c8ctl-plugin.js). Update the comment to reflect the actual file extension.
| // c8ctl-plugin.ts | |
| // c8ctl-plugin.js |
| - name: Generate checksums | ||
| run: | | ||
| cd dist-deno | ||
| sha256sum * > SHA256SUMS.txt |
There was a problem hiding this comment.
The checksum generation command sha256sum * > SHA256SUMS.txt will include SHA256SUMS.txt in the glob pattern on subsequent runs if the step is re-executed, which could cause issues. Additionally, the SHA256SUMS.txt file itself will be uploaded to the release, and then sha256sum will attempt to hash it along with the binaries. Consider using a more specific pattern like sha256sum c8ctl-* > SHA256SUMS.txt to only hash the actual binaries and exclude the checksum file itself.
| sha256sum * > SHA256SUMS.txt | |
| sha256sum c8ctl-* > SHA256SUMS.txt |
Fixes #36.
Signed-off-by: Joshua J Wulf joshua.wulf@camunda.com