[PM-33572] llm: Add implementing-ios-code skill with Swift templates by SaintPatrck · Pull Request #2446 · bitwarden/ios

SaintPatrck · 2026-03-13T19:52:35Z

🎟️ Tracking

https://bitwarden.atlassian.net/browse/PM-33572

📔 Objective

Adds a Claude skill for implementing Bitwarden iOS features following established patterns.

What changed:

skills/implementing-ios-code/SKILL.md — Step-by-step implementation workflow: determines scope from a design doc, builds the Core layer first (models, services, repositories), then the full UI file-set (Coordinator, Processor, State, Action, Effect, View), wires DI into ServiceContainer, runs a security checklist (zero-knowledge, Keychain, InputValidator, NonLoggableError, extension memory), and ensures DocC documentation. References Docs/Architecture.md as authoritative and does not duplicate it.
skills/implementing-ios-code/templates.md — Minimal copy-paste skeletons derived from actual codebase files: Coordinator, Processor, State/Action/Effect, View (with store.binding and store.perform), Service (protocol + Default* + Has* + // sourcery: AutoMockable), and Repository. Each template notes the source file it was based on.

github-actions · 2026-03-13T19:59:44Z

Checkmarx One – Scan Summary & Details – 195d6e23-5994-41c4-aeef-d21a91832600

Great job! No new security vulnerabilities introduced in this pull request

claude · 2026-03-13T20:05:08Z

Claude finished @SaintPatrck's task in 7m 27s —— View job

I'll analyze this and get back to you.

codecov · 2026-03-13T20:06:08Z

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 85.95%. Comparing base (8bf3557) to head (642a37e).
⚠️ Report is 2 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #2446      +/-   ##
==========================================
- Coverage   87.09%   85.95%   -1.14%     
==========================================
  Files        1859     2089     +230     
  Lines      164348   179153   +14805     
==========================================
+ Hits       143132   153995   +10863     
- Misses      21216    25158    +3942

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

claude · 2026-03-13T20:10:22Z

Overall Assessment: APPROVE

Reviewed 2 new files adding an LLM skill for implementing Bitwarden iOS features with step-by-step workflow and code templates.

Code Review Details

⚠️ IMPORTANT: Coordinator template declares stackNavigator as non-optional and non-weak, creating retain cycles
- .claude/skills/implementing-ios-code/templates.md line 46
❓ QUESTION: The referenced planning-ios-implementation skill does not exist in the repository
- .claude/skills/implementing-ios-code/SKILL.md line 12

claude · 2026-03-13T20:12:20Z

Inline Findings

Finding 1 - ⚠️ IMPORTANT

File: .claude/skills/implementing-ios-code/templates.md line 46

Coordinator template declares stackNavigator as non-optional and non-weak, which will cause retain cycles

The HasStackNavigator protocol (BitwardenKit/UI/Platform/Application/Utilities/Coordinator.swift lines 95-98) defines stackNavigator as StackNavigator? and its documentation explicitly states: This value should be weak, otherwise a retain cycle might be introduced.

Every coordinator in the codebase uses:

private(set) weak var stackNavigator: StackNavigator?

The template shows var stackNavigator: StackNavigator (non-optional, non-weak). Since weak requires optionality in Swift, code generated from this template cannot be marked weak, creating retain cycles between coordinators and navigators.

This also means line 78 should use optional chaining: stackNavigator?.push(view) instead of stackNavigator.push(view).

Fix: Change line 46 to private(set) weak var stackNavigator: StackNavigator? and line 78 to stackNavigator?.push(view).

Finding 2 - ❓ QUESTION

File: .claude/skills/implementing-ios-code/SKILL.md line 12

The referenced planning-ios-implementation skill does not exist in the repository

This line instructs the LLM to invoke planning-ios-implementation if no plan exists, but that skill is not present in the .claude/skills/ directory. An LLM following this skill would fail at this prerequisite step.

Is this skill planned for a future PR, or should this reference be updated?

- SKILL.md: step-by-step implementation workflow (Core layer first, then UI file-set, DI wiring, security checklist, documentation); prerequisites check for design doc and Docs/Architecture.md - templates.md: minimal copy-paste skeletons derived from actual codebase files — Coordinator, Processor, State/Action/Effect, View, Service, Repository; each template references the source file it was based on

github-actions · 2026-04-08T13:37:55Z

Bitwarden Claude Code Review

Overall Assessment: APPROVE

Reviewed 2 new markdown files adding an LLM skill for implementing Bitwarden iOS features. SKILL.md provides a step-by-step workflow (scope determination, core layer, UI file-set, DI wiring, security checklist, documentation) and templates.md provides copy-paste skeletons for Coordinator, Processor, State/Action/Effect, View, Service, and Repository patterns. The latest commit addresses prior review feedback on persistence guidance, security checklist terminology, MARK conventions, and coordinator creation heuristics. Templates correctly reflect codebase conventions including the weak optional stackNavigator pattern.

Code Review Details

No new findings. Prior review threads have been addressed or acknowledged by the author.

.claude/skills/implementing-ios-code/SKILL.md

KatherineInCode · 2026-04-08T13:43:36Z

.claude/skills/implementing-ios-code/SKILL.md

+**Data Models** (if needed)
+- Request/Response types in `Core/<Domain>/Models/Request/` and `Response/`
+- Enum types in `Core/<Domain>/Models/Enum/`
+- CoreData entities only if persistence is needed (add to `Bitwarden.xcdatamodeld`)


🤔 We basically never add anything to Core Data. If something's stored on disk, it's either in UserDefaults or the Keychain, depending on how sensitive of data it is.

Maybe this should be

- More-sensitive information persisted to disk is in `KeychainRepository`. Less-sensitive information persisted to disk is in `AppSettingsStore`. Both of these are exposed through `StateService`. Try to use a separate protocol instead of adding to the `StateService, `AppSettingsStore`, and `KeychainRepository` protocols, to ensure interface segregation.

Rephrased. LMK what you think.

KatherineInCode · 2026-04-08T13:44:19Z

.claude/skills/implementing-ios-code/SKILL.md

+
+Before finishing:
+- [ ] Vault data? → Must use `BitwardenSdk` for all encryption/decryption
+- [ ] Storing credentials? → Must use `KeychainRepository`, not `UserDefaults`


Suggested change

- [ ] Storing credentials? → Must use `KeychainRepository`, not `UserDefaults`

- [ ] Storing credentials? → Must use `KeychainRepository`, not `AppSettingsStore`

KatherineInCode · 2026-04-08T13:46:01Z

.claude/skills/implementing-ios-code/SKILL.md

+
+All new public types and methods require DocC (`///`) documentation.
+Exceptions: protocol property/function implementations (docs live in the protocol), mock classes.
+Use `// MARK: -` sections to organize code within files.


Suggested change

Use `// MARK: -` sections to organize code within files.

Use pragma marks to organize code. `// MARK: -` is used to denote different objects in the same file; `// MARK:` is used to denote different sections within an object.

KatherineInCode · 2026-04-08T13:47:14Z

.claude/skills/implementing-ios-code/templates.md

+When adding a new screen, create these files (replace `<Feature>` throughout):
+
+```
+BitwardenShared/UI/<Domain>/<Feature>/


🤔 Sometimes a new screen requires a coordinator, but not always; often one coordinator manages multiple screens (and therefore multiple processors). I'm not sure how best to capture that judgement call here.

Added detail about this, giving guidance on how to make the "correct" decision. LMK what you think.

Oh, that sounds good.

…onventions, coordinator heuristic

KatherineInCode · 2026-04-08T15:00:27Z

.claude/skills/implementing-ios-code/templates.md

+When adding a new screen, create these files (replace `<Feature>` throughout):
+
+```
+BitwardenShared/UI/<Domain>/<Feature>/


Oh, that sounds good.

github-actions bot added the t:llm Change Type - LLM related change (e.g. CLAUDE.md files) label Mar 13, 2026

SaintPatrck added the ai-review Request a Claude code review label Mar 13, 2026

SaintPatrck added 2 commits April 8, 2026 09:01

Fix coordinator template retain cycle: use weak optional stackNavigator

8668f72

SaintPatrck force-pushed the llm/implementing-ios-code branch from 6f815a6 to 8668f72 Compare April 8, 2026 13:36

claude bot reviewed Apr 8, 2026

View reviewed changes

.claude/skills/implementing-ios-code/SKILL.md Show resolved Hide resolved

KatherineInCode reviewed Apr 8, 2026

View reviewed changes

Address code review: persistence guidance, security checklist, MARK c…

642a37e

…onventions, coordinator heuristic

SaintPatrck requested a review from KatherineInCode April 8, 2026 14:59

SaintPatrck marked this pull request as ready for review April 8, 2026 14:59

SaintPatrck requested review from a team as code owners April 8, 2026 14:59

SaintPatrck enabled auto-merge (squash) April 8, 2026 14:59

KatherineInCode approved these changes Apr 8, 2026

View reviewed changes

SaintPatrck merged commit 262e93f into main Apr 8, 2026
41 checks passed

SaintPatrck deleted the llm/implementing-ios-code branch April 8, 2026 15:27

	- [ ] Storing credentials? → Must use `KeychainRepository`, not `UserDefaults`
	- [ ] Storing credentials? → Must use `KeychainRepository`, not `AppSettingsStore`

	Use `// MARK: -` sections to organize code within files.
	Use pragma marks to organize code. `// MARK: -` is used to denote different objects in the same file; `// MARK:` is used to denote different sections within an object.

Conversation

SaintPatrck commented Mar 13, 2026

🎟️ Tracking

📔 Objective

Uh oh!

github-actions bot commented Mar 13, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Great job! No new security vulnerabilities introduced in this pull request

Uh oh!

claude bot commented Mar 13, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

codecov bot commented Mar 13, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

claude bot commented Mar 13, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

claude bot commented Mar 13, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Inline Findings

Finding 1 - ⚠️ IMPORTANT

Finding 2 - ❓ QUESTION

Uh oh!

github-actions bot commented Apr 8, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Bitwarden Claude Code Review

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

github-actions bot commented Mar 13, 2026 •

edited

Loading

claude bot commented Mar 13, 2026 •

edited

Loading

codecov bot commented Mar 13, 2026 •

edited

Loading

claude bot commented Mar 13, 2026 •

edited

Loading

claude bot commented Mar 13, 2026 •

edited

Loading

github-actions bot commented Apr 8, 2026 •

edited

Loading