Skip to content

more deterministic tests#482

Merged
khaliqgant merged 2 commits intomainfrom
hardened-tests-workflows
Mar 4, 2026
Merged

more deterministic tests#482
khaliqgant merged 2 commits intomainfrom
hardened-tests-workflows

Conversation

@khaliqgant
Copy link
Copy Markdown
Member

@khaliqgant khaliqgant commented Mar 4, 2026

Summary

Test Plan

  • Tests added/updated
  • Manual testing completed

Screenshots


Open with Devin

@khaliqgant khaliqgant requested a review from willwashburn as a code owner March 4, 2026 21:18
@khaliqgant khaliqgant merged commit d431233 into main Mar 4, 2026
9 checks passed
@khaliqgant khaliqgant deleted the hardened-tests-workflows branch March 4, 2026 21:19
Copy link
Copy Markdown
Contributor

@devin-ai-integration devin-ai-integration Bot left a comment

Choose a reason for hiding this comment

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

Devin Review found 1 potential issue.

View 4 additional findings in Devin Review.

Open in Devin Review

RUSTFLAGS="-C target-feature=+crt-static" cargo build --release --bin agent-relay-broker --target ${{ matrix.target }}
strip target/${{ matrix.target }}/release/agent-relay-broker 2>/dev/null || \
aarch64-linux-musl-strip target/${{ matrix.target }}/release/agent-relay-broker 2>/dev/null || true
strip target/${{ matrix.target }}/release/agent-relay-broker 2>/dev/null || true
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.

🟡 publish.yml aarch64 binary is never stripped due to missing cross-strip tool

The strip command on line 125 runs the host's x86_64 strip against a cross-compiled aarch64 binary, which silently fails (masked by || true). The old code had a fallback to aarch64-linux-musl-strip, and the companion workflow build-broker-binary.yml:51-53 correctly installs binutils-aarch64-linux-gnu and uses aarch64-linux-gnu-strip for the aarch64 case. The publish workflow omits both the package install and the cross-architecture strip invocation, so the aarch64 binary will be published unstripped (larger file size).

Prompt for agents
In .github/workflows/publish.yml, the aarch64 binary strip is ineffective because the host strip cannot process an ARM binary. To fix:

1. Add a step after the 'Install cross (aarch64)' step (around line 111) to install binutils-aarch64-linux-gnu, similar to what build-broker-binary.yml does at line 53:
   - name: Install cross strip tools (aarch64)
     if: matrix.target == 'aarch64-unknown-linux-musl'
     run: sudo apt-get update && sudo apt-get install -y binutils-aarch64-linux-gnu

2. Update the strip line (currently line 125) to use aarch64-linux-gnu-strip for the aarch64 target, keeping the generic strip for other targets. Change the 'Build broker binary' step's strip command to:
   if [[ "${{ matrix.target }}" == "aarch64-unknown-linux-musl" ]]; then
     aarch64-linux-gnu-strip target/${{ matrix.target }}/release/agent-relay-broker 2>/dev/null || true
   else
     strip target/${{ matrix.target }}/release/agent-relay-broker 2>/dev/null || true
   fi

This matches the pattern already used in .github/workflows/build-broker-binary.yml lines 51-66.
Open in Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant