Merge branch 'stage' into ADFA-2686-consistent-feedback-FAB

Author: hal-eisen-adfa

.claude/skills/architecture-review/SKILL.md

@@ -0,0 +1,87 @@
+---
+name: architecture-review
+description: Review a code change against Code On The Go's architecture — ARCHITECTURE.md and the ADRs in docs/adr/. Forces a read of those authoritative docs (they are NOT reliably in context otherwise), then checks the diff against UDF/state, Koin DI, Room-vs-SQLite, Compose, module boundaries, ABI flavors, dependency substitution, @Parcelize, and strings placement, reporting violations with the ADR/section each comes from. Use when asked to review architecture alignment, check a change against ARCHITECTURE.md/the ADRs, or as the §10 step of a code review.
+metadata:
+  author: Hal Eisen
+  keywords:
+  - architecture
+  - review
+  - adr
+  - udf
+  - code-review
+  - codeonthego
+---
+
+## Why this is a skill (not just REVIEW.md §10)
+
+ARCHITECTURE.md (~4.9k tokens) and the ADRs (~7.3k) are **not** in context during normal work, and prose links in REVIEW.md are not reliably followed. This skill exists to **guarantee the read**: it opens the authoritative docs, then checks the diff against them. Do not review architecture from memory or from the summary in this file alone.
+
+## When to invoke
+
+- "Review this against the architecture / ARCHITECTURE.md / the ADRs."
+- "Does this change follow our patterns?" / architecture-alignment review.
+- As the §10 step of a full code review — its output feeds REVIEW.md's evidence ledger.
+
+## Step 1 — Scope the diff
+
+Pick the target and get the changed files + hunks:
+- Working tree: `git diff --stat` and `git diff`.
+- This branch vs the integration branch: `git diff origin/stage...HEAD` (feature branches are based on `stage`).
+- A GitHub PR: `gh pr diff <N>`.
+
+**Exclude vendored/generated code** — it's not held to our patterns: `composite-builds/build-deps*`, `subprojects/{aaptcompiler,builder-model-impl,flashbar,xml-dom}`, `termux/`, `eventbus/`, `LayoutEditor/`, `**/build/`, generated `R`/`BuildConfig`. Review only first-party Kotlin/Java/XML/Gradle changes.
+
+## Step 2 — READ the authoritative docs (mandatory)
+
+Before judging anything, read:
+- `ARCHITECTURE.md` (whole file — module map, dependency rules, tech stack, **State Management**, testing).
+- **Every** file in `docs/adr/*.md`. At minimum open the ones a diff can plausibly violate: 0001 (Room), 0003 (substitution), 0005 (flavors), 0006 (Koin), 0009 (Compose). Read the rest if the change is broad.
+
+Do not skip this because the rules "look familiar" — they are the source of truth and they change.
+
+## Step 3 — Check the diff against the rules
+
+For each changed first-party file, check the applicable rules. Each rule cites its source so findings are traceable.
+
+| # | Rule | Source |
+|---|---|---|
+| 1 | **UDF:** new screens use `ViewModel` + `StateFlow<UiState>`, sealed `UiEvent`/`UiEffect`, a repository for data; composables collect via `collectAsStateWithLifecycle()` (lifecycle-aware, Android's strongly-recommended default; `collectAsState()` is only for platform-agnostic/KMP code, which we don't have); no I/O or business logic in composables/Activities/Fragments. | ARCHITECTURE.md → State Management |
+| 2 | **Sealed state for mutually-exclusive states** (loading/content/error/…): not a `data class` of independent `Boolean`s that can contradict each other ("boolean hell"). | ARCHITECTURE.md → State Management |
+| 3 | **Koin DI**, constructor injection; register new singletons/ViewModels in the module. No hand-rolled singletons/service locators (the documented `ServiceLocator` aside). | ADR 0006 |
+| 4 | **Persistence:** Room is the default for relational data; raw SQLite only for a justified exception (prebuilt read-only DB, perf/allocation-critical indexing, cross-boundary schema) — and the PR must say which. Non-relational → filesystem/preferences (DataStore). | ADR 0001 |
+| 5 | **`@Parcelize`** (`kotlin-parcelize`) for `Parcelable`; never hand-implement it unless Parcelize genuinely can't. | ARCHITECTURE.md → Parceling |
+| 6 | **New UI is Jetpack Compose** — a new XML-layout / `Fragment`-rendered screen for the IDE's own UI is a violation (existing XML screens are fine until reworked). | ADR 0009 |
+| 7 | **Module boundaries / dependency direction:** UI → ViewModel → Repository → data source; features depend on `common`/`utils`, not the reverse; no new cross-feature or upward dependency. | ARCHITECTURE.md → module map |
+| 8 | **ABI flavors:** new Android modules get `v7`/`v8` via `composite-builds/build-logic` centrally — no per-module flavor blocks, no flavorless `assembleDebug`. (`:plugin-api` is intentionally flavorless.) | ADR 0005 |
+| 9 | **Dependency substitution:** don't add a Maven coordinate for something already vendored/substituted (`build-deps*`); don't add a new dependency without checking `gradle/libs.versions.toml` first. | ADR 0003 |
+| 10 | **Strings** live in the `:resources` module's `strings.xml` (not per-module, not inline literals). | REVIEW.md §7 |
+| 11 | **UI never drawn over the two system bars** (top status bar, bottom navigation bar). | CLAUDE.md |
+
+Rules 1, 2, 6 apply to UI changes; 4, 5 to data/model changes; 8, 9 to Gradle changes. Judge by what the diff touches — don't flag rules a file doesn't engage.
+
+For a **large diff (~15+ first-party files)**, fan out: spawn a subagent per dimension (UI/state, DI, persistence, Gradle/modules), each instructed to read the relevant ADR and report only its dimension's findings; then merge. For a small diff, do it inline.
+
+## Step 4 — Report
+
+Output a findings table, most-severe first. Every finding must cite the rule source and give a concrete fix. If a rule was checked and passes, say so (the evidence ledger wants pass/fail, not silence).
+
+```
+### Architecture review — <scope>
+Docs read: ARCHITECTURE.md, docs/adr/0001,0003,0005,0006,0009 (+others as needed)
+
+| Verdict | File:line | Rule | Source | Fix |
+|---|---|---|---|---|
+| ❌ | ui/FooScreen.kt:42 | Mutually-exclusive state as booleans | ADR 0001 sibling / State Mgmt | Model as a sealed FooUiState (Loading/Content/Error) |
+| ⚠️ | data/BarStore.kt:10 | Raw SQLite without stated justification | ADR 0001 | Use Room, or state which exception applies in the PR |
+| ✅ | — | Koin DI | ADR 0006 | new VM registered in module, constructor-injected |
+
+Summary: <n> violations, <n> warnings, <n> checks passed.
+```
+
+- **❌ violation** = contradicts a rule; blocking. **⚠️ warning** = likely issue / missing justification. **✅** = checked and clean.
+- If nothing architectural changed (e.g. a docs- or test-only diff), say so explicitly rather than inventing findings.
+
+## Notes
+
+- This is a *conformance* check against our documented patterns — not a general bug hunt (use `/code-review` for correctness). Keep findings tied to a specific ADR/section.
+- If a rule seems wrong or outdated for the change at hand, flag it as a possible ADR update rather than forcing the code to fit — the ADRs are `Proposed`, not immutable.

