Skip to content

Commit cadee5f

Browse files
authored
Merge pull request #4 from CybotTM/fix/code-quality-cleanup
refactor: extract shared rate limiter, standardize error codes, name constants
2 parents 597d338 + 406f90e commit cadee5f

13 files changed

Lines changed: 482 additions & 242 deletions

File tree

AGENTS.md

Lines changed: 55 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -4,33 +4,61 @@
44

55
# AGENTS.md
66

7-
**Precedence:** The **closest AGENTS.md** to changed files wins. Root holds global defaults only.
7+
**Precedence:** The **closest `AGENTS.md`** to the files you're changing wins. Root holds global defaults only.
88

9-
## Project Overview
9+
## Overview
1010

1111
KidSync -- privacy-first co-parenting coordination app with E2E encryption.
1212
Local-first, append-only OpLog. Server is a dumb encrypted relay (cannot decrypt user data).
1313

14-
| Component | Stack | Entry Point |
15-
|-----------|-------|-------------|
16-
| Server | Kotlin 2.1.0, Ktor 3.0.3, Exposed ORM, SQLite WAL, JDK 21 | `server/.../Application.kt` |
17-
| Android | Kotlin, Jetpack Compose, Room + SQLCipher, BouncyCastle, Hilt | `android/.../ui/MainActivity.kt` |
18-
| Specs | Markdown + YAML + JSON test vectors | `docs/`, `tests/conformance/` |
14+
## Commands
1915

20-
## Global Rules
16+
| Task | Command | ~Time |
17+
|------|---------|-------|
18+
| Server tests | `docker run --rm -v "$(pwd)/server:/app" -w /app gradle:8.12-jdk21 gradle test --no-daemon` | ~90s |
19+
| Conformance | `python3 tests/conformance/verify_conformance.py` | ~2s |
20+
| Server build | `docker run --rm -v "$(pwd)/server:/app" -w /app gradle:8.12-jdk21 gradle buildFatJar --no-daemon` | ~60s |
21+
| Docker image | `docker build -t kidsync-server server/` | ~120s |
22+
| Android tests | `cd android && ./gradlew test` | ~120s |
23+
| Android build | `cd android && ./gradlew assembleDebug` | ~90s |
2124

22-
- Conventional Commits: `type(scope): subject`
23-
- All data models use `kotlinx.serialization`
24-
- Hash chain: `SHA256(hexDecode(prevHash) + base64Decode(encryptedPayload))`
25-
- AAD format: `familyId|deviceId|deviceSequence|keyEpoch`
26-
- No Java locally -- server tests run via Docker
25+
## File Map
26+
27+
```
28+
server/ # Kotlin/Ktor relay server (routes, services, DB, plugins)
29+
android/ # Jetpack Compose Android app (crypto, data, domain, UI)
30+
docs/ # Protocol specs (wire-format, sync-protocol, encryption-spec), OpenAPI, reviews
31+
tests/ # Conformance test vectors (JSON + Python verifier)
32+
```
33+
34+
## Golden Samples
35+
36+
| For | Reference | Key patterns |
37+
|-----|-----------|--------------|
38+
| Route handler | `server/.../routes/AuthRoutes.kt` | Service delegation, ApiException, rate limiter |
39+
| DB transaction | `server/.../db/DatabaseFactory.kt` | Explicit DB ref, `throw ApiException` for rollback |
40+
| Use case | `android/.../usecase/sync/SyncOpsUseCase.kt` | Repository injection, suspend, error handling |
41+
| ViewModel | `android/.../viewmodel/AuthViewModel.kt` | `@HiltViewModel`, StateFlow, viewModelScope |
42+
| Composable | `android/.../screens/auth/LoginScreen.kt` | Material 3, collectAsStateWithLifecycle |
43+
44+
## Heuristics
45+
46+
| When | Do |
47+
|------|-----|
48+
| Adding server endpoint | Follow Route -> Service -> dbQuery pattern |
49+
| Touching crypto code | Ask first -- protocol spec changes need approval |
50+
| Adding dependency | Ask first -- we minimize deps |
51+
| Unsure about pattern | Check Golden Samples above |
52+
| Exposed `deleteWhere` with `<`, `<=` | Explicit import: `import org.jetbrains.exposed.sql.SqlExpressionBuilder.lessEq` |
2753

2854
## Boundaries
2955

3056
### Always Do
31-
- Run server tests before committing: `docker run --rm -v "$(pwd)/server:/app" -w /app gradle:8.12-jdk21 gradle test --no-daemon`
57+
- Run server tests before committing (see Commands above)
3258
- Use `throw ApiException(...)` inside `dbQuery {}` blocks (NOT `return Result.failure`)
3359
- Pass explicit database reference in Exposed transactions
60+
- Use conventional commit format: `type(scope): subject`
61+
- Show test output as evidence before claiming work is complete
3462

