324 lines
16 KiB
Markdown
324 lines
16 KiB
Markdown
# AGENTS.md
|
|
|
|
Guidance for AI coding agents working on this repository.
|
|
This complements [README.md](README.md) (for humans) and [DESIGN.md](DESIGN.md)
|
|
(architecture & design rationale) with agent-focused context. Before making any
|
|
non-trivial architectural change — new screens, navigation changes, theme/state
|
|
patterns, module boundaries — read `DESIGN.md` first and keep changes consistent
|
|
with the decisions recorded there. If a change would contradict `DESIGN.md`,
|
|
flag it explicitly rather than silently diverging.
|
|
|
|
## Project Overview
|
|
|
|
**Hermes Mobile** is a Jetpack Compose Android app — a mobile control panel for
|
|
[Hermes Agent](https://hermes-agent.nousresearch.com). It talks to the Hermes
|
|
dashboard's REST API and WebSocket TUI Gateway (JSON-RPC 2.0) over a trusted LAN.
|
|
|
|
- **Package:** `com.m57.hermescontrol`
|
|
- **Min SDK 26 / Target SDK 37 / Compile SDK 37**
|
|
- **Kotlin 2.4.10**, KSP 2.3.10 (standalone versioning, NOT `kotlinVersion-kspVersion`)
|
|
- **Jetpack Compose**, Room 2.7.x, Navigation3, OkHttp WebSocket, Retrofit, Kotlinx Serialization
|
|
- **Auth:** `EncryptedSharedPreferences` (AES256-GCM), Bearer token (REST) + `?token=` (WS)
|
|
|
|
## Build & Test Commands
|
|
|
|
### ✅ Local Android SDK — if available
|
|
|
|
If you have a local Android SDK (`ANDROID_HOME` set), these work:
|
|
|
|
```bash
|
|
./gradlew assembleDebug # full APK build
|
|
./gradlew testDebugUnitTest # unit tests (MockK)
|
|
./gradlew ktlintCheck # style check
|
|
```
|
|
|
|
**ktlint standalone** (no SDK needed):
|
|
|
|
```bash
|
|
# Download the matching binary
|
|
curl -sSLO https://github.com/pinterest/ktlint/releases/download/1.8.0/ktlint
|
|
chmod +x ktlint
|
|
./ktlint <file> # check one file
|
|
./ktlint --format <file> # auto-fix
|
|
```
|
|
|
|
**No SDK? CI handles everything** — push small and watch the checks below.
|
|
|
|
### CI pipeline (`.github/workflows/android.yml`)
|
|
|
|
| Job | Purpose |
|
|
| -------------------- | ---------------------------------------------------------------------------------------------------------------------------------- |
|
|
| `ktlint` | ktlint 1.8.0 style check + `checkColorLiterals` (hardcoded Color guard, issue #622) |
|
|
| `android-lint` | Android Lint |
|
|
| `unit-tests` | JUnit |
|
|
| `build` | assembleDebug (gated by the 3 above) |
|
|
| `release-compile` | compileReleaseKotlin — catches release-variant compilation issues (e.g. debugImplementation-scoped deps referenced in main source) |
|
|
| `instrumented-tests` | Compose UI tests on Android emulator (API 34 ATD) |
|
|
| `ci-summary` | Aggregator (`if: always()`) — branch protection gates on THIS check |
|
|
|
|
Also: There is a separate workflow `merge-conflict-detector.yml` that auto-labels PRs with `has-conflicts`.
|
|
|
|
Every Gradle job validates `gradle-wrapper.jar` and uses the remote build cache
|
|
(`GRADLE_ENCRYPTED_KEY` secret). `concurrency.cancel-in-progress: true`.
|
|
|
|
### Releasing (`.github/workflows/release.yml`)
|
|
|
|
Triggers on `git push tag v*`. Release APK uses R8 minification + resource shrinking.
|
|
Requires `permissions: contents: write` on the `build-release` job.
|
|
|
|
## Code Style
|
|
|
|
- **ktlint 1.8.0** is enforced in CI. Run `./ktlint --format` before pushing.
|
|
- Import ordering is the #1 CI failure: ktlint enforces ASCII-lexicographic order
|
|
(uppercase before lowercase: `LaunchedEffect` before `collectAsState`).
|
|
- `const val` must use SCREAMING_SNAKE_CASE.
|
|
- Trailing commas required. No trailing whitespace.
|
|
- 120 char max line length.
|
|
|
|
## Architecture Conventions
|
|
|
|
### Navigation (Navigation3 — NOT Navigation Compose)
|
|
|
|
Uses `androidx.navigation3` (`NavKey`, `NavBackStack`, `NavDisplay`, `entry<T>`).
|
|
|
|
**⚠ Always route navigation through `NavigationController.navigateTo()`.** Never call
|
|
`backStack.add(ScreenKey)` directly from UI callbacks — the controller has a
|
|
deduplication guard. Bypassing it stacks duplicate screen entries that compete for
|
|
touch events and become unresponsive.
|
|
|
|
**Drawer gesture state is screen-owned (issue #619).** Each screen declares whether
|
|
the modal drawer's swipe gestures should be active while it is visible, via a single
|
|
source of truth — no global gesture set, no defensive `LaunchedEffect(snapTo(Closed))`,
|
|
no `closeDrawer` callback on `NavigationController`.
|
|
|
|
- `HermesScaffold(drawerGesturesEnabled = true)` — default; primary screens and
|
|
most list screens. The scaffold reconciles this preference into the
|
|
`DrawerGestureController` via a `SideEffect`.
|
|
- `HermesScaffold(drawerGesturesEnabled = false)` — drill-down sub-pages (e.g.
|
|
`SettingsConnectionPage`, `SettingsAppearancePage`, …). The controller closes
|
|
the drawer itself if it was open when the screen composed, so the scrim can't
|
|
stick around and intercept the next tap.
|
|
- `DisableDrawerGestures()` — for entry screens that don't use `HermesScaffold`
|
|
(Landing, AuthLogin, PairingCodeEntry).
|
|
|
|
`ModalNavigationDrawer` in `Navigation.kt` reads `gestureController.enabled`
|
|
(provided via `LocalDrawerGestureController`) and passes it to Material's
|
|
`gesturesEnabled` parameter. To change a screen's gesture behavior, edit the
|
|
screen — not `Navigation.kt`.
|
|
|
|
### Activity-Scoped ViewModels
|
|
|
|
Some ViewModels (e.g. `AuthLoginViewModel`) are created at the Activity scope via
|
|
`viewModel()` at the navigation entry level. This means they **survive navigation**
|
|
and cached state like `connectionSuccess` persists across screen entries.
|
|
|
|
Always pair transient state flags with a `DisposableEffect` cleanup:
|
|
|
|
```kotlin
|
|
DisposableEffect(Unit) {
|
|
onDispose { viewModel.clearTransientState() }
|
|
}
|
|
```
|
|
|
|
The cleanup function should reset self-transitioning flags (`connectionSuccess`,
|
|
`isLoading`, `errorMessage`) so the screen can re-enter cleanly.
|
|
|
|
### WebSocket Lifecycle
|
|
|
|
`HermesWsClient` is a singleton that outlives individual screens. It must be
|
|
explicitly disconnected on logout and reconnected after login:
|
|
|
|
| Event | Action |
|
|
| ------------- | ------------------------------------------------------ |
|
|
| Logout | `HermesWsClient.disconnect()` before clearing tokens |
|
|
| Login success | `HermesWsClient.connect()` after `ApiClient.rebuild()` |
|
|
|
|
The singleton's `connect()` has a guard (`if connected → skip`) so it's safe to
|
|
call unconditionally.
|
|
|
|
### Shared Components
|
|
|
|
- **`HermesScaffold`** — drawer-aware Scaffold + TopAppBar with refresh slot,
|
|
pull-to-refresh, optional snackbar host. New screens MUST use this instead of raw
|
|
`Scaffold`. API: `title: @Composable () -> Unit` (not `String` — wrap in
|
|
`{ Text("...") }`).
|
|
- **`StateViews`** — `LoadingState`, `ErrorState`, `EmptyState`. Every data screen
|
|
must implement all three branches in its `when { }` block.
|
|
|
|
### ⚠ HermesScaffold Padding Foot-Gun
|
|
|
|
**This is the #1 recurring bug in this codebase.** It has been re-introduced on 4+ screens
|
|
across 3+ PRs (Settings, Achievements, Webhooks, Config — PRs #445, #454, #455).
|
|
|
|
**Root cause:** `HermesScaffold` wraps content in an internal `Box(Modifier.padding(paddingValues))`
|
|
that already offsets for the top bar. But it also passes `paddingValues` into the content lambda,
|
|
which looks like it should be applied — and every new screen does exactly that:
|
|
|
|
```kotlin
|
|
HermesScaffold(...) { paddingValues -> // ← scaffold already handles top bar offset via Box
|
|
LazyColumn(
|
|
modifier = Modifier
|
|
.fillMaxSize()
|
|
.padding(paddingValues), // ← ❌ double-stacked top padding
|
|
contentPadding = listContentPadding, // ← extra 8dp, now triple-stacked
|
|
) { ... }
|
|
}
|
|
```
|
|
|
|
**The deeper issue:** Passing `paddingValues` into the lambda implies "you need to use this,"
|
|
when the scaffold has ALREADY pre-applied it in its own outer `Box`. This API design creates
|
|
a natural foot-gun: every developer instinctively adds `.padding(paddingValues)` on inner
|
|
content because it seems correct.
|
|
|
|
**Correct pattern — do NOT apply `paddingValues` on inner content:**
|
|
|
|
```kotlin
|
|
// ✅ List screens:
|
|
LazyColumn(
|
|
modifier = Modifier.fillMaxSize(), // no .padding(paddingValues)!!
|
|
contentPadding = listContentPadding, // this is the ONLY padding needed
|
|
verticalArrangement = listItemSpacing,
|
|
)
|
|
|
|
// ✅ Non-list screens (Column/Box wrapping):
|
|
Column(
|
|
modifier = Modifier.fillMaxSize(), // no .padding(paddingValues)!!
|
|
) { ... }
|
|
|
|
// ✅ Loading/Error/Empty states — these DO need it:
|
|
LoadingState(modifier = Modifier.padding(paddingValues))
|
|
```
|
|
|
|
**Quick test:** If your screen's top gap is wider than CronJobsScreen's, you've double-stacked.
|
|
|
|
**See also:** `references/hermes-scaffold-padding.md` in the skill doc for the full
|
|
breakdown, edge cases, and timeline of previous occurrences.
|
|
|
|
### Room Persistence
|
|
|
|
- `ChatMessageEntity` / `ChatMessageDao` / `HermesDatabase` — chat messages survive
|
|
app kills. `getMessagesForSession()` returns `suspend fun ...: List<ChatMessageEntity>`
|
|
(not `Flow` — the caller controls the coroutine scope).
|
|
- Room 2.7.x requires `room { schemaDirectory("$projectDir/schemas") }` DSL.
|
|
- `ChatViewModel` extends `AndroidViewModel` (needs Application for DB access).
|
|
|
|
### Theme
|
|
|
|
`Theme.kt` uses a preset-based theme system with 6 built-in presets: Default,
|
|
Monochrome, Gruvbox, Catppuccin, AMOLED, and Nord. Each preset provides light
|
|
and dark color schemes plus matching semantic status colors (AMOLED is
|
|
dark-only — light mode falls back to the default theme).
|
|
Every preset is a `ThemePalette` template fill (`PaletteTemplate.kt`): raw
|
|
`PaletteColors` + `HermesStatusColors` mapped into the Material 3
|
|
`ColorScheme` slots, with the error slots derived from the status set.
|
|
|
|
Dynamic color (Material You on API 31+) is controlled by a `useDynamicColors`
|
|
parameter on `HermesControlTheme`, not a compile-time constant. When dynamic
|
|
colors are active they override the preset's `ColorScheme`, but semantic status
|
|
colors (success/warning/error/info) are ALWAYS resolved from the active preset
|
|
via `LocalHermesStatusColors` — never from `MaterialTheme.colorScheme`.
|
|
|
|
Access status colors via `LocalHermesStatusColors.current.success`, not
|
|
`MaterialTheme.colorScheme`.
|
|
|
|
## Git Workflow
|
|
|
|
### PR-Always (ENFORCED)
|
|
|
|
**Every change goes through a PR, targeting `dev`. Never push directly to `dev` (or `main`).**
|
|
|
|
```bash
|
|
git checkout dev && git pull origin dev
|
|
git checkout -b fix/issue-N-description # or feat/...
|
|
# make changes, run ./ktlint --format
|
|
git commit -m "fix(#N): description"
|
|
git push -u origin HEAD
|
|
gh pr create --base dev --title "fix(#N): description" --body "Closes #N"
|
|
```
|
|
|
|
### Commit Conventions — STRICT
|
|
|
|
**⚠ Agent-authored commits:** Do NOT override git authorship (`--author`) with the agent's identity. Use the default git config. No `Co-Authored-By` or `Generated with` trailers.
|
|
|
|
- **Short commits:** subject line + max 2 lines of body. Split into atomic commits rather than writing long bodies.
|
|
- **Conventional Commits** types: `feat`, `fix`, `refactor`, `docs`, `test`, `ci`, `chore`.
|
|
- Bug fixes annotated inline with the tracking issue/regression ID.
|
|
|
|
## Security Considerations
|
|
|
|
- **Cleartext HTTP/WS is intentional** — trusted LAN only. `usesCleartextTraffic="true"`
|
|
and `http://`/`ws://` URLs are by design. Don't "fix" this unless adding HTTPS support.
|
|
- **`HttpLoggingInterceptor.Level.BODY`** must stay gated on `BuildConfig.DEBUG` — it
|
|
prints the Authorization header. (B1)
|
|
- **AuthManager token** is ephemeral — it becomes invalid when the Hermes dashboard
|
|
restarts. Symptom: 401 on every call. Fix: re-extract the token from the running
|
|
gateway, don't assume the stored one is valid.
|
|
- **Release signing** uses a keystore with env-var credentials (`KEYSTORE_PATH`,
|
|
`KEYSTORE_PASSWORD`, `KEY_ALIAS`, `KEY_PASSWORD`). Don't commit keystore files or
|
|
hardcode passwords.
|
|
|
|
## Things to Avoid
|
|
|
|
- Don't run `./gradlew` tasks without an Android SDK — CI handles compilation, lint, and tests.
|
|
- Don't remove `@OptIn(ExperimentalMaterial3Api::class)` from screens that use
|
|
`SegmentedButton`, `PullToRefreshBox`, or other experimental M3 APIs.
|
|
- Don't access `MaterialTheme.colorScheme.*` inside non-`@Composable` lambdas
|
|
(`remember {}`, `buildAnnotatedString`, `LaunchedEffect`). Extract to a local
|
|
`val` at the composable scope first.
|
|
- Don't add new screens without checking if an existing one already covers the
|
|
functionality (28+ screens exist). Extend rather than duplicate.
|
|
- **⚠ Never scope a dependency to `debugImplementation` if its import is used in
|
|
`main/` source code.** The CI `release-compile` job catches this, but save the
|
|
cycle. `okhttp3.logging.HttpLoggingInterceptor` is the classic example — it's
|
|
imported in `ApiClient.kt` (main source) so it must be `implementation`, not
|
|
`debugImplementation`. If you want it debug-only, wrap the usage in
|
|
`if (BuildConfig.DEBUG)` or extract it behind an interface.
|
|
|
|
### Hardcoded Color Guard (issue #622)
|
|
|
|
A custom Gradle task `checkColorLiterals` fails the build if `Color(0x...)` hex
|
|
literals or named `Color.White`/`Color.Black`/etc. appear outside the allowed
|
|
paths (`theme/`, `*Preview.kt`, `PairingScreen.kt`, `AuthLoginScreen.kt`).
|
|
|
|
Use `MaterialTheme.colorScheme.<token>`, `LocalHermesStatusColors.current.<semantic>`,
|
|
or `Color.Transparent` / `Color.Unspecified` instead. The guard runs in CI as part
|
|
of the `ktlint` job.
|
|
|
|
## Project Layout
|
|
|
|
```
|
|
com.m57.hermescontrol/
|
|
├── data/
|
|
│ ├── config/ ServerStore, ConnectionProfile, migrations
|
|
│ ├── local/ AuthManager, Room (ChatMessageEntity/Dao, HermesDatabase), AnalyticsCacheStore
|
|
│ ├── model/ 40+ data classes for API responses + requests
|
|
│ ├── remote/ ApiClient, Retrofit service, OkHttp provider, cookie management
|
|
│ ├── session/ ActiveSessionHolder
|
|
│ └── ws/ HermesWsClient, JSON-RPC models, WsEvent, BillingRepository
|
|
├── notification/ ChatNotificationService, NotificationReplyReceiver
|
|
├── theme/ Color, Theme, Motion, Spacing, Shapes, Type, HermesStatusColors
|
|
│ └── presets/ Default, Monochrome, Gruvbox, Catppuccin, AMOLED, Nord
|
|
├── ui/
|
|
│ ├── common/ HermesScaffold, StateViews, SharedComponents, DetailDialog, DetailRows
|
|
│ └── 28 feature packages (achievements, analytics, authlogin, billing, channels,
|
|
│ chat, config, connect, cron, gateway, kanban, keys, landing, logs, mcp,
|
|
│ model, pairing, plugins, process, profiles, providers, sessions, settings,
|
|
│ skills, system, toolsets, webhooks)
|
|
├── util/ CronExpressionFormatter, LocaleContextWrapper
|
|
├── HermesControlApp.kt Application class
|
|
├── Navigation.kt Drawer + NavDisplay + entry wiring
|
|
├── NavigationController.kt Central navigation guard (dedup)
|
|
├── NavigationKeys.kt @Serializable NavKey data objects (28 screens + 6 settings sub-pages)
|
|
├── ScreenRegistry.kt entry<T> registrations for all NavKeys
|
|
└── MainActivity.kt
|
|
```
|
|
|
|
## Further Reading
|
|
|
|
- [README.md](README.md) — human-facing overview, features, screenshots, tech stack
|
|
- [DESIGN.md](DESIGN.md) — architecture and design decisions, rationale for the
|
|
patterns in this doc (Navigation3, HermesScaffold, theme presets, etc.)
|
|
- [CONTRIBUTING.md](CONTRIBUTING.md) — contributor workflow, PR checklist, code style
|
|
- [.github/workflows/android.yml](.github/workflows/android.yml) — CI pipeline source of truth
|
|
- [.github/workflows/merge-conflict-detector.yml](.github/workflows/merge-conflict-detector.yml) — auto-labels conflicting PRs
|