.githooks/pre-push/0002-architecture-review-nudge

@@ -0,0 +1,51 @@
+#!/bin/bash
+
+# Non-blocking nudge: when a push includes changes to first-party app source,
+# remind the author to run an architecture pass (the architecture-review skill /
+# REVIEW.md §10) before opening the PR.
+#
+# This is a REMINDER, not a gate — it always exits 0 and never blocks a push.
+# It stays silent unless the push actually touches first-party Kotlin/Java, so
+# docs-, test-, and vendored-only pushes produce no output.
+
+set -u
+
+cyan=$(tput setaf 6 2>/dev/null || true)
+yellow=$(tput setaf 3 2>/dev/null || true)
+reset=$(tput sgr0 2>/dev/null || true)
+
+# Determine the commits being pushed. Prefer the tracked upstream; fall back to
+# the integration branch (feature branches are based on stage). If neither is
+# resolvable, stay quiet rather than nag.
+if git rev-parse --abbrev-ref --symbolic-full-name '@{upstream}' >/dev/null 2>&1; then
+	range="@{upstream}..HEAD"
+elif git rev-parse --verify -q origin/stage >/dev/null 2>&1; then
+	range="origin/stage..HEAD"
+else
+	exit 0
+fi
+
+changed=$(git diff --name-only "$range" 2>/dev/null) || exit 0
+[ -n "$changed" ] || exit 0
+
+# Keep only first-party production Kotlin/Java: drop tests, generated build
+# output, and vendored subtrees (mirrors Spotless's commonTargetExcludes).
+firstparty=$(printf '%s\n' "$changed" \
+	| grep -E '\.(kt|java)$' \
+	| grep -vE '(^|/)(build|src/test|src/androidTest)/' \
+	| grep -vE '^(composite-builds/build-deps|termux/|eventbus/|LayoutEditor/|subprojects/(aaptcompiler|builder-model-impl|flashbar|xml-dom|llama\.cpp)/)' \
+	|| true)
+
+[ -n "$firstparty" ] || exit 0
+
+count=$(printf '%s\n' "$firstparty" | grep -c .)
+
+echo ""
+echo "${cyan}[architecture nudge]${reset} this push changes ${count} first-party source file(s)."
+echo "${yellow}  Consider an architecture pass before opening the PR:${reset}"
+echo "    - run the ${cyan}architecture-review${reset} skill (reads ARCHITECTURE.md + the ADRs, checks the diff), or"
+echo "    - self-check against ${cyan}REVIEW.md section 10${reset} (UDF, sealed state, Koin, Room, module boundaries, Compose)."
+echo "${yellow}  (Reminder only — your push continues.)${reset}"
+echo ""
+
+exit 0

