chore(ci): Improve X-Test workflow performance - #378
Conversation
Add AI assistant configuration files with project guidelines and development instructions. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> Signed-off-by: Paul Flynn <pflynn@virtru.com>
Summary of ChangesHello @pflynn-virtru, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request focuses on optimizing the X-Test workflow to decrease feedback time and improve CI efficiency. It addresses a bug in the PR matrix configuration, introduces concurrency controls, and implements dependency caching. Additionally, it introduces a new documentation file outlining repository guidelines. Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Ignored Files
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request has a major discrepancy between its stated goals and the actual changes. The title and description claim to improve CI workflow performance, but the diff only contains the addition of two documentation files (AGENTS.md and CLAUDE.md). The CI changes described are completely missing from the pull request. This must be rectified by either updating the PR description to reflect the documentation changes or by including the missing CI workflow modifications.
Regarding the files that were changed:
AGENTS.mdadds valuable developer guidelines, though I've suggested renaming the file for better clarity.CLAUDE.mdappears to be an unintentionally generated file and should be removed.
a01d7f5 to
d99382c
Compare
- Fix PR matrix bug: remove hardcoded 'main lts' default so DEFAULT_TAGS logic applies correctly (PRs now run 3 jobs instead of 6) - Add concurrency controls with github.run_id fallback to cancel superseded PR runs without affecting scheduled runs - Add dependency caching for npm, Go modules, and Maven repository (placed after SDK checkouts where lock files exist) Closes #377 Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> Signed-off-by: Paul Flynn <pflynn@virtru.com>
d99382c to
76f65f8
Compare
X-Test Failure Report |
X-Test Results |
X-Test Failure Report |
1 similar comment
X-Test Failure Report |
The script was silently failing when pom.xml had no <properties> section
(e.g., Java SDK v0.7.5). The sed command to add <platform.branch> would
do nothing, but the replacement of branch=main with ${platform.branch}
would still run, causing Maven to pass the literal string to buf/git.
Now the branch replacement only runs if the property was successfully
added (or already existed).
Fixes #379
Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Signed-off-by: Paul Flynn <pflynn@virtru.com>
X-Test Failure Report |
…a.sh Java SDK versions need compatible platform protocol branches: - v0.7.5 uses protocol/go/v0.2.18, not main - Added get_platform_branch() function mapping versions to protocols - Mappings match those in resolve-version.py This fixes compilation errors when building older Java SDK versions against incompatible protocol definitions. Fixes #379 Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> Signed-off-by: Paul Flynn <pflynn@virtru.com>
X-Test Failure Report |
Update post-checkout-java.sh to: - Check if the CORRECT platform.branch value is set, not just any value - Replace existing incorrect values with the version-specific protocol branch - Add explicit return statement to get_platform_branch function This fixes Java SDK v0.7.5 (lts) builds that were failing because the pom.xml already had <platform.branch>main</platform.branch> defined, but needed <platform.branch>protocol/go/v0.2.18</platform.branch> for compatibility. Fixes #379 Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> Signed-off-by: Paul Flynn <pflynn@virtru.com>
X-Test Failure Report |
…xists When the pom.xml has no <properties> section (like Java SDK v0.7.5), directly replace `branch=main` with `branch=protocol/go/v0.2.18` instead of trying to use Maven property substitution which doesn't work. Fixes #379 Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> Signed-off-by: Paul Flynn <pflynn@virtru.com>
X-Test Failure Report❌ js-v0.4.34 |
Update LTS versions to more recent releases from summer 2025: - Go: 0.15.0 → 0.24.0 (2025-07-31) - Java: 0.7.5 → 0.9.0 (2025-07-14) - JS: 0.2.0 → 0.4.0 (2025-06-30) - Platform: 0.4.34 → 0.9.0 (2025-08-27) Also update PR runs to test main, latest, and lts versions (9 jobs total) to ensure comprehensive coverage. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> Signed-off-by: Paul Flynn <pflynn@virtru.com>
X-Test Failure Report |
Don't overwrite platform.branch in pom.xml files that already have a valid protocol/go/v* value set. Only modify files that: - Are missing the platform.branch property - Have a specific mapping in the script (older versions) This fixes Java v0.9.0+ builds which already have the correct platform.branch defined in their pom.xml. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> Signed-off-by: Paul Flynn <pflynn@virtru.com>
X-Test Failure Report |
1 similar comment
X-Test Failure Report |
- Update bufbuild/buf-setup-action to v1.50.0 - Use github.token instead of secrets.GITHUB_TOKEN - Set buf version to 1.56.0 - Remove BUF_INPUT_HTTPS env vars (secret doesn't exist in tests repo) This should fix the buf rate limiting issue by using proper authentication. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> Signed-off-by: Paul Flynn <pflynn@virtru.com>
X-Test Failure Report |
- Replace buf-setup-action with buf-action (consolidated action) - Use setup_only: true to just install buf CLI - Add BUF_TOKEN secret for BSR rate limit authentication - Fixes BSR 'too many requests' errors during parallel Java builds Signed-off-by: Paul Flynn <pflynn@virtru.com>
X-Test Failure Report |
- Grant `id-token: write` permission for GCP authentication - Add steps to authenticate with Google Cloud and retrieve BUF_TOKEN secret from Secret Manager - Update BUF action to use token from retrieved secrets
X-Test Failure Report |
- Replace hardcoded GCP auth parameters with reusable workflow variables - Pin google-github-actions/auth and get-secretmanager-secrets actions to specific commits for better reproducibility
X-Test Failure Report |
- Upgrade `google-github-actions/auth` to v3.0.0 - Use `secrets.GCP_WORKLOAD_IDENTITY` instead of deprecated vars - Update BUF_TOKEN secret reference to a specific project identifier
X-Test Failure Report |
- Eliminate GCP authentication and secret retrieval steps - Update BUF_TOKEN reference to use `secrets.BUF_TOKEN` directly
X-Test Failure Report❌ js-v0.9.0 |
X-Test Results✅ js-main |
X-Test Results✅ js-main |
dmihalcik-virtru
left a comment
dmihalcik-virtru
left a comment
There was a problem hiding this comment.
The AGENTS.md appears more aspirational than functional at this point, but at least it gives us something to iterate on
|
X-Test Results✅ js-v0.11.6 |
2 participants
Summary
Addresses performance issues in the X-Test workflow to improve PR feedback time through caching and concurrency controls.
'main lts'default fromPLATFORM_REFenv var soDEFAULT_TAGSlogic applies correctly (was being overridden)github.event.pull_request.number(avoids fork collisions), withgithub.run_idfallback for scheduled runsPR Test Matrix
PRs run 6 jobs (2 platforms × 3 SDKs) for full compatibility coverage:
Performance improvements come from caching (~2-3 min savings per job) and concurrency controls (cancels superseded runs).
CI Performance Comparison
Analyzed recent workflow runs using
gh run listandgh run view:This branch (with caching):
Main branch (baseline):
Key Finding
The perceived slowdown is NOT from caching changes - it's from the SDK version difference:
The newer v0.11.6 SDK takes significantly longer to test than v0.4.34. Caching changes have no negative impact on performance. When main branch tested the same workload (42 min run on Jan 21), performance was comparable to this branch, confirming baseline performance is similar.
Test plan
Closes #377
🤖 Generated with Claude Code