Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
34 changes: 33 additions & 1 deletion .github/workflows/reborn-tests.yml
Original file line number Diff line number Diff line change
Expand Up @@ -100,7 +100,9 @@ jobs:
- id: packages
name: Build package matrix from Reborn crate families
run: |
packages="$(
# Reborn/product crate families that are always tested, including
# crates the shipped binary does not link (channel adapters, webui_v2).
allowlist_packages="$(
cargo metadata --no-deps --format-version 1 \
| jq -c '
[
Expand All @@ -120,6 +122,36 @@ jobs:
'
)"

# The full ironclaw_reborn_cli dependency closure: every workspace
# crate the shipped Reborn binary links (normal + build deps). Running
# each crate's own suite on every PR is the "run everything on every
# PR" gate -- a green reborn-tests run means the whole closure is
# green, so the ~43 shared crates (auth, host_runtime, skills,
# extensions, ...) can no longer regress unnoticed.
closure_packages="$(
comm -12 \
<(cargo tree -p ironclaw_reborn_cli -e normal,build --prefix none \
| grep -oE 'ironclaw_[a-z0-9_]+' \
| sort -u) \
<(cargo metadata --no-deps --format-version 1 \
| jq -r '.packages[].name' \
| sort -u) \
| jq -R -s -c 'split("\n") | map(select(length > 0))'
)"

if [ -z "${closure_packages}" ] || [ "${closure_packages}" = "[]" ]; then
echo "No Reborn CLI workspace dependency closure crates discovered" >&2
exit 1
fi
Comment on lines +142 to +145

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick | 🔵 Trivial | 💤 Low value

Misleading error message on cargo tree failure.

If cargo tree fails, the script will hit this error message, but "No Reborn CLI workspace dependency closure crates discovered" suggests a dependency-graph issue rather than a tool failure. Consider checking cargo tree exit status separately or clarifying the message.

Clearer error handling
+          if ! cargo tree -p ironclaw_reborn_cli -e normal,build --prefix none >/dev/null 2>&1; then
+            echo "cargo tree failed for ironclaw_reborn_cli" >&2
+            exit 1
+          fi
+
           closure_packages="$(
             comm -12 \
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In @.github/workflows/reborn-tests.yml around lines 142 - 145, The error message
in the condition checking closure_packages is misleading because it doesn't
distinguish between cargo tree failing versus returning an empty result. Modify
the script to separately capture and check the exit status of the cargo tree
command that generates closure_packages. If cargo tree exits with a non-zero
status, output an error message indicating the tool failure. Only check for
empty or "[]" results if cargo tree succeeded, and keep the current message for
that case.


# Union so closure coverage never drops the non-closure allowlist crates.
packages="$(
jq -n -c \
--argjson allowlist "${allowlist_packages}" \
--argjson closure "${closure_packages}" \
'$allowlist + $closure | unique'
)"

if [ -z "${packages}" ] || [ "${packages}" = "[]" ]; then
echo "No Reborn workspace crates discovered" >&2
exit 1
Expand Down
57 changes: 56 additions & 1 deletion scripts/ci/package-feature-flags.sh
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,40 @@ if [ "$#" -ne 1 ]; then
exit 2
fi

case "$1" in
package="$1"

# Default flags for closure crates without an explicit recipe above: opt into
# `default` and `libsql` when the crate declares them, so storage-backed crates
# build their libSQL paths. Crates with no matching features build bare.
fallback_feature_flags() {
local metadata
metadata="$(cargo metadata --no-deps --format-version 1)"

local feature_list
feature_list="$(
jq -r --arg package "${package}" '
.packages[]
| select(.name == $package)
| .features
| keys[]
' <<< "${metadata}"
)"

local features=()
if printf '%s\n' "${feature_list}" | grep -Fxq "default"; then
features+=("default")
fi
if printf '%s\n' "${feature_list}" | grep -Fxq "libsql"; then
features+=("libsql")
fi

if [ "${#features[@]}" -gt 0 ]; then
local IFS=,
printf '%s\n' "--features ${features[*]}"
fi
Comment on lines +18 to +39

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

We can simplify this function significantly by performing the entire feature filtering and formatting logic directly inside jq. This avoids creating multiple subshells (printf and grep twice) and simplifies the Bash array/IFS manipulation, making the script cleaner, faster, and more maintainable.

Suggested change
local feature_list
feature_list="$(
jq -r --arg package "${package}" '
.packages[]
| select(.name == $package)
| .features
| keys[]
' <<< "${metadata}"
)"
local features=()
if printf '%s\n' "${feature_list}" | grep -Fxq "default"; then
features+=("default")
fi
if printf '%s\n' "${feature_list}" | grep -Fxq "libsql"; then
features+=("libsql")
fi
if [ "${#features[@]}" -gt 0 ]; then
local IFS=,
printf '%s\n' "--features ${features[*]}"
fi
jq -r --arg package "${package}" '
.packages[]
| select(.name == $package)
| .features
| keys
| map(select(. == "default" or . == "libsql"))
| if length > 0 then "--features " + join(",") else empty end
' <<< "${metadata}"

}

case "${package}" in
ironclaw_reborn_cli)
printf '%s\n' "--features webui-v2-beta,slack-v2-host-beta"
;;
Expand All @@ -31,9 +64,31 @@ case "$1" in
ironclaw_reborn_webui_ingress)
printf '%s\n' "--features dev-in-memory-session"
;;
ironclaw_host_runtime)
# Integration tests (tests/) link the lib as a normal dependency, so
# cfg(test) is false there; the deterministic test-mode behavior they assert
# is gated behind `feature = "test-support"`. libsql exercises the embedded
# DB paths without a Postgres server (which the crate-tests job has none of).
printf '%s\n' "--features test-support,libsql"
;;
ironclaw_webui_v2 | ironclaw_webui_v2_static)
printf '%s\n' "--features webui-v2-beta"
;;
ironclaw_architecture | \
ironclaw_product_adapter_registry | \
ironclaw_product_context | \
ironclaw_reborn_config | \
ironclaw_reborn_identity | \
ironclaw_reborn_openai_compat | \
ironclaw_reborn_openai_compat_storage | \
ironclaw_reborn_traces | \
ironclaw_slack_v2_adapter | \
ironclaw_telegram_v2_adapter | \
ironclaw_wasm_product_adapters)
# Already on the allowlist with no feature flags; keep them flag-free now
# that the default branch derives fallback features for closure crates.
;;
*)
fallback_feature_flags
;;
esac
Loading