-
Notifications
You must be signed in to change notification settings - Fork 1k
llm: Add evaluating-sdk-internal-updates skill and CI workflow #7251
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. Weβll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
11796e6
2f98b14
56b0153
9f6ec21
e845fe0
b131141
6c94a28
afa443a
0c1f28e
9416397
1501047
a446e12
a339992
bb0aabf
b02782b
ffbd736
fa15c9e
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,31 @@ | ||
| --- | ||
| name: evaluating-sdk-internal-updates | ||
| description: Evaluates a bitwarden/android "Update SDK to" PR against the sdk-internal commit range for compile-time and runtime breaking changes, maps affected symbols to Android call sites, and applies clear in-scope fixes. Use when reviewing an SDK bump PR, a bitwardenSdk version change, or triaging sdk-internal breaking changes. | ||
| allowed-tools: Bash(gh pr diff:*), Bash(git -C *:*), Bash(grep:*), Bash(./gradlew*:*), Read, Grep, Glob, Skill(plan-android-work), Skill(work-on-android) | ||
| --- | ||
|
|
||
| # Evaluating sdk-internal Updates | ||
|
|
||
| **Identify both compile-time and runtime breaks before fixing anything β fixing the first break found is not finishing.** Steps 3-7 always cover the entire commit range before step 8 starts, no matter how obvious or urgent an early compile-time break looks. | ||
|
|
||
| ## Identify | ||
|
|
||
| Binding surface facts specific to this SDK: `#[uniffi::export]` / `derive(uniffi::...)` annotations are scattered across many crates, not just `crates/bitwarden-uniffi`; `crates/bitwarden-ffi` is unrelated ("do not use"). No `.udl` files. UniFFI emits one Kotlin package per crate (`com.bitwarden.core`, `com.bitwarden.vault`, `com.bitwarden.crypto`, etc.) β only the top-level `Client`/`AuthClient`/`GeneratorClients` actually live under `com.bitwarden.sdk`. Both `app` and `authenticator` depend on the SDK; neither is optional to search. | ||
|
|
||
| A hunk that only touches a macro invocation (e.g. `state_bridge! { ... }`) doesn't show the binding surface β the expansion lives in the macro's definition, often in a different crate (e.g. `bitwarden-state-bridge-macro`). `#[uniffi::export(with_foreign)]` marks a callback interface: a trait Kotlin must implement, where adding a field/method is never additive-safe for the implementor. | ||
|
|
||
| 1. Locate the local `bitwarden/sdk-internal` clone (check sibling directories to this repo). If none exists, stop and tell the user it's a required prerequisite for this skill β do not clone it yourself. | ||
| 2. `gh pr diff <PR> -R bitwarden/android | grep bitwardenSdk` β old/new `bitwardenSdk` string. Everything after the second `-` is the git ref β a commit SHA or a branch name (`.dev` SDK builds use both); resolve a branch name as `origin/<branch>` in the clone. | ||
| 3. Attempt `./gradlew <module>:compileStandardDebugKotlin` at the current checkout before crawling sdk-internal. A failure confirms a compile-time break directly, with a more precise location than any git search β note it and continue to steps 4-7 for the full commit range; do not fix it yet. A clean build only rules out compile-time breaks, not runtime ones. | ||
| 4. `git -C <sdk-internal-path> log --oneline OLD..NEW -G'uniffi::export|derive\(uniffi|#\[uniffi' -- '*.rs'` β candidate binding-surface commits. | ||
| 5. Classify per hunk, not per commit β a commit with one additive headline change can still have a second, unrelated breaking hunk. If a hunk only touches a macro invocation, read the macro's definition before classifying. `git -C <sdk-internal-path> show <sha> -- '*.rs'`. | ||
| 6. For every distinct symbol/type touched (every hunk, not just the commit's headline change), grep the whole repo for the bare symbol name to find Android call sites β a fixed module list or a `com.bitwarden.sdk.<Symbol>` import-prefix check both miss real consumers. | ||
| 7. Report: compile-time breaks, runtime breaks, safe/no-call-site β each with commit, symbol, and call sites. | ||
|
|
||
| ## Resolve | ||
|
|
||
| Resolve the findings from Step 7 by deciding on a fix, invoking `/plan-android-work` with the report to plan the implementation, followed by invoking `/work-on-android` with the generated plan to implement required changes. | ||
|
|
||
| 8. Decide the fix for anything found, compile-time or runtime, whenever the correct behavior is clear and within scope. For a new required method, grep for the underlying concept, not the new method/type name (it won't exist yet) β no existing consumer means stub it: a `// no-op` comment or a null/default return that satisfies the compiler, not a behavioral decision. Never `TODO()` β it throws at runtime, which is a crash, not a stub. A sibling's structure (naming, placement, style) is a template; its behavior (storage, defaulting, error handling, side effects) is not evidence for yours. If unsure, report it instead of guessing, along with anything needing a product decision. | ||
| 9. Implement every fix from step 8 by `/plan-android-work` (pass it the step 7 findings) followed by `/work-on-android` (pass it the resulting plan) β never edit the fix in yourself, not even a one-line stub. | ||
| 10. Verify with the same compile task used in step 3. | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,140 @@ | ||
| name: SDLC / SDK Update Evaluation | ||
|
|
||
| on: | ||
| pull_request: | ||
| types: [opened, synchronize] | ||
|
|
||
| concurrency: | ||
| group: ${{ github.workflow }}-${{ github.event.pull_request.number }} | ||
| cancel-in-progress: true | ||
|
|
||
| permissions: {} | ||
|
|
||
| jobs: | ||
| evaluate: | ||
| name: Evaluate SDK Update | ||
| runs-on: ubuntu-24.04 | ||
| if: ${{ github.event.pull_request.head.ref == 'sdlc/sdk-update' }} | ||
| permissions: | ||
| id-token: write | ||
| actions: read | ||
| contents: write | ||
| packages: read | ||
| pull-requests: write | ||
|
|
||
| steps: | ||
| - name: Skip if this bump was already evaluated | ||
| id: gate | ||
| env: | ||
| GH_TOKEN: ${{ github.token }} | ||
| PR_NUMBER: ${{ github.event.pull_request.number }} | ||
| REPO: ${{ github.repository }} | ||
| run: | | ||
| LATEST_SUBJECT=$(gh pr view "$PR_NUMBER" -R "$REPO" --json commits --jq '.commits[-1].messageHeadline') | ||
| if [[ "$LATEST_SUBJECT" != "SDK Update - "* ]]; then | ||
| echo "Branch tip is not a fresh SDK version bump ('$LATEST_SUBJECT') β already evaluated, skipping." | ||
| echo "skip=true" >> "$GITHUB_OUTPUT" | ||
| else | ||
| echo "skip=false" >> "$GITHUB_OUTPUT" | ||
| fi | ||
|
|
||
| - name: Log in to Azure | ||
| if: steps.gate.outputs.skip == 'false' | ||
| uses: bitwarden/gh-actions/azure-login@main | ||
| with: | ||
| subscription_id: ${{ secrets.AZURE_SUBSCRIPTION_ID }} | ||
| tenant_id: ${{ secrets.AZURE_TENANT_ID }} | ||
| client_id: ${{ secrets.AZURE_CLIENT_ID }} | ||
|
|
||
| - name: Get Azure Key Vault secrets | ||
| id: get-kv-secrets | ||
| if: steps.gate.outputs.skip == 'false' | ||
| uses: bitwarden/gh-actions/get-keyvault-secrets@main | ||
| with: | ||
| keyvault: gh-org-bitwarden | ||
| secrets: "ANTHROPIC-RESPONSE-API-KEY,BW-GHAPP-ID,BW-GHAPP-KEY" | ||
|
|
||
| - name: Log out from Azure | ||
| if: steps.gate.outputs.skip == 'false' | ||
| uses: bitwarden/gh-actions/azure-logout@main | ||
|
|
||
| - name: Generate GH App token | ||
| if: steps.gate.outputs.skip == 'false' | ||
| uses: actions/create-github-app-token@bcd2ba49218906704ab6c1aa796996da409d3eb1 # v3.2.0 | ||
| id: app-token | ||
| with: | ||
| client-id: ${{ steps.get-kv-secrets.outputs.BW-GHAPP-ID }} | ||
| private-key: ${{ steps.get-kv-secrets.outputs.BW-GHAPP-KEY }} | ||
| permission-contents: write | ||
| permission-pull-requests: write | ||
|
|
||
| - name: Check out repo | ||
| if: steps.gate.outputs.skip == 'false' | ||
| uses: actions/checkout@df4cb1c069e1874edd31b4311f1884172cec0e10 # v6.0.3 | ||
| with: | ||
| token: ${{ steps.app-token.outputs.token }} | ||
| ref: ${{ github.event.pull_request.head.sha }} | ||
| fetch-depth: 0 | ||
| persist-credentials: true | ||
|
|
||
| - name: Clone sdk-internal | ||
| if: steps.gate.outputs.skip == 'false' | ||
| run: git clone --quiet https://github.com/bitwarden/sdk-internal "${{ github.workspace }}/../sdk-internal" | ||
|
|
||
| - name: Setup Android Build | ||
| if: steps.gate.outputs.skip == 'false' | ||
| uses: ./.github/actions/setup-android-build | ||
|
|
||
| - name: Run Claude Code | ||
|
SaintPatrck marked this conversation as resolved.
|
||
| id: claude | ||
| if: steps.gate.outputs.skip == 'false' | ||
| env: | ||
| GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} # settings.gradle.kts resolves the SDK from GitHub Packages | ||
| uses: anthropics/claude-code-action@be7b93b1907a4abad570368f3c74b6fe3807510b # v1.0.183 | ||
| with: | ||
| anthropic_api_key: ${{ steps.get-kv-secrets.outputs.ANTHROPIC-RESPONSE-API-KEY }} | ||
| allowed_bots: "bw-ghapp" | ||
| bot_id: "178206702" | ||
| bot_name: "bw-ghapp[bot]" | ||
| track_progress: true | ||
| use_sticky_comment: true | ||
| plugin_marketplaces: "https://github.com/bitwarden/ai-plugins.git" | ||
| plugins: | | ||
| bitwarden-delivery-tools@bitwarden-marketplace | ||
| prompt: | | ||
| PR #${{ github.event.pull_request.number }} in ${{ github.repository }} bumps the | ||
| Bitwarden SDK. Evaluate it using the `evaluating-sdk-internal-updates` skill and | ||
| resolve anything within scope that needs resolving, per the skill's own Resolve | ||
| steps. `bitwarden/sdk-internal` is already cloned as a sibling directory at | ||
| `${{ github.workspace }}/../sdk-internal`, satisfying step 1's prerequisite β use it | ||
| directly. Commit any fix on the current branch (already checked out) using | ||
| `Skill(bitwarden-delivery-tools:committing-changes)` for the message β do not push; | ||
| the workflow pushes separately. | ||
|
|
||
| When done, update the sticky PR comment with the step 7 report, structured as: | ||
| "## SDK bump evaluated" (old β new version, commit count), "## Compile-time breaks" | ||
| (found-and-fixed, or "none found"), "## Runtime considerations" (any behavioral notes | ||
| worth flagging, or omit the section if there are none), "## Everything else in range | ||
| β confirmed safe" (brief, one line per commit or group), and "## Commit" (the SHA, or | ||
| "no commit β nothing needed fixing"). Always post this, even when the conclusion is | ||
| that nothing needs fixing β the comment is the audit trail that the check ran. | ||
| claude_args: | | ||
| --model opus | ||
| --allowedTools "Bash(gh pr diff:*),Bash(git -C *:*),Bash(git log:*),Bash(git show:*),Bash(git diff:*),Bash(grep:*),Bash(./gradlew*:*),Read,Grep,Glob,Edit,Write,Skill,mcp__github_comment__update_claude_comment" | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Details and fixThe allowlist enumerates read-only git commands ( The failure is silent: line 128 compares Suggested:
if ! git diff --quiet; then
echo "::error::Claude left uncommitted changes; nothing was pushed."
exit 1
fi
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Skipped adding
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Addressed in 1501047 |
||
|
|
||
| - name: Push any resolved fix | ||
| if: steps.gate.outputs.skip == 'false' | ||
| env: | ||
| _BRANCH_NAME: ${{ github.event.pull_request.head.ref }} | ||
| run: | | ||
| if [ -n "$(git status --porcelain)" ]; then | ||
|
SaintPatrck marked this conversation as resolved.
|
||
| echo "::error::Claude left uncommitted changes; nothing was pushed." | ||
| git status --porcelain | ||
| exit 1 | ||
| fi | ||
|
|
||
| if git diff --quiet "origin/$_BRANCH_NAME" HEAD; then | ||
| echo "No new commits to push." | ||
| else | ||
| git push origin "HEAD:$_BRANCH_NAME" | ||
|
SaintPatrck marked this conversation as resolved.
|
||
| fi | ||
Uh oh!
There was an error while loading. Please reload this page.