.github/workflows/analyze.yml

@@ -79,7 +79,7 @@ jobs:
           # Firebase config).  Scoped to this step so SonarCloud and other
           # third-party actions never see them.
           FIREBASE_CONSOLE_URL: ${{ secrets.FIREBASE_CONSOLE_URL }}
-          SENTRY_DSN_DEBUG: ${{ secrets.SENTRY_DSN_DEBUG }}
+          GLITCHTIP_DSN: ${{ secrets.GLITCHTIP_DSN }}
         run: |
           echo "gradle_time_start=$(date +%s)" >> $GITHUB_ENV
           flox activate -d flox/base -- ./gradlew :app:assembleV8Debug --no-daemon
@@ -104,7 +104,7 @@ jobs:
           # The Gradle build also drives Sentry/Firebase configuration during
           # the unit-test compile path.
           FIREBASE_CONSOLE_URL: ${{ secrets.FIREBASE_CONSOLE_URL }}
-          SENTRY_DSN_DEBUG: ${{ secrets.SENTRY_DSN_DEBUG }}
+          GLITCHTIP_DSN: ${{ secrets.GLITCHTIP_DSN }}
         run: flox activate -d flox/base -- ./gradlew :testing:tooling:assemble :testing:common:assemble sonarqube --info --no-build-cache -x lint --continue
 
       - name: Upload JaCoCo report

.github/workflows/debug.yml

@@ -28,7 +28,7 @@ env:
   ORG_GRADLE_PROJECT_signingInMemoryKeyId: ${{ secrets.MVN_SIGNING_KEY_ID }}
   ORG_GRADLE_PROJECT_signingInMemoryKeyPassword: ${{ secrets.MVN_SIGNING_KEY_PASSWORD }}
   FIREBASE_CONSOLE_URL: ${{ secrets.FIREBASE_CONSOLE_URL }}
-  SENTRY_DSN_DEBUG: ${{ secrets.SENTRY_DSN_DEBUG }}
+  GLITCHTIP_DSN: ${{ secrets.GLITCHTIP_DSN }}
 
 jobs:
   check_changes:

.github/workflows/release.yml

@@ -25,10 +25,7 @@ env:
   ORG_GRADLE_PROJECT_signingInMemoryKeyId: ${{ secrets.MVN_SIGNING_KEY_ID }}
   ORG_GRADLE_PROJECT_signingInMemoryKeyPassword: ${{ secrets.MVN_SIGNING_KEY_PASSWORD }}
   FIREBASE_CONSOLE_URL: ${{ secrets.FIREBASE_CONSOLE_URL }}
-  SENTRY_DSN_RELEASE: ${{ secrets.SENTRY_DSN_RELEASE }}
-  SENTRY_ORG: ${{ secrets.SENTRY_ORG }}
-  SENTRY_PROJECT: ${{ secrets.SENTRY_PROJECT }}
-  SENTRY_AUTH_TOKEN: ${{ secrets.SENTRY_AUTH_TOKEN }}
+  GLITCHTIP_DSN: ${{ secrets.GLITCHTIP_DSN }}
 
 jobs:
   merge_stage_to_main:

AGENTS.md

@@ -0,0 +1,5 @@
+# AGENTS.md
+
+The operational rules for AI agents and contributors live in **[CLAUDE.md](CLAUDE.md)** — build/test commands, ABI flavors, emulator/device selection, project constraints, and the CI/Jira/SonarQube/git-messaging conventions.
+
+This file is a pointer so agents that follow the `AGENTS.md` convention find the guidance; the content is maintained in one place (CLAUDE.md) to avoid drift.