ci(kai-ci): always build from the newest tags, and fix the SIGPIPE - #92
Conversation
Two things, both from the first real run. REMOVE THE REF INPUTS. Four boxes you had to fill in, and the answer was always the same. There is nothing to type now: each module resolves to its own newest released tag. A pin is described by version — "kai-cli v0.35.65 on kai-engine v0.6.55" — so the build works in versions too, and a branch tip is not something anyone can point at afterwards, which is the whole objection this workflow exists to answer. Building from a branch is still possible locally with deploy/kai-ci/build.sh; it is just not what production images are made from. The form is one dropdown, defaulting to bumping both pins. FIX THE SIGPIPE. `git ls-remote ... | head -1` makes head close the pipe while git is still writing; git takes SIGPIPE and `set -o pipefail` turns that into exit 141. That is how the first run died, before it had checked anything out. It is a RACE, not a certainty, and that is the part worth recording: the same command passed when I tested it locally, because that tag list was small enough for git to finish first. A test that passes by luck is indistinguishable from one that passes. Reading the output into a variable and slicing the first line in bash removes the race rather than making it rarer. The error path around it was broken too: under `set -e` a failing command substitution killed the step silently, so the message naming the repo without a tag never printed. It is an `if !` now. Verified by extracting the step's script from the YAML and running it under bash with the runner's env — the real script, not something shaped like it.
|
Warning Review limit reachedNext included review available in 33 minutes. View limit detailsLimit details: You’ve used the included review currently available. You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Review configuration: ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Advanced Run ID: 📒 Files selected for processing (1)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
| Filename | Overview |
|---|---|
| .github/workflows/build-kai-ci.yml | The tag-resolution and failure-handling changes are sound, with only malformed generated PR wording on the newly common both path. |
Prompt To Fix All With AI
### Issue 1
.github/workflows/build-kai-ci.yml:43
**Default produces malformed PR wording**
With `both` as the default, the pin job interpolates that value into text such as `pin the both image`, making the automatically generated kai-server pull request confusing.
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.Reviews (1): Last reviewed commit: "ci(kai-ci): always build from the newest..." | Re-trigger Greptile
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| type: choice | ||
| default: none | ||
| options: [none, agent, review, both] | ||
| default: both |
There was a problem hiding this comment.
With both as the default, the pin job interpolates that value into text such as pin the both image, making the automatically generated kai-server pull request confusing.
Prompt To Fix With AI
This is a comment left during a code review.
Path: .github/workflows/build-kai-ci.yml
Line: 43
Comment:
**Default produces malformed PR wording**
With `both` as the default, the pin job interpolates that value into text such as `pin the both image`, making the automatically generated kai-server pull request confusing.
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
![kaicontext[bot]](https://avatars.githubusercontent.com/in/4190771?s=60&v=4){kind=small}
There was a problem hiding this comment.
Kai review
Kai Summary
Read through this one. 2 things worth your eyes before it merges. 👇
Where I'd land: 5/5 — ready to merge.
Review — kai-cli, bdadfcf — .github/workflows/build-kai-ci.yml
I read the diff and the full workflow file, and grepped the whole repo for every removed input name. I could not read kai-server (separate repo) to confirm the pin-PR targets exist, but that path is unchanged by this commit, so the gap doesn't bear on the change. What's here is solid.
The change does three things: removes the four per-module *_ref workflow-dispatch inputs (production images now always build from each repo's newest v* semver tag), reorders the pin dropdown so both is the default and first option, and rewrites the latest_tag helper to avoid a SIGPIPE race plus fix a silent-failure error path. All three are correct.
SIGPIPE / error-path fix. The new latest_tag reads git ls-remote into a variable and slices the first line in bash (first="${out%%$'\n'*}"), which eliminates the | head -1 pipe that caused exit 141 under set -o pipefail. The diagnosis is right: SIGPIPE on a closed pipe is the classic race, and under set -e a failing command substitution would have killed the step before the error message could print. The if ! want="$(latest_tag "$m")" || \[ -z "$want" \] guard now catches both a non-zero return and an empty result, and prints the offending repo before exiting. The inner \[ -n "$out" \] || return 1 makes the function return 1 on empty output so the if ! branch fires; if git ls-remote itself fails (network blip, expired token), the command substitution propagates that non-zero status and the same guard catches it. Both failure modes land in the same helpful message. Good.
Removed inputs. I grepped for every removed input name (kai_cli_ref, kai_tui_ref, kai_engine_ref, kai_core_ref) and for inputs.kai / inputs.kai_cli_ref. Within this repo, the only consumers are the workflow's own downstream steps — the checkout steps at lines 136/142/148/154 and the "Record what is going into the image" step at 171-174 — and those read steps.resolve.outputs.kai_*_ref, which the resolve loop still writes via echo "$(echo "$m" | tr 'a-z-' 'a-z_')_ref=$want" \>\> "$GITHUB_OUTPUT". That mapping produces kai_cli_ref, kai_tui_ref, kai_engine_ref, kai_core_ref correctly (kai-cli → kai_cli, etc.), so every downstream consumer is fed. No dangling reference to a now-deleted inputs.* field remains. The default: both / options: \[both, agent, review, none\] reordering is a valid choice list and both is a member, so the default is valid. I could not see whether anything outside this repo (another workflow, a dispatch client, a wrapper script) fires this workflow with the old input names; if something does, it would now error on the unknown inputs. Worth a glance at whatever triggers build-kai-ci automatically, if anything does.
Decision (not a defect). Changing the pin default from none to both means a dispatcher who clicks "Run workflow" without touching the dropdown now opens a kai-server PR pinning both the agent and review images, where before they pinned neither. That's the author's stated intent, and the PR is opened not merged — so this changes the default noise level in kai-server, not what ships.
Decision (not a defect). Removing the ref inputs makes "build from a branch" impossible in production (still possible locally via deploy/kai-ci/build.sh). This is the whole point — a production image now always corresponds to a released version, never a branch tip — but it removes the "ship a hotfix before tagging" escape hatch from the workflow. The author names this explicitly.
No defects. The SIGPIPE fix is the kind of careful, race-aware fix that's hard to get right under set -e, and the comment explaining why it's a race (not a certainty) is exactly what the next reader needs.
Merge it.
Important files changed
| File | Change |
|---|---|
.github/workflows/build-kai-ci.yml |
modified · +29 −39 |
Decisions
Correct as written, but somebody should say yes to these:
- The
pindefault is nowboth: a dispatcher who clicks "Run workflow" without changing the dropdown opens a kai-server PR pinning both agent and review images (previously: pinned neither). The PR still requires human merge, so this changes the default noise level in kai-server, not what ships. - Production builds can no longer target a branch tip — only the newest released
v*tag of each module. Branch builds remain possible only via the localdeploy/kai-ci/build.sh. This is the intended provenance guarantee, but it removes the "ship a hotfix before tagging" escape hatch from the workflow.
+29 −39 · 1 file · reaches 0 · the full analysis
💬 Reply to any of my comments and I'll answer, or say @kaicontext anywhere on this PR — a question, or "take another look at the retry logic".
1 participant
Two things, both from the first real run of this workflow — which failed before it checked anything out.
Remove the ref inputs
Four boxes you had to fill in, and the answer was always the same. There is nothing to type now: each module resolves to its own newest released tag.
A pin is described by version — "kai-cli v0.35.65 on kai-engine v0.6.55" — so the build should work in versions too. And a branch tip is not something anyone can point at afterwards, which is the whole objection this workflow exists to answer.
The form is now one dropdown, defaulting to bumping both pins. Building from a branch is still possible locally with
deploy/kai-ci/build.sh; it just isn't what production images are made from.Verified against the live repos:
Fix the SIGPIPE
git ls-remote ... | head -1makesheadclose the pipe while git is still writing. git takes SIGPIPE, andset -o pipefailturns that into exit 141 — run 34208363371.It is a race, not a certainty, and that is the part worth recording. The same command passed when I tested it locally, because that tag list was small enough for git to finish writing before
headexited. A test that passes by luck is indistinguishable from one that passes — which is why the verification changed too (below).Reading the output into a variable and slicing the first line in bash removes the race rather than making it rarer. No pipe, nothing to signal.
The error path around it was broken as well: under
set -e,want="$(latest_tag "$m")"killed the step silently when the function failed, so the message naming the repo without a tag never printed. It's anif !now.How this was verified
By extracting the resolve step's script out of the YAML and running it under bash with the runner's environment — the real script, not something shaped like it. That is what produced the tag list above.