3563
### Ask First
3664
- Changing protocol specs, database schema, encryption/key management code, or dependencies
@@ -41,36 +69,23 @@ Local-first, append-only OpLog. Server is a dumb encrypted relay (cannot decrypt
4169
- Use `return@dbQuery Result.failure()` inside transactions (breaks rollback)
4270
- Leak exception details to clients
4371

44-
## Security
72+
## Terminology
4573

46-
- E2E encrypted: X25519 key agreement + AES-256-GCM
47-
- Auth: Ed25519 challenge-response. Sessions: opaque tokens (1h TTL)
48-
- CORS restricted via `KIDSYNC_CORS_ORIGINS` env var
49-
- Rate limiting per endpoint. `FLAG_SECURE` on sensitive screens.
50-
51-
## Testing (456 server tests, 881+ Android tests)
52-
53-
```bash
54-
docker run --rm -v "$(pwd)/server:/app" -w /app gradle:8.12-jdk21 gradle test --no-daemon
55-
python3 tests/conformance/verify_conformance.py # Conformance vectors
56-
```
57-
58-
## Documentation
59-
60-
| Document | Path |
61-
|----------|------|
62-
| Specs | `docs/protocol/wire-format.md`, `sync-protocol.md`, `encryption-spec.md` |
63-
| OpenAPI | `docs/api/openapi.yaml` |
64-
| Reviews | `docs/reviews/phase1-review.md`, `phase4-6-review.md`, `phase7-server-review.md` |
65-
| Ops | `docs/disaster-recovery.md`, `docker-compose.yml`, `.env.example` |
74+
| Term | Means |
75+
|------|-------|
76+
| OpLog | Append-only operation log (encrypted ops synced between devices) |
77+
| Bucket | A family's data container (replaces "family" in sync context) |
78+
| DEK | Data Encryption Key -- AES-256-GCM key, wrapped per device |
79+
| AAD | Additional Authenticated Data: `familyId\|deviceId\|deviceSequence\|keyEpoch` |
80+
| Hash chain | `SHA256(hexDecode(prevHash) + base64Decode(encryptedPayload))` |
6681

67-
## Index of Scoped AGENTS.md
82+
## Index of scoped AGENTS.md
6883

6984
| Path | Scope |
7085
|------|-------|
71-
| `server/AGENTS.md` | Ktor sync server (routes, services, DB, plugins) |
72-
| `android/AGENTS.md` | Android app (crypto, data, domain, UI) |
86+
| [server/AGENTS.md](./server/AGENTS.md) | Ktor sync server -- routes, services, DB, plugins, rate limiting |
87+
| [android/AGENTS.md](./android/AGENTS.md) | Android app -- crypto, data layer, domain logic, Compose UI |
7388

74-
## When Instructions Conflict
89+
## When instructions conflict
7590

76-
Nearest AGENTS.md wins. User prompts override files. Protocol specs override implementation code.
91+
Nearest `AGENTS.md` wins. User prompts override files. Protocol specs override implementation code.

CONTRIBUTING.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -44,7 +44,7 @@ docker run --rm -v "$(pwd)/server:/app" -w /app gradle:8.12-jdk21 gradle buildFa
4444

4545
## Testing Requirements
4646

47-
- Server: All 456 tests must pass (`gradle test`)
47+
- Server: All 464 tests must pass (`gradle test`)
4848
- Android: Unit tests must pass
4949
- New features should include appropriate test coverage
5050

android/AGENTS.md

Lines changed: 111 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -1,22 +1,46 @@
11
<!-- FOR AI AGENTS - Scoped to android/ -->
2-
<!-- Last updated: 2026-02-25 -->
2+
<!-- Managed by agent: keep sections and order; edit content, not structure -->
3+
<!-- Last updated: 2026-02-25 | Last verified: 2026-02-25 -->
34

45
# Android AGENTS.md
56

7+
## Overview
8+
69
Jetpack Compose Android app. Local-first with E2E encrypted sync to Ktor server.
10+
Clean architecture (UI -> Domain -> Data -> Crypto), Hilt DI, Room + SQLCipher, WorkManager sync.
711

8-
## Stack
12+
**Stack:** Kotlin, Jetpack Compose, Room + SQLCipher, BouncyCastle + JCA, Hilt, WorkManager, Retrofit, min SDK 26, target SDK 35
913

10-
Kotlin, Jetpack Compose, Room + SQLCipher, BouncyCastle + JCA (crypto), Hilt (DI), WorkManager, Retrofit, min SDK 26, target SDK 35
14+
## Setup
1115

12-
## Package Map
16+
- Android Studio with Kotlin plugin
17+
- JDK 17 for compilation
18+
- No server dependency for unit tests (mocked)
19+
20+
Release signing reads from env vars or `local.properties`:
21+
- `KIDSYNC_KEYSTORE_PATH` / `keystore.path`
22+
- `KIDSYNC_KEYSTORE_PASSWORD` / `keystore.password`
23+
- `KIDSYNC_KEY_ALIAS` / `key.alias`
24+
- `KIDSYNC_KEY_PASSWORD` / `key.password`
25+
26+
Only activates when all 4 values present.
27+
28+
## Commands
29+
30+
| Task | Command | ~Time |
31+
|------|---------|-------|
32+
| Test (all) | `./gradlew test` | ~120s |
33+
| Build debug | `./gradlew assembleDebug` | ~90s |
34+
| Build release | `./gradlew assembleRelease` | ~90s |
35+
36+
## File Map
1337

1438
```
1539
app/src/main/java/com/kidsync/app/
1640
KidSyncApplication.kt # @HiltAndroidApp
1741
crypto/
1842
CryptoManager.kt # Interface + buildPayloadAad() helper
19-
TinkCryptoManager.kt # AES-256-GCM encrypt/decrypt, X25519 (uses BouncyCastle + JCA)
43+
TinkCryptoManager.kt # AES-256-GCM encrypt/decrypt, X25519 (BouncyCastle + JCA)
2044
data/
2145
local/ # Room DB, DAOs, entities, converters
2246
remote/api/ # ApiService (Retrofit), DTOs
@@ -36,56 +60,106 @@ app/src/main/java/com/kidsync/app/
3660
navigation/ # NavGraph.kt, Routes.kt (sealed class)
3761
screens/ # auth/, calendar/, dashboard/, expense/, family/, settings/
3862
theme/ # Color, Type, Theme (Material 3 + dynamic colors)
39-
viewmodel/ # AuthViewModel, CalendarViewModel, ExpenseViewModel, FamilyViewModel, SettingsViewModel
63+
viewmodel/ # AuthVM, CalendarVM, ExpenseVM, FamilyVM, SettingsVM
4064
```
4165

42-
## Commands
66+
## Testing (881+ tests)
4367

44-
| Command | Purpose |
45-
|---------|---------|
46-
| Open `android/` in Android Studio | IDE setup |
47-
| `./gradlew test` | Run unit tests |
48-
| `./gradlew assembleRelease` | Build release APK (requires signing config) |
49-
| `./gradlew assembleDebug` | Build debug APK |
68+
All tests are unit tests using JUnit, Mockito/MockK, and Kotlin coroutines test.
5069

51-
## Architecture Layers
70+
Test pattern: mock repositories/DAOs, inject into use case or ViewModel, assert StateFlow emissions.
5271

53-
```
54-
UI (Compose screens + ViewModels)
55-
↓ StateFlow + collectAsStateWithLifecycle
56-
Domain (use cases, models, repository interfaces)
57-
↓ suspend functions
58-
Data (Room DAOs, Retrofit API, repository impls, SyncWorker)
59-
60-
Crypto (BouncyCastle + JCA: AES-256-GCM, X25519, HKDF, BIP39)
61-
```
72+
## Code Style
6273

63-
All ViewModels: `@HiltViewModel`, `viewModelScope`, `MutableStateFlow`/`StateFlow`
74+
- `@HiltViewModel` + `viewModelScope` + `MutableStateFlow`/`StateFlow` for all ViewModels
75+
- `collectAsStateWithLifecycle` in Composables (never `collectAsState`)
76+
- Clean architecture: UI -> Domain -> Data (repositories bridge the layers)
77+
- `SharedPreferences.commit()` for security-critical state, `.apply()` only for non-critical
78+
- Sealed class `Routes` with `data object` entries for navigation
79+
- `popUpTo` with `inclusive = true` at auth flow transitions
6480

65-
## Critical Patterns
81+
## Security
6682

67-
**AAD construction**: `CryptoManager.buildPayloadAad(familyId, deviceId, deviceSequence, keyEpoch)` -> `"familyId|deviceId|deviceSequence|keyEpoch"`
83+
- **E2E encryption:** AES-256-GCM with X25519 key agreement, HKDF key derivation
84+
- **Key storage:** Android Keystore for session keys, SQLCipher for Room DB encryption
85+
- **Recovery:** BIP39 mnemonic seed phrase (12 words)
86+
- **Nonce:** Random 12 bytes prepended to ciphertext: `nonce(12) || ciphertext || tag`, then Base64
87+
- **AAD:** `CryptoManager.buildPayloadAad(familyId, deviceId, deviceSequence, keyEpoch)` -> `"a|b|c|d"`
88+
- **Hash chain:** `HashChainVerifier.computeHash(prevHash, encryptedPayload)` = `SHA256(hexDecode(prevHash) + base64Decode(encryptedPayload))`
89+
- **FLAG_SECURE** on sensitive screens
90+
- **Key zeroing:** Caller owns the key -- don't zero input parameters in decrypt methods
6891

69-
**Nonce**: Random 12 bytes prepended to ciphertext: `nonce(12) || ciphertext || tag`, then Base64-encoded
92+
## Critical Patterns
7093

71-
**Hash chain**: `HashChainVerifier.computeHash(prevHash, encryptedPayload)` = `SHA256(hexDecode(prevHash) + base64Decode(encryptedPayload))`
94+
**Architecture layers:**
95+
```
96+
UI (Compose screens + ViewModels)
97+
-> StateFlow + collectAsStateWithLifecycle
98+
Domain (use cases, models, repository interfaces)
99+
-> suspend functions
100+
Data (Room DAOs, Retrofit API, repository impls, SyncWorker)
101+
-> Crypto (BouncyCastle + JCA)
102+
```
72103

73-
**Navigation**: Sealed class `Routes` with `data object` entries. `popUpTo` with `inclusive = true` at auth flow transitions.
104+
**Sync backends** (all transfer already-encrypted ops -- zero-knowledge maintained):
105+
- Server relay (default): REST API push/pull
106+
- File export/import: ZIP `.kidsync` bundles via SAF
107+
- WebDAV/NextCloud: OkHttp PROPFIND/MKCOL/PUT/GET, WorkManager periodic
108+
- P2P local: Google Nearby Connections, HMAC-SHA256 handshake
109+
110+
## Examples
111+
112+
Good -- ViewModel with proper state management:
113+
```kotlin
114+
@HiltViewModel
115+
class ExampleViewModel @Inject constructor(
116+
private val useCase: ExampleUseCase,
117+
) : ViewModel() {
118+
private val _state = MutableStateFlow(ExampleUiState())
119+
val state: StateFlow<ExampleUiState> = _state.asStateFlow()
120+
121+
fun onAction() {
122+
viewModelScope.launch {
123+
_state.update { it.copy(loading = true) }
124+
useCase.execute().fold(
125+
onSuccess = { _state.update { s -> s.copy(data = it, loading = false) } },
126+
onFailure = { _state.update { s -> s.copy(error = it.message, loading = false) } },
127+
)
128+
}
129+
}
130+
}
131+
```
74132

75-
## Signing (Release)
133+
Bad -- bypassing repository, direct DAO in ViewModel:
134+
```kotlin
135+
@HiltViewModel
136+
class BadViewModel @Inject constructor(
137+
private val dao: ExampleDao, // Should use repository/use case
138+
) : ViewModel() { /* ... */ }
139+
```
76140

77-
Reads from env vars or `local.properties`:
78-
- `KIDSYNC_KEYSTORE_PATH` / `keystore.path`
79-
- `KIDSYNC_KEYSTORE_PASSWORD` / `keystore.password`
80-
- `KIDSYNC_KEY_ALIAS` / `key.alias`
81-
- `KIDSYNC_KEY_PASSWORD` / `key.password`
141+
## Checklist
82142

83-
Only activates when all 4 values present.
143+
Before committing Android changes:
144+
- [ ] `./gradlew test` passes
145+
- [ ] ViewModels use `@HiltViewModel` + `StateFlow` (not LiveData)
146+
- [ ] Composables use `collectAsStateWithLifecycle` (not `collectAsState`)
147+
- [ ] Security-critical SharedPreferences use `.commit()` (not `.apply()`)
148+
- [ ] Crypto key zeroing: caller owns the key, don't zero input params
149+
- [ ] New screens with sensitive data use `FLAG_SECURE`
84150

85151
## Known Tech Debt
86152

87153
- CalendarViewModel is 743 lines (SRP violation, should split)
88-
- ExpenseViewModel bypasses repository, injects DAO directly
89154
- Monolithic UI state objects (AuthUiState: 16 fields, CalendarUiState: 25 fields)
90155
- In-memory expense filtering instead of SQL WHERE
91156
- No sync status UI indicator (offline/syncing/synced)
157+
158+
## When Stuck
159+
160+
1. Check `di/` modules for what's provided by Hilt (DatabaseModule, NetworkModule, CryptoModule, AppModule)
161+
2. Check `domain/repository/` interfaces for available data operations
162+
3. Check `data/local/dao/` for Room query patterns
163+
4. Check `ui/navigation/Routes.kt` for all navigation destinations
164+
5. Check `crypto/CryptoManager.kt` interface for encryption API
165+
6. Run `./gradlew test` -- failures give clear assertion messages

0 commit comments

Comments
 (0)