Skip to content

Commit 292b5f6

Browse files
authored
PM-26544 - Initial create of a CLAUDE.md and a code review action (#493)
## 🎟️ Tracking https://bitwarden.atlassian.net/browse/PM-26544 ## 📔 Objective An initial creation of a Claude markdown and a code review action. Tried to leverage both Claude and my own limited knowledge of our internal SDK to create the Claude markdown. ## ⏰ Reminders before review - Contributor guidelines followed - All formatters and local linters executed and passed - Written new unit and / or integration tests where applicable - Protected functional changes with optionality (feature flags) - Used internationalization (i18n) for all UI strings - CI builds passed - Communicated to DevOps any deployment requirements - Updated any necessary documentation (Confluence, contributing docs) or informed the documentation team ## 🦮 Reviewer guidelines <!-- Suggested interactions but feel free to use (or not) as you desire! --> - 👍 (`:+1:`) or similar for great changes - 📝 (`:memo:`) or ℹ️ (`:information_source:`) for notes or general info - ❓ (`:question:`) for questions - 🤔 (`:thinking:`) or 💭 (`:thought_balloon:`) for more open inquiry that's not quite a confirmed issue and could potentially benefit from discussion - 🎨 (`:art:`) for suggestions / improvements - ❌ (`:x:`) or ⚠️ (`:warning:`) for more significant problems or concerns needing attention - 🌱 (`:seedling:`) or ♻️ (`:recycle:`) for future improvements or indications of technical debt - ⛏ (`:pick:`) for minor or nitpick changes
1 parent e2dca98 commit 292b5f6

File tree

2 files changed

+167
-0
lines changed

2 files changed

+167
-0
lines changed

.github/workflows/review-code.yml

Lines changed: 109 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,109 @@
1+
name: Review code
2+
3+
on:
4+
pull_request:
5+
types: [opened, synchronize, reopened]
6+
7+
permissions: {}
8+
9+
jobs:
10+
review:
11+
name: Review
12+
runs-on: ubuntu-24.04
13+
permissions:
14+
contents: read
15+
id-token: write
16+
pull-requests: write
17+
18+
steps:
19+
- name: Check out repo
20+
uses: actions/checkout@08c6903cd8c0fde910a37f88322edcfb5dd907a8 # v5.0.0
21+
with:
22+
fetch-depth: 0
23+
persist-credentials: false
24+
25+
- name: Check for Vault team changes
26+
id: check_changes
27+
run: |
28+
# Ensure we have the base branch
29+
git fetch origin ${{ github.base_ref }}
30+
31+
echo "Comparing changes between origin/${{ github.base_ref }} and HEAD"
32+
CHANGED_FILES=$(git diff --name-only origin/${{ github.base_ref }}...HEAD)
33+
34+
if [ -z "$CHANGED_FILES" ]; then
35+
echo "Zero files changed"
36+
echo "vault_team_changes=false" >> $GITHUB_OUTPUT
37+
exit 0
38+
fi
39+
40+
# Handle variations in spacing and multiple teams
41+
VAULT_PATTERNS=$(grep -E "@bitwarden/team-vault-dev(\s|$)" .github/CODEOWNERS 2>/dev/null | awk '{print $1}')
42+
43+
if [ -z "$VAULT_PATTERNS" ]; then
44+
echo "⚠️ No patterns found for @bitwarden/team-vault-dev in CODEOWNERS"
45+
echo "vault_team_changes=false" >> $GITHUB_OUTPUT
46+
exit 0
47+
fi
48+
49+
vault_team_changes=false
50+
for pattern in $VAULT_PATTERNS; do
51+
echo "Checking pattern: $pattern"
52+
53+
# Handle **/directory patterns
54+
if [[ "$pattern" == "**/"* ]]; then
55+
# Remove the **/ prefix
56+
dir_pattern="${pattern#\*\*/}"
57+
# Check if any file contains this directory in its path
58+
if echo "$CHANGED_FILES" | grep -qE "(^|/)${dir_pattern}(/|$)"; then
59+
vault_team_changes=true
60+
echo "✅ Found files matching pattern: $pattern"
61+
echo "$CHANGED_FILES" | grep -E "(^|/)${dir_pattern}(/|$)" | sed 's/^/ - /'
62+
break
63+
fi
64+
else
65+
# Handle other patterns (shouldn't happen based on your CODEOWNERS)
66+
if echo "$CHANGED_FILES" | grep -q "$pattern"; then
67+
vault_team_changes=true
68+
echo "✅ Found files matching pattern: $pattern"
69+
echo "$CHANGED_FILES" | grep "$pattern" | sed 's/^/ - /'
70+
break
71+
fi
72+
fi
73+
done
74+
75+
echo "vault_team_changes=$vault_team_changes" >> $GITHUB_OUTPUT
76+
77+
if [ "$vault_team_changes" = "true" ]; then
78+
echo ""
79+
echo "✅ Vault team changes detected - proceeding with review"
80+
else
81+
echo ""
82+
echo "❌ No Vault team changes detected - skipping review"
83+
fi
84+
85+
- name: Review with Claude Code
86+
if: steps.check_changes.outputs.vault_team_changes == 'true'
87+
uses: anthropics/claude-code-action@a5528eec7426a4f0c9c1ac96018daa53ebd05bc4 # v1.0.7
88+
with:
89+
anthropic_api_key: ${{ secrets.ANTHROPIC_API_KEY }}
90+
track_progress: true
91+
prompt: |
92+
REPO: ${{ github.repository }}
93+
PR NUMBER: ${{ github.event.pull_request.number }}
94+
TITLE: ${{ github.event.pull_request.title }}
95+
BODY: ${{ github.event.pull_request.body }}
96+
AUTHOR: ${{ github.event.pull_request.user.login }}
97+
98+
Please review this pull request with a focus on:
99+
- Code quality and best practices
100+
- Potential bugs or issues
101+
- Security implications
102+
- Performance considerations
103+
104+
Note: The PR branch is already checked out in the current working directory.
105+
106+
Provide detailed feedback using inline comments for specific issues.
107+
108+
claude_args: |
109+
--allowedTools "mcp__github_inline_comment__create_inline_comment,Bash(gh pr comment:*),Bash(gh pr diff:*),Bash(gh pr view:*)"

CLAUDE.md

Lines changed: 58 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,58 @@
1+
# Bitwarden Internal SDK
2+
3+
Rust SDK centralizing business logic. You're reviewing code as a senior Rust engineer mentoring
4+
teammates.
5+
6+
## Client Pattern
7+
8+
PasswordManagerClient ([bitwarden-pm](crates/bitwarden-pm/src/lib.rs)) wraps
9+
[bitwarden_core::Client](crates/bitwarden-core/src/client/client.rs) and exposes sub-clients:
10+
`auth()`, `vault()`, `crypto()`, `sends()`, `generators()`, `exporters()`.
11+
12+
**Lifecycle**
13+
14+
- Init → Lock/Unlock → Logout (drops instance). Memento pattern for state resurrection.
15+
16+
**Storage**
17+
18+
- Consuming apps use `HashMap<UserId, PasswordManagerClient>`.
19+
20+
## Issues necessitating comments
21+
22+
**Auto-generated code changes**
23+
24+
- Changes to `bitwarden-api-api/` or `bitwarden-api-identity/` are generally discouraged. These are
25+
auto-generated from swagger specs.
26+
27+
**Secrets in logs/errors**
28+
29+
- Do not log keys, passwords, or vault data in logs or error paths. Redact sensitive data.
30+
31+
**Business logic in WASM**
32+
33+
- `bitwarden-wasm-internal` contains only thin bindings. Move business logic to feature crates.
34+
35+
**Unsafe without justification**
36+
37+
- Any `unsafe` block needs a comment explaining why it's safe and what invariants are being upheld.
38+
39+
**Changes to `bitwarden-crypto/` core functionality**
40+
41+
- Generally speaking, this crate should not be modified. Changes need a comment explaining why.
42+
43+
**New crypto algorithms or key derivation**
44+
45+
- Detailed description, review and audit trail required. Document algorithm choice rationale and
46+
test vectors.
47+
48+
**Encryption/decryption modifications**
49+
50+
- Verify backward compatibility. Existing encrypted data must remain decryptable.
51+
52+
**Breaking serialization**
53+
54+
- Backward compatibility required. Users must decrypt vaults from older versions.
55+
56+
**Breaking API changes**
57+
58+
- Document migration path for clients.

0 commit comments

Comments
 (0)