llm: Add evaluating-sdk-internal-updates skill and CI workflow - #7251
Conversation
github-actions
Bot
added
the
t:llm
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #7251 +/- ##
==========================================
+ Coverage 85.88% 86.31% +0.42%
==========================================
Files 934 921 -13
Lines 66626 66443 -183
Branches 9748 9794 +46
==========================================
+ Hits 57221 57347 +126
+ Misses 5925 5592 -333
- Partials 3480 3504 +24
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
The skill no longer offers to clone sdk-internal or falls back to gh api calls when it's missing — it locates the local clone and stops with a clear message if there isn't one, matching how branching and pushing are already the caller's concern. The CI workflow clones it as a sibling before invoking Claude, satisfying the prerequisite directly instead of leaving the decision to an unattended agent.
Step 11 invokes Skill(bitwarden-delivery-tools:committing-changes), but plugin skills aren't available in claude-code-action by default — they need an explicit plugin_marketplaces/plugins declaration, same as respond.yml does for its own plugin set.
Kotlin's TODO() throws NotImplementedError when called. Listing it alongside genuinely inert options (no-op, null/default return) contradicted the point of stubbing: satisfying the compiler without introducing a behavioral decision. A crash is a behavioral decision.
Whether and how to commit a fix depends on the caller (interactive session vs. CI), same as branching, pushing, and sdk-internal cloning already do. The CI workflow's own prompt now carries the commit instruction directly, since it already owned the adjacent push step.
Adds a required pr-number input and a Resolve PR context step that uses it on manual runs, falling back to the triggering pull_request event's PR otherwise. Checkout, the push step, and the agent prompt all read from this resolved context instead of assuming a pull_request event.
github.event.pull_request.head.ref (a branch name, attacker-influenceable in general) was spliced directly into the run: script via template expansion, which happens before the shell parses it. Moved every event/input-derived value into env and referenced them as shell variables instead, per GitHub's documented mitigation.
🤖 Bitwarden Claude Code ReviewOverall Assessment: REQUEST CHANGES Re-reviewed after the four newest commits. The prior findings are addressed: the uncommitted-work guard now uses Code Review Details
|
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
| 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" |
There was a problem hiding this comment.
Details and fix
The allowlist enumerates read-only git commands (git -C, git log, git show, git diff); git add and git commit are absent, as is git config. The checkout is also detached at head-sha with no committer identity configured, so git commit would fail with "Author identity unknown" even if allowed.
The failure is silent: line 128 compares origin/$_BRANCH_NAME against HEAD, i.e. committed trees only. Uncommitted edits produce no difference, the step prints "No new commits to push", and the job ends green with the fix discarded along with the runner.
Suggested:
- add
Bash(git add:*),Bash(git commit:*),Bash(git config:*)to--allowedTools - seed
git config user.name/user.emailwith the bot identity (assdlc-sdk-update.ymldoes) in a step before Claude runs - fail loudly on leftover changes before the push check:
if ! git diff --quiet; then
echo "::error::Claude left uncommitted changes; nothing was pushed."
exit 1
fiThere was a problem hiding this comment.
Skipped adding git add/commit/config to allowedTools. Checked the action source, git add/commit are already granted implicitly by tag mode (track_progress forces it). A git config step wouldn't stick either, configureGitAuth authors commits from bot_name/bot_id, not local git config, so set those to bw-ghapp[bot]'s identity instead. Added the fail-loudly check on leftover changes as suggested.
| if [ "$EVENT_NAME" == "workflow_dispatch" ]; then | ||
| NUMBER="$PR_NUMBER_INPUT" | ||
| HEAD_SHA=$(gh pr view "$NUMBER" -R "$REPO" --json headRefOid --jq .headRefOid) | ||
| HEAD_REF=$(gh pr view "$NUMBER" -R "$REPO" --json headRefName --jq .headRefName) |
There was a problem hiding this comment.
workflow_dispatch path accepts any PR number with no head-repo or branch check.
Details and fix
The pull_request path is gated to sdlc/sdk-update (line 23), but dispatch takes whatever gh pr view returns. Dispatched against a fork PR, this job checks out untrusted code with persist-credentials: true and a GitHub App token holding contents: write, then runs ./gradlew — Gradle build scripts are arbitrary code, which can read that token out of .git/config. The final step would then git push origin "HEAD:<fork branch name>", creating a branch in this repo.
Validate after resolution:
HEAD_REPO=$(gh pr view "$NUMBER" -R "$REPO" --json headRepositoryOwner,headRepository \
--jq '"\(.headRepositoryOwner.login)/\(.headRepository.name)"')
if [ "$HEAD_REPO" != "$REPO" ] || [ "$HEAD_REF" != "sdlc/sdk-update" ]; then
echo "::error::Only same-repo sdlc/sdk-update PRs can be evaluated."
exit 1
fiThere was a problem hiding this comment.
Dropped workflow_dispatch instead of adding this check. detectMode throws for any event outside PR/issue, and use_sticky_comment is gated on isPullRequestEvent, so the manual trigger path doesn't work with claude-code-action regardless of validation. Will add branch/repo checks if manual triggering comes back with a supported mechanism.
…ector claude-code-action's detectMode throws for any event outside the PR/ issue set, and use_sticky_comment is separately gated on isPullRequestEvent. The manual-trigger path added in b131141/6c94a28d8 can't work with this action version regardless of how its own validation is scoped. Reverting to pull_request-only until there's a supported way to drive this manually.
checkHumanActor rejects any non-User actor unless explicitly allow- listed via allowed_bots, which defaults to empty. The sdlc/sdk-update PR is opened and pushed by bw-ghapp[bot], so every opened/synchronize event died at the action's own prepare step before this addition — neither the prompt nor the skill ever ran.
settings.gradle.kts authenticates maven.pkg.github.com/bitwarden/sdk with GITHUB_TOKEN, and this job had neither that token, packages: read, nor the JDK 21 / Gradle setup sdlc-sdk-update.yml's own Test job uses. Without this, every ./gradlew invocation fails at dependency resolution — steps 3 and 10 of the skill become misleading rather than merely unavailable, since they report a compile-time break directly from the failure.
configureGitAuth authors commits from the action's bot_name/bot_id defaults (claude[bot]), not our intended identity. sdlc-sdk-update.yml hard-fails when the branch tip's author isn't 178206702+bw-ghapp[bot], so the first fix commit would have permanently blocked the SDK update pipeline on that branch. Setting bot_id/bot_name to match. Separately, the push step compared committed trees only (origin vs. HEAD), so an edit Claude started but never committed produced no diff, printed "No new commits to push," and the job went green having silently done nothing. Now fails the job instead.
Bash(./gradlew*compileKotlin*:*) requires the literal substring compileKotlin; step 3's compileStandardDebugKotlin ends in DebugKotlin, so the rule never matched and blocked both the compile-break pre-check and the fix verification. Library modules also use compileDebugKotlin (no product flavors), so a narrower per-flavor pattern would still need to cover both spellings — using a plain ./gradlew wildcard instead. Also fixed step 10's stale "compileKotlin" restatement to reference step 3 instead of drifting from it again.
"Kotlin symbols surface as com.bitwarden.sdk.*" was wrong — UniFFI emits one package per crate (com.bitwarden.core, .vault, .crypto, etc.); only the top-level Client/AuthClient/GeneratorClients actually live under com.bitwarden.sdk. Grepping for that import prefix in a fixed app/core/network/ui module list also skipped authenticator entirely, which depends on the SDK too (implementation(libs.bitwarden.sdk) in authenticator/build.gradle.kts) and has its own real call sites. An authenticator-only break would have been reported as "no call sites" and step 3's single-module compile wouldn't have caught it either. Searching the whole repo for the bare symbol name avoids both gaps.
App-token pushes fire synchronize (unlike GITHUB_TOKEN), so every auto-fix commit re-triggers this workflow for a second full run against code that's already fixed. Checks the branch tip's commit subject via the API before any Azure/build setup — sdlc-sdk-update.yml always titles its bump commits "SDK Update - ...", so anything else on the tip (our own prior fix, or a manual change) means this bump was already evaluated and the rest of the job is skipped.
20 minutes was a guess and didn't obviously cover the fix path (cold Gradle setup + compile + sdk-internal crawl + plan/work-on-android's own implement/test/verify/preflight/commit cycle + re-compile), and a timeout cancels the job before the push step ever runs, discarding any commit that only ever existed on the runner. Falling back to the platform default rather than guessing a number.
Bare git diff only compares the working tree to the index — a file that's git add'ed but not committed shows no diff (worktree matches index), and untracked files are invisible to git diff entirely. Both cases fell through to the committed-trees comparison, printed "No new commits to push", and discarded the fix. A new required SDK method is exactly the case likely to need a new file. git status --porcelain covers staged, unstaged, and untracked in one check.
… SHA "Trailing segment is the SHA" breaks for .dev SDK builds (scripts/update-sdk-version.sh's own usage examples show both 1.0.0-2586-20e3dfa6 and 1.0.0-2577-fix-wasm-import), which land on sdlc/sdk-update routinely since that's the Update mode's default package. sdlc-sdk-update.yml extracts the same field with cut -d'-' -f3- specifically because it "handles both commit hashes and branch names." Taking a literal branch name as OLD/NEW in step 4 would fail to resolve in the clone; step 2 now says to resolve it as origin/<branch> instead.
Claude Code Configuration Validation — PR #7251Scope: Also read as required context:
✅ Security scan — cleanAll four mandatory security checks pass. No blocking security issues.
✅ Frontmatter & structure — validParsed with a real YAML loader: Findings🔴 Critical — errors, must fixThese are functional defects that break the automation. All three were independently verified against the repo, not just reported. C1. A successful fix run will fail the job — the plan artifact dirties the tree
Step 9 mandates So the plan file alone fails the job and discards the fix it was written to produce. This fires on the success path, not an edge case. Fix (pick one): add C2. Steps delegate to interactive commands that commit and open a PR
Verified in Fix: in step 9, name the applicable phases and the stop point — e.g. "Phases 1-3 of C3. Committing is instructed but not permitted
Fix: add 🟠 Major — warnings, should fixM1. The step-4 search misses the most common class of UniFFI break
Fix: two-phase search — enumerate the full range ( M2. Wrong Gradle task for
|
3 participants
🎟️ Tracking
AI-74
📔 Objective
Adds a new skill,
evaluating-sdk-internal-updates, that evaluates a bitwarden/android SDK-bump PR against the sdk-internal commit range for compile-time and runtime breaking changes, maps affected symbols to Android call sites, and resolves in-scope fixes.Adds
.github/workflows/sdlc-sdk-update-evaluate.yml, which runs this skill via Claude Code against the bot'ssdlc/sdk-updatePR branch on open/synchronize, auto-commits and pushes any resolved fix, and reports results as a sticky PR comment.