feat: enhance CI and smoke testing with checksum verification and modular job structure#61
Closed
feat: enhance CI and smoke testing with checksum verification and modular job structure#61
Conversation
…ular job structure Signed-off-by: Austin Michne <[email protected]>
📝 WalkthroughWalkthroughThe changes update CI and smoke tooling: add SHA-256 digest to installer metadata, expand CLI smoke tests to apply and verify edits in workspace files, refactor the GitHub Actions workflow into multi-job stages with artifact handling and Java toolchain changes, and add related docs and test adjustments. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.github/scripts/smoke-installer.sh:
- Around line 85-100: Add a negative tamper test to the smoke-installer
workflow: after computing the real digest (variable digest) and building
payload, create a second test case that either corrupts the asset (modify
asset_path bytes) or sets a bogus digest (e.g., tampered_digest) in the payload,
then run the installer (install.sh) against that tampered metadata and assert it
fails and does not create the target install tree; specifically, reuse
metadata_path and asset_path variables but produce a payload where
"assets[0].digest" is the wrong sha256 and verify install.sh exits non‑zero and
the target install directory is absent before exiting.
In @.github/workflows/ci.yml:
- Around line 40-47: The CI job "Test and build portable distribution" dropped
running the kast CLI unit tests; update the build-and-test step to include the
Gradle task :kast-cli:test in the multi-line run block so the CLI module tests
execute (i.e., alongside :kast:test, :kast:portableDistZip, etc., add
:kast-cli:test) to ensure module-level CLI regressions are caught.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 9544d03c-6016-42d6-8f0b-38285c256190
📒 Files selected for processing (3)
.github/scripts/smoke-installer.sh.github/scripts/smoke-kast-cli.sh.github/workflows/ci.yml
Comment on lines
85
to
+100
| python3 - "$metadata_path" "$asset_path" <<'PY' | ||
| import hashlib | ||
| import json | ||
| import sys | ||
| from pathlib import Path | ||
|
|
||
| metadata_path = Path(sys.argv[1]) | ||
| asset_path = Path(sys.argv[2]) | ||
| digest = hashlib.sha256(asset_path.read_bytes()).hexdigest() | ||
| payload = { | ||
| "tag_name": "v0.0.0-smoke", | ||
| "assets": [ | ||
| { | ||
| "name": asset_path.name, | ||
| "browser_download_url": asset_path.as_uri(), | ||
| "digest": "", | ||
| "digest": f"sha256:{digest}", |
There was a problem hiding this comment.
This still doesn't prove checksum enforcement.
The digest is computed from the same ZIP the installer later consumes, so this path stays green even if install.sh ignores assets[0].digest completely. Please add a negative case that tampers with the digest or asset and asserts the install fails before creating the target install tree.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.github/scripts/smoke-installer.sh around lines 85 - 100, Add a negative
tamper test to the smoke-installer workflow: after computing the real digest
(variable digest) and building payload, create a second test case that either
corrupts the asset (modify asset_path bytes) or sets a bogus digest (e.g.,
tampered_digest) in the payload, then run the installer (install.sh) against
that tampered metadata and assert it fails and does not create the target
install tree; specifically, reuse metadata_path and asset_path variables but
produce a payload where "assets[0].digest" is the wrong sha256 and verify
install.sh exits non‑zero and the target install directory is absent before
exiting.
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
evals/harness/run-evals.sh (1)
264-269:⚠️ Potential issue | 🟡 MinorFail fast when an explicit
KAST_BINoverride is invalid.On Line 264, an invalid overridden
KAST_BINstill triggers a local build but keeps using the bad path. Prefer distinguishing an explicit override from default fallback and erroring early for a bad override.Proposed hardening
-KAST_BIN="${KAST_BIN:-$REPO_ROOT/kast/build/install/kast/bin/kast}" -if [[ ! -x "$KAST_BIN" ]]; then +DEFAULT_KAST_BIN="$REPO_ROOT/kast/build/install/kast/bin/kast" +KAST_BIN="${KAST_BIN:-$DEFAULT_KAST_BIN}" +if [[ ! -x "$KAST_BIN" ]]; then + if [[ "$KAST_BIN" != "$DEFAULT_KAST_BIN" ]]; then + die "KAST_BIN override is not executable: $KAST_BIN" + fi log_step "Building kast (installDist)…" (cd "$REPO_ROOT" && ./gradlew :kast:installDist --no-daemon --quiet) || die "kast build failed" + KAST_BIN="$DEFAULT_KAST_BIN" + [[ -x "$KAST_BIN" ]] || die "kast binary not found after build: $KAST_BIN" log_success "kast built" fi🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@evals/harness/run-evals.sh` around lines 264 - 269, The script currently treats an invalid KAST_BIN the same whether it was explicitly overridden or defaulted; change the logic in the block that checks KAST_BIN so that if the environment variable KAST_BIN was explicitly set (detect via test -n "${KAST_BIN+x}" or comparing to the default computed from REPO_ROOT) and the path is not executable, immediately die with a clear error via die "..." instead of attempting to build; only attempt the local build (cd "$REPO_ROOT" && ./gradlew :kast:installDist ...) when KAST_BIN was not explicitly provided and the default path "$REPO_ROOT/kast/build/install/kast/bin/kast" is missing/unexecutable, preserving log_step/log_success behavior for the build case.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.agents/wiki/sources/whats-missing.md:
- Line 18: The markdown row for `imports/optimize` contains unresolved fragment
links like `(`#4-cite-0`)` (and other `#4-cite-*` anchors referenced elsewhere) so
fix by either adding matching anchor targets in the document (e.g., HTML anchor
IDs or markdown headings matching `4-cite-0`, `4-cite-1`, etc.) or replacing
each `[4-cite-X](`#4-cite-X`)` reference with a valid existing link or citation
reference; update all occurrences of the `#4-cite-*` fragments (including the
other reported instances) to ensure each link points to a defined anchor or a
correct external/internal URL and verify navigation works for `imports/optimize`
and related table entries.
- Line 18: The table row "| `imports/optimize` | Clean up imports after edits |
Edit plan for unused/missing imports | [4-cite-0](`#4-cite-0`)" has four cells
while the header defines three; edit the row so it has exactly three cells by
moving the citation out of the row or merging it into one cell (e.g., "|
`imports/optimize` | Clean up imports after edits | Edit plan for unused/missing
imports [4-cite-0](`#4-cite-0`) |"), ensuring the table header and all rows have
the same number of pipe-separated columns.
In `@response.md`:
- Around line 69-78: The markdown has unlabeled fenced code blocks (triggering
MD040) — change the opening fences for the two blocks shown: the block that
starts with ":analysis-api:test" and the ASCII graph block that starts with "┌─
test-analysis-api" to include a language identifier (e.g., replace ``` with
```text) so both fenced blocks are labeled and satisfy markdownlint; update both
occurrences referenced in response.md.
- Around line 1-119: response.md appears to be an accidental ad-hoc review
artifact and should be removed or relocated into durable documentation; either
delete response.md from the repo or convert it into a scoped, versioned
RFC/design doc under a proper docs or rfcs directory (e.g., MOVE content to
docs/architecture/kast-smoke-rfc.md), give it a clear title, summary, and
actionable sections, and update any references (CI job names like ci.yml,
release.yml and module names kast, kast-cli, backend-standalone, analysis-api)
to be concise and maintainable so it doesn't become stale in-tree.
---
Outside diff comments:
In `@evals/harness/run-evals.sh`:
- Around line 264-269: The script currently treats an invalid KAST_BIN the same
whether it was explicitly overridden or defaulted; change the logic in the block
that checks KAST_BIN so that if the environment variable KAST_BIN was explicitly
set (detect via test -n "${KAST_BIN+x}" or comparing to the default computed
from REPO_ROOT) and the path is not executable, immediately die with a clear
error via die "..." instead of attempting to build; only attempt the local build
(cd "$REPO_ROOT" && ./gradlew :kast:installDist ...) when KAST_BIN was not
explicitly provided and the default path
"$REPO_ROOT/kast/build/install/kast/bin/kast" is missing/unexecutable,
preserving log_step/log_success behavior for the build case.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 47e24662-0a2e-42cf-b844-283c275891e1
📒 Files selected for processing (5)
.agents/wiki/sources/whats-missing.md.github/workflows/ci.ymlbackend-standalone/src/test/kotlin/io/github/amichne/kast/standalone/CacheManagerTest.ktevals/harness/run-evals.shresponse.md
🚧 Files skipped from review as they are similar to previous changes (1)
- .github/workflows/ci.yml
| | `workspace-symbol` | Find declarations by name | Substring or regex, filterable by `kind` | | ||
| | `rename` → `apply-edits` → `diagnostics` | Compiler-backed refactoring | Hash-based conflict detection, post-edit diagnostic validation | | ||
| | `semantic-insertion-point` | Where to insert new code | Offsets for class body start/end, file top/bottom, after imports | | ||
| | `imports/optimize` | Clean up imports after edits | Edit plan for unused/missing imports | [4-cite-0](#4-cite-0) |
There was a problem hiding this comment.
Repair broken citation fragment links.
These (#4-cite-*) anchors don’t resolve in this file, so readers can’t navigate to evidence references. Define matching anchors in the document or replace with valid links.
Also applies to: 67-67, 71-71, 107-107, 110-110, 111-111, 159-159, 161-161, 165-165, 167-167, 220-220, 222-222
🧰 Tools
🪛 markdownlint-cli2 (0.22.0)
[warning] 18-18: Link fragments should be valid
(MD051, link-fragments)
[warning] 18-18: Table pipe style
Expected: leading_and_trailing; Actual: leading_only; Missing trailing pipe
(MD055, table-pipe-style)
[warning] 18-18: Table column count
Expected: 3; Actual: 4; Too many cells, extra data will be missing
(MD056, table-column-count)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.agents/wiki/sources/whats-missing.md at line 18, The markdown row for
`imports/optimize` contains unresolved fragment links like `(`#4-cite-0`)` (and
other `#4-cite-*` anchors referenced elsewhere) so fix by either adding matching
anchor targets in the document (e.g., HTML anchor IDs or markdown headings
matching `4-cite-0`, `4-cite-1`, etc.) or replacing each `[4-cite-X](`#4-cite-X`)`
reference with a valid existing link or citation reference; update all
occurrences of the `#4-cite-*` fragments (including the other reported
instances) to ensure each link points to a defined anchor or a correct
external/internal URL and verify navigation works for `imports/optimize` and
related table entries.
Fix malformed table row on Line 18.
The row has 4 cells while the header defines 3 columns, so markdown renderers/lints will drop or misplace content. Keep the citation outside the table row (or move it into one of the 3 cells).
🧰 Tools
🪛 markdownlint-cli2 (0.22.0)
[warning] 18-18: Link fragments should be valid
(MD051, link-fragments)
[warning] 18-18: Table pipe style
Expected: leading_and_trailing; Actual: leading_only; Missing trailing pipe
(MD055, table-pipe-style)
[warning] 18-18: Table column count
Expected: 3; Actual: 4; Too many cells, extra data will be missing
(MD056, table-column-count)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.agents/wiki/sources/whats-missing.md at line 18, The table row "|
`imports/optimize` | Clean up imports after edits | Edit plan for unused/missing
imports | [4-cite-0](`#4-cite-0`)" has four cells while the header defines three;
edit the row so it has exactly three cells by moving the citation out of the row
or merging it into one cell (e.g., "| `imports/optimize` | Clean up imports
after edits | Edit plan for unused/missing imports [4-cite-0](`#4-cite-0`) |"),
ensuring the table header and all rows have the same number of pipe-separated
columns.
Comment on lines
+1
to
+119
| Now I have enough to give a thorough review. Here it is: | ||
|
|
||
| --- | ||
|
|
||
| ## CI & Smoke Review | ||
|
|
||
| ### The core architectural problem | ||
|
|
||
| The `kast` module is the glue: it depends on both `:kast-cli` and `:backend-standalone` and produces the one portable zip that CI tests. This means the two units are **co-packaged and never independently validated**. Every smoke test exercises them as a single artifact. If you want atomic, independently releasable units, this coupling is the root issue to resolve — everything else follows from it. | ||
|
|
||
| --- | ||
|
|
||
| ### Flaws in the smoke scripts | ||
|
|
||
| **1. Installer smoke bypasses checksum verification (`smoke-installer.sh:99`)** | ||
|
|
||
| ```python | ||
| "digest": "", | ||
| ``` | ||
|
|
||
| An empty string means the installer's SHA-256 verification path is never exercised. The smoke gives false confidence: it proves the install *flow* works but not the *security-critical* checksum path. The fix is to compute the actual SHA-256 of the test zip and populate `digest` before running the installer: | ||
|
|
||
| ```python | ||
| import hashlib | ||
| digest = hashlib.sha256(asset_path.read_bytes()).hexdigest() | ||
| payload = { ..., "assets": [{ ..., "digest": f"sha256:{digest}" }] } | ||
| ``` | ||
|
|
||
| **2. Native binary is never tested in CI** | ||
|
|
||
| `ci.yml` builds `:kast:portableDistZip` but **GraalVM native compilation is not in the CI build** — it's only in `release.yml`. The native binary (`bin/kast`) is therefore absent from the portable zip that CI smokes. What CI actually validates is the JVM fallback path in the wrapper script. You can ship a broken native binary and CI will never catch it. | ||
|
|
||
| Mitigation options: add `:kast-cli:nativeCompile` to CI with `--no-fallback` disabled for speed, or have a separate CI job that at least confirms the native binary compiles and runs `--help`. The `kast` module's `syncPortableDist` already skips native if `jvmOnly` property is set — use that property explicitly in CI rather than implicitly relying on it. | ||
|
|
||
| **3. `analysis-server` is never independently smoked** | ||
|
|
||
| Every smoke call goes through the kast CLI → daemon subprocess path. There's zero validation that `backend-standalone` starts and responds to RPC in isolation. Since you want the server to be startable as a standalone background process, there should be a smoke that: | ||
|
|
||
| 1. Invokes `backend-standalone --transport=unix-socket --socket-path=<tmp>` directly | ||
| 2. Sends a `capabilities` RPC request against the socket | ||
| 3. Asserts the response | ||
|
|
||
| Without this, the "decoupled runnable" story is untestable in CI. | ||
|
|
||
| **4. Rename smoke validates planning, not edit application** | ||
|
|
||
| The smoke asserts `rename["edits"]` contains the right files but never calls `kast apply-edits` to verify those edits land on disk correctly. The mutation path is the most dangerous — if the apply-edits command regresses, CI won't catch it. | ||
|
|
||
| **5. Stop-polling is fragile** | ||
|
|
||
| ```bash | ||
| for _ in $(seq 1 30); do | ||
| if ! find "$instance_dir" -name '*.json' -print -quit | grep -q .; then | ||
| break | ||
| fi | ||
| sleep 1 | ||
| done | ||
| ``` | ||
|
|
||
| This can either flake (daemon took 31 seconds to clean up) or silently pass when the daemon didn't actually stop (loop exits after 30s regardless). Replace the sleep loop with a proper wait — the `workspace stop` command should block until the descriptor is gone, or the smoke should fail immediately if descriptors remain after a timeout. | ||
|
|
||
| --- | ||
|
|
||
| ### Flaws in `ci.yml` structure | ||
|
|
||
| **6. Monolithic `cli-smoke` job** | ||
|
|
||
| All of these run in a single sequential job: | ||
| ``` | ||
| :analysis-api:test | ||
| :analysis-server:test | ||
| :backend-standalone:test | ||
| :backend-intellij:test | ||
| :backend-intellij:buildPlugin | ||
| :backend-intellij:verifyPluginStructure | ||
| :kast:test | ||
| :kast:portableDistZip | ||
| ``` | ||
|
|
||
| `backend-intellij:test` pulls the IntelliJ distribution and takes significant time. A `backend-intellij` test failure blocks the CLI smoke entirely. These should be split into independent parallel jobs with explicit `needs:` wiring: | ||
|
|
||
| ``` | ||
| ┌─ test-analysis-api ─────────┐ | ||
| │ ├─> smoke-kast-cli | ||
| ├─ test-analysis-server ──────┤ | ||
| │ │ | ||
| ├─ build-intellij-plugin ─────┘ | ||
|
|
||
| └─ eval-agent-routing (needs: smoke-kast-cli) | ||
| ``` | ||
|
|
||
| **7. GraalVM setup is wasted for the JVM-only smoke** | ||
|
|
||
| CI sets up `graalvm/setup-graalvm` then builds a portable zip without the native binary (because `nativeCompile` isn't in the task list). GraalVM setup adds minutes for no benefit. Use `actions/setup-java` with Temurin in CI and reserve GraalVM for the release workflow. | ||
|
|
||
| **8. `eval-agent-routing` builds `kast` redundantly** | ||
|
|
||
| The eval job runs `./gradlew :kast:installDist` after `cli-smoke` already built the full portable zip. If the eval job received the built distribution as an artifact from `cli-smoke`, you'd avoid rebuilding from scratch. | ||
|
|
||
| --- | ||
|
|
||
| ### What true atomic releases require | ||
|
|
||
| | Unit | Today | Needed | | ||
| |------|-------|--------| | ||
| | `kast-cli` | Co-packaged with `backend-standalone` via `kast` module | Own release zip (native binary + embedded skill). Needs a way to locate/start a separately-installed backend. | | ||
| | `analysis-server` | Bundled inside `kast` distribution; never independently released | Own `backend-standalone-<version>-<platform>.zip` release asset. Own smoke. Own install path in `install.sh`. | | ||
| | `analysis-api` | `kast.kotlin-library` plugin only — no `maven-publish` visible in build file | Apply `maven-publish`, publish to GitHub Packages or Maven Central. | | ||
| | `backend-intellij` | Already effectively atomic (own zip) | Already correct. Verify the plugin zip release is independently installable via JetBrains toolbox/settings without `kast-cli` present. | | ||
|
|
||
| The `kast` module today acts as a meta-package. To decouple: | ||
|
|
||
| 1. **`kast-cli` release**: Drop the `:backend-standalone` dependency from the `kast` module (or create a new `kast-cli-dist` module). The CLI already supports `internalDaemonRunner = null` and can discover an external daemon. Make the external backend path first-class rather than an afterthought. | ||
|
|
||
| 2. **`backend-standalone` release**: Add its own release job to `release.yml` (parallel with the CLI builds). Add a standalone smoke to `ci.yml`. The installer already has the infrastructure for multiple assets — extend it to handle `--components=standalone` independently. | ||
|
|
||
| 3. **`analysis-api` Maven publish**: Add `maven-publish` to the `kast.kotlin-library` plugin or specifically to `analysis-api/build.gradle.kts`. Coordinate versioning so the API version is independently consumable by plugin authors. | ||
|
|
||
| The most impactful single change is **splitting `kast` module's dependencies** so `kast-cli` can be built and distributed without `backend-standalone`. That one structural change unlocks independent versioning, independent smoke testing, and independent release for both units. No newline at end of file |
There was a problem hiding this comment.
This file looks like an accidental review artifact, not durable repo documentation.
The content is written as an ad-hoc conversational review and is likely to go stale quickly in-tree. Recommend removing it from the repo (or replacing it with a scoped, objective design/RFC doc if that was the intent).
🧰 Tools
🪛 markdownlint-cli2 (0.22.0)
[warning] 69-69: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
[warning] 82-82: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@response.md` around lines 1 - 119, response.md appears to be an accidental
ad-hoc review artifact and should be removed or relocated into durable
documentation; either delete response.md from the repo or convert it into a
scoped, versioned RFC/design doc under a proper docs or rfcs directory (e.g.,
MOVE content to docs/architecture/kast-smoke-rfc.md), give it a clear title,
summary, and actionable sections, and update any references (CI job names like
ci.yml, release.yml and module names kast, kast-cli, backend-standalone,
analysis-api) to be concise and maintainable so it doesn't become stale in-tree.
Comment on lines
+69
to
+78
| ``` | ||
| :analysis-api:test | ||
| :analysis-server:test | ||
| :backend-standalone:test | ||
| :backend-intellij:test | ||
| :backend-intellij:buildPlugin | ||
| :backend-intellij:verifyPluginStructure | ||
| :kast:test | ||
| :kast:portableDistZip | ||
| ``` |
There was a problem hiding this comment.
Add language identifiers to fenced code blocks to satisfy markdownlint.
Line 69 and Line 82 use unlabeled fenced blocks, which triggers MD040.
Proposed markdownlint fix
-```
+```text
:analysis-api:test
:analysis-server:test
:backend-standalone:test
:backend-intellij:test
:backend-intellij:buildPlugin
:backend-intellij:verifyPluginStructure
:kast:test
:kast:portableDistZip@@
- +text
┌─ test-analysis-api ─────────┐
│ ├─> smoke-kast-cli
├─ test-analysis-server ──────┤
│ │
├─ build-intellij-plugin ─────┘
└─ eval-agent-routing (needs: smoke-kast-cli)
Also applies to: 82-90
🧰 Tools
🪛 markdownlint-cli2 (0.22.0)
[warning] 69-69: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@response.md` around lines 69 - 78, The markdown has unlabeled fenced code
blocks (triggering MD040) — change the opening fences for the two blocks shown:
the block that starts with ":analysis-api:test" and the ASCII graph block that
starts with "┌─ test-analysis-api" to include a language identifier (e.g.,
replace ``` with ```text) so both fenced blocks are labeled and satisfy
markdownlint; update both occurrences referenced in response.md.
| - name: Test and build portable distribution | ||
| run: > | ||
| ./gradlew | ||
| ./gradlew -PjvmOnly |
Contributor
There was a problem hiding this comment.
🔴 JVM-only build breaks smoke-installer.sh: installer expects native binary absent from distribution
The CI build step now uses -PjvmOnly (line 42), which produces a portable distribution without bin/kast (the native binary is only included when jvmOnly property is absent — see kast/build.gradle.kts:87-92). However, the downstream "Smoke installer" step (line 136) runs smoke-installer.sh, which invokes install.sh without --jvm-only. Since install.sh defaults jvm_only="false" (install.sh:424), it checks [[ -f "${release_dir}/bin/kast" ]] at install.sh:653 and dies with "Installed archive did not contain the kast native binary". Even if that check were bypassed, smoke-installer.sh:125 independently asserts [[ -x "${installed_root}/bin/kast" ]]. Both checks fail because the JVM-only zip never contains bin/kast.
The two failing assertions
install.sh:652-653 (reached first):
if [[ "$jvm_only" != "true" ]]; then
[[ -f "${release_dir}/bin/kast" ]] || die "Installed archive did not contain the kast native binary"smoke-installer.sh:125 (would also fail):
[[ -x "${installed_root}/bin/kast" ]] || die "Installed kast native binary is missing from ${installed_root}/bin"Prompt for agents
The build step uses -PjvmOnly to skip native compilation (since GraalVM was replaced with Temurin), but the smoke-installer.sh test still expects a non-JVM-only distribution. There are two places that need fixing:
1. smoke-installer.sh line 117: The /bin/bash -c invocation of install.sh needs --jvm-only appended, e.g.: /bin/bash -c "$installer_content" -- --jvm-only
2. smoke-installer.sh line 125: The assertion [[ -x "${installed_root}/bin/kast" ]] needs to be removed or made conditional on whether the distribution is JVM-only.
Alternatively, if the intent is to test the full native path in CI, revert to GraalVM setup and remove -PjvmOnly. But given the deliberate switch to Temurin, the smoke-installer.sh should be updated to match the JVM-only distribution.
Was this helpful? React with 👍 or 👎 to provide feedback.
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
1 participant
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.
This pull request refactors the CI workflow to improve build and test separation, artifact handling, and platform coverage. It introduces a portable distribution artifact, splits jobs for better clarity and caching, and enhances smoke testing for the CLI. Additionally, it updates the installer metadata to include a SHA256 digest and improves CLI smoke tests to verify file edits after applying refactors.
CI Workflow Refactoring and Artifact Handling:
build-and-test, with improved separation between building/testing the main project, the IntelliJ plugin, and CLI smoke tests. The workflow now builds a portable distribution artifact for use in downstream jobs, and replacesgraalvm/setup-graalvmwithactions/setup-javausing the Temurin distribution. [1] [2] [3]test-intellij-pluginfor building and verifying the IntelliJ plugin, and splits smoke testing into a separatesmoke-kast-clijob for both Ubuntu and macOS runners, improving platform coverage and job clarity.eval-agent-routingjob to depend on the newsmoke-kast-clijob and uses the portable distribution artifact, ensuring the correct binary is tested. [1] [2]Installer and Metadata Improvements:
CLI Smoke Test Enhancements:
Summary by CodeRabbit
Tests
Chores
Documentation