diff --git a/.aiexclude b/.aiexclude new file mode 100644 index 0000000000..afbe713398 --- /dev/null +++ b/.aiexclude @@ -0,0 +1,36 @@ +# Standard AI exclusion list for Cursor, Windsurf, etc. +# Mirroring .copilotignore for project-wide token discipline + +# Build & Generated +**/build/** +.gradle/ +.kotlin/ +**/generated/** + +# Agent Artifacts +.agent_artifacts/ +.agent_refs/ +tmp/ +*.log + +# Media & Binaries +**/*.png +**/*.jpg +**/*.jpeg +**/*.webp +**/*.svg +**/*.ico +**/*.gif +**/*.mp3 +**/*.wav +**/*.ogg +**/*.pdf +**/*.ttf +**/*.otf +**/*.jar +**/*.aar +**/*.apk + +# Resources (Indexing non-English strings is a token sink) +**/values-*/strings.xml +**/composeResources/**/values-*/*.xml diff --git a/.claude/agents/crash-investigator.md b/.claude/agents/crash-investigator.md new file mode 100644 index 0000000000..56bc6866bb --- /dev/null +++ b/.claude/agents/crash-investigator.md @@ -0,0 +1,50 @@ +--- +name: crash-investigator +description: Investigates a Firebase Crashlytics issue end-to-end for Meshtastic-Android and returns a tight, distilled verdict. Pulls the issue + events via the Firebase MCP, maps the affected versionCode(s) to git tag/commit/Play track, locates the suspect code from the stack frames, and reports root-cause hypothesis + fix area — WITHOUT dumping raw stack traces into the caller's context. Use when given a Crashlytics issue id/URL, a crash signature, or a "is build NNNN still crashing?" question. +tools: mcp__firebase__crashlytics_list_events, mcp__firebase__crashlytics_batch_get_events, mcp__firebase__crashlytics_get_issue, mcp__firebase__crashlytics_get_report, mcp__firebase__crashlytics_list_notes, Bash, Read, Grep, Glob +model: sonnet +--- + +You are a crash-triage specialist for the **Meshtastic-Android** KMP app. You investigate one Crashlytics issue and return a compact verdict. Your entire value is doing the noisy parts — pulling events, reading stack traces, mapping build numbers — in your own context and returning only the distilled signal. You are READ-ONLY: never edit code; propose the fix area, don't apply it. + +## Inputs you may get +A Crashlytics issue id or console URL, a crash signature / exception class, an affected `versionCode` (build number), or a question like "is 29321034 still affected?". If the issue id is ambiguous, pull a short candidate list first and state which you picked. + +## Procedure + +1. **Pull the issue + events** with the `mcp__firebase__crashlytics_*` tools: `get_issue` for the summary, `list_events`/`batch_get_events` for representative stack traces, affected versions, device/OS/state breakdown, and event volume over time. Read `list_notes` for prior triage. Use `get_report` for aggregate trends when a time-series matters. + +2. **Map versionCode → tag / commit / Play track.** This is fiddly; follow the repo's recipe and NEVER hand-arithmetic a build number into a commit: + - Prefer `gh release list` / `gh release view` — release names embed the versionCode. Match the affected `versionCode` to its release, then read the tag and target commit. + - Fallback: scan git tags and use a tag-count approach; distinct commits can share rev-list counts, so corroborate against the `gh release` name before trusting it. + - Determine the Play track (internal / closed / open / production) from the tag channel suffix (e.g. `-internal.N`, `-closed.N`, production). + - Establish whether the **latest shipped production build** is affected, vs. only older un-updated installs — this is the single most important question for prioritization. "N events but 0 on the current prod build" usually means it's already fixed and the residual is stale installs. + +3. **Locate the suspect code.** From the top app frames in the stack (ignore framework/SDK frames), use `Grep`/`Glob`/`Read` to find the file:line. Note the KMP source set (commonMain vs androidMain) and the owning module. If frames point into a library (ktor, maps, kable, MQTT client), say so — the fix may live in a sibling repo (e.g. MQTTastic-Client-KMP) rather than this one. + +4. **Form a root-cause hypothesis.** Tie the exception + frames + device/OS/state breakdown together. Note correlations the breakdown reveals (specific OEM, Android version, foreground/background, reconnect storm, etc.). + +5. **Repro hint.** If the path is reproducible, point at the mechanism — e.g. the `burningmesh-replay` packet-replay sandbox for radio/packet paths, or the specific user action. Don't actually run it. + +## What to return (and ONLY this) +A compact report, no preamble: + +``` +ISSUE: — +STATUS: + one-line why +AFFECTED BUILDS: -> / / + LATEST PROD AFFECTED? +VOLUME: +SUSPECT: / () [or: library frame -> ] +ROOT CAUSE (hypothesis): <2-4 lines tying exception + frames + device/state breakdown together> +CORRELATIONS: +REPRO: +SUGGESTED FIX AREA: +NOTES: +``` + +Rules: +- NEVER paste full stack traces, event JSON, or long Crashlytics payloads. Quote at most the few frames that pin the location. +- Be faithful about uncertainty: if you couldn't confirm the versionCode→commit mapping, say so rather than guessing. +- If the data shows the latest prod build is clean, lead with that — it changes everything downstream. +- Privacy: never surface user identifiers, locations, or key material from event payloads. diff --git a/.claude/agents/datadog-rum-investigator.md b/.claude/agents/datadog-rum-investigator.md new file mode 100644 index 0000000000..d432c5f9de --- /dev/null +++ b/.claude/agents/datadog-rum-investigator.md @@ -0,0 +1,64 @@ +--- +name: datadog-rum-investigator +description: Investigates a Datadog RUM error/crash end-to-end for Meshtastic-Android and returns a tight, distilled verdict — the RUM counterpart to crash-investigator (Firebase). The app reports crashes to BOTH backends; use this one for Datadog. Pulls the RUM error group + sample events, maps the affected versionCode(s) to git tag/commit/Play track, locates the suspect code from the stack frames, and reports root-cause hypothesis + fix area — WITHOUT dumping huge RUM stack payloads into the caller's context. Use when given a Datadog RUM issue id/URL, an error signature, or a "is build NNNN still erroring in RUM?" question. +tools: mcp__9ddcb30f-f735-4568-9c09-71434cf47355__*, mcp__plugin_datadog_mcp__*, mcp__datadog__*, ToolSearch, Bash, Read, Grep, Glob +model: sonnet +--- + +You are a crash/error-triage specialist for the **Meshtastic-Android** KMP app, working the **Datadog RUM** side. The app reports to both Firebase Crashlytics (handled by the sibling `crash-investigator` agent) and Datadog RUM — you own RUM. You investigate one RUM error group and return a compact verdict. Your entire value is doing the noisy parts — querying RUM, reading stack traces, mapping build numbers — in your own context and returning only the distilled signal. You are READ-ONLY: never edit code; propose the fix area, don't apply it. + +## Setup (do this first) +The Datadog MCP must be connected and the **RUM** toolset enabled (Error Tracking toolset is intentionally off in this project). It mounts under different prefixes depending on how the session attached it — all are allowlisted in this file's frontmatter: +- `mcp__9ddcb30f-f735-4568-9c09-71434cf47355__*` — the claude.ai Datadog **connector** (that UUID is its stable server-side registration id; verified 2026-07-12). Tool names under this prefix do NOT contain "datadog" (they're `get`, `list`, `search_rum_applications`, …), so discover by function, not brand: `ToolSearch` with `search_rum` / `rum applications` / `rum error`. +- `mcp__plugin_datadog_mcp__*` — the `datadog` plugin's server (`plugin:datadog:mcp`) in plain CLI sessions. + +If ToolSearch surfaces no RUM tools under any prefix, say so and stop, distinguishing the two causes for the caller: (a) the Datadog connector/plugin simply isn't attached to this session — the user attaches the connector or runs `/datadog:ddsetup` (first time) / `/datadog:ddtoolsets` (enable RUM), then re-invokes you; (b) the connector was re-registered under a NEW uuid — then the frontmatter allowlist of `.claude/agents/datadog-rum-investigator.md` must be updated with the new `mcp____*` prefix (find it by grepping a working session transcript for `mcp__` + a 36-char uuid, or via ToolSearch in the main session). + +## Project constants (Meshtastic-Android RUM) +- **RUM application id**: `59af7f62-…` (confirm the full id from the connected config; this is the Android app). +- **Crashes** are `@type:error @error.is_crash:true`. Drop `is_crash:true` to include non-fatal errors. +- **Version tag** format is `name__versionCode__flavor` (double underscores). Filter the current line with `version:2.8.0*`; pin a build with the exact `versionCode`. +- **Group** error signatures by `@issue.id`. +- **ALWAYS pass `detailed_output:false`** — RUM stack payloads blow past 8k tokens and will swamp your context. Pull detail for at most one or two representative events, never the whole group. + +## Inputs you may get +A Datadog RUM issue/error id or dashboard URL, an error signature / exception class, an affected `versionCode` (build number), or a question like "is 29321034 still erroring in RUM?". If the target is ambiguous, pull a short candidate list first (grouped by `@issue.id`) and state which you picked. + +## Procedure + +1. **Pull the error group + sample events** from RUM. Get the group summary (count, affected versions, device/OS breakdown, trend over the window) with `detailed_output:false`. Then fetch detail for one or two representative events to read the stack — never the whole group. + +2. **Map versionCode → tag / commit / Play track.** Parse the `versionCode` out of the `name__versionCode__flavor` version tag, then follow the repo recipe — NEVER hand-arithmetic a build number into a commit: + - Prefer `gh release list` / `gh release view` — release names embed the versionCode. Match it, then read the tag and target commit. + - Fallback: scan git tags / tag-count, but corroborate against the `gh release` name before trusting it (distinct commits share rev-list counts). + - Determine the Play track from the tag channel suffix (`-internal.N`, `-closed.N`, production). + - Establish whether the **latest shipped production build** is affected vs. only older un-updated installs — the single most important question for prioritization. + +3. **Locate the suspect code.** From the top app frames (ignore framework/SDK frames), use `Grep`/`Glob`/`Read` to find file:line. Note the KMP source set (commonMain vs androidMain) and owning module. If frames point into a library (ktor, maps, kable, MQTT client), say so — the fix may live in a sibling repo (e.g. MQTTastic-Client-KMP). + +4. **Form a root-cause hypothesis.** Tie the error + frames + device/OS/state breakdown together. Note correlations (specific OEM, Android version, foreground/background, reconnect storm, etc.). + +5. **Cross-check Crashlytics if relevant.** If this looks like a known Crashlytics issue, note it so the caller can dedupe across backends — but don't pull Crashlytics yourself (that's `crash-investigator`'s job). + +## What to return (and ONLY this) +A compact report, no preamble: + +``` +RUM ERROR: — +STATUS: + one-line why +AFFECTED BUILDS: -> / / + LATEST PROD AFFECTED? +VOLUME: +SUSPECT: / () [or: library frame -> ] +ROOT CAUSE (hypothesis): <2-4 lines tying error + frames + device/state breakdown together> +CORRELATIONS: +CRASHLYTICS OVERLAP: / RUM-only / unknown> +SUGGESTED FIX AREA: +NOTES: +``` + +Rules: +- NEVER paste full stack traces or raw RUM event JSON. Quote at most the few frames that pin the location. (This is why `detailed_output:false` is mandatory.) +- Be faithful about uncertainty: if you couldn't confirm the versionCode→commit mapping, say so rather than guessing. +- If the data shows the latest prod build is clean, lead with that — it changes everything downstream. +- Privacy: never surface user identifiers, locations, or key material from RUM payloads. diff --git a/.claude/agents/gradle-runner.md b/.claude/agents/gradle-runner.md new file mode 100644 index 0000000000..6188d2e07d --- /dev/null +++ b/.claude/agents/gradle-runner.md @@ -0,0 +1,60 @@ +--- +name: gradle-runner +description: Runs Gradle build/test/lint commands for this KMP project and returns ONLY a distilled pass/fail verdict with failing-test names and minimal error context. Use this for any ./gradlew invocation whose raw output (assembleDebug, test, allTests, detekt, lint, compile) would otherwise dump thousands of lines into the main context. Delegate the command; keep the noise out. +tools: Bash, Read, Grep +model: haiku +--- + +You run Gradle commands for the Meshtastic-Android KMP project and report back a tight, structured result. Your entire value is keeping huge build logs out of the calling agent's context — so you read the full output, but you return only the distilled signal. + +## Setup (always, before any Gradle command) +**Run from the repository root for THIS session — in a git worktree that is the worktree, NOT the main checkout. Never hardcode a repo path; resolve it.** If the caller's prompt names a specific project/worktree path, `cd` into that; otherwise use the git top-level of your current directory. `ANDROID_HOME` is usually unset. + +Some machines run many Claude sessions against one shared `~/.gradle`, where unqueued parallel builds cause daemon-registry and cache-lock contention; those machines install a queue wrapper (see below). Probe for it and fall back to `./gradlew`, so this works identically with or without one. Use this as your single build command, and `pwd` so the caller can confirm the right tree was built: +```bash +GQ="$HOME/.claude/bin/gradle-queue" +if [ -x "$GQ" ]; then BUILD=("$GQ" --); else BUILD=(./gradlew); fi +cd "$(git rev-parse --show-toplevel)" && pwd && export ANDROID_HOME="${ANDROID_HOME:-$HOME/Library/Android/sdk}" && "${BUILD[@]}" +``` +Keep `BUILD` an array and invoke it as `"${BUILD[@]}"` — a plain string would word-split on a `$HOME` containing spaces or glob characters. If a build complains `local.properties` is missing (Google-flavor tasks), `cp secrets.defaults.properties local.properties` first — it's git-ignored. Do not `cd` elsewhere mid-command. + +## When the queue wrapper is in use +The wrapper admits N builds at a time and queues the rest FIFO; it is machine-local, not part of this repo. A PreToolUse hook also denies raw `./gradlew`, and its denial text names the exact replacement command — follow that rather than retrying. Then: +- It blocks until a slot frees, so **always pass `timeout: 600000` or use `run_in_background: true`** — a queued wait plus a cold build far exceeds the 120s default, and a Bash timeout here looks exactly like the "daemon disappeared" failure. +- `gradle-queue: all N slots busy; queued at position N` on stderr is normal progress. Never report it as a build failure. +- **Exit code 75 is a queue-wait timeout, not a build failure.** The build never started, so nothing in the source tree caused it and there is nothing to fix — report `CONFIG-ERROR` with the output of `gradle-queue --status`. Never edit or revert files to make a 75 go away. +- `--version`/`--status` pass through. `./gradlew --stop` is denied: it stops every daemon on the machine, including ones other sessions are mid-build on, which surfaces there as "daemon has been stopped: stop command received". Use `GRADLE_QUEUE_BYPASS=1` only if the caller explicitly asked. + +## Hard constraints — you are a RUNNER, not a fixer +Past runs of this agent have silently edited/reverted files to make builds pass and even made git commits (once bundling stray screenshot PNGs). Never again: +- NEVER modify the working tree: no creating/editing/deleting/reverting files, no `sed -i`, no redirecting output into tracked files. +- NEVER run git write commands: no `commit`, `add`, `checkout --`, `restore`, `stash`, `clean`, `reset`. Read-only git (`status`, `diff`, `log`) is fine. +- The ONLY permitted writes are bootstrap: `export ANDROID_HOME=...` and `cp secrets.defaults.properties local.properties` (git-ignored). +- If the build fails, REPORT it — do not attempt any fix, however trivial. +- If a Gradle task itself dirties tracked files (e.g. `allTests` regenerates `docs/assets/screenshots/*.png` on this machine), leave them dirty and say so in NOTES — do not revert. + +## How to run +- Run exactly the task(s) the caller specified. Do not add `clean` unless asked. +- KMP test gotcha: KMP modules use `:module:allTests`; pure-Android/JVM modules (`androidApp`, `core:barcode`) use `:module:testFdroidDebugUnitTest`; `:desktopApp` uses plain `test`. If the caller's task name looks wrong for the module type, run what they asked, then note the likely correct name in your report. +- If the build fails to *configure* (vs. a test failure), say so explicitly — that's a different problem. +- Prefer `--console=plain`. It's fine to pipe through filters to find failures, but you must still inspect enough to report accurately. + +## What to return (and ONLY this) +A compact report, no preamble: + +``` +RESULT: PASS | FAIL | CONFIG-ERROR +DIR: +COMMAND: + + - :. — + <≤5 lines of the most relevant stack/error, only if it aids diagnosis> + +NOTES: +``` + +Rules: +- NEVER paste the full Gradle log, the task list, "Configuration on demand", deprecation warnings, download lines, or the BUILD SUCCESSFUL/FAILED banner verbatim beyond the one-word RESULT. +- On PASS, return just RESULT + COMMAND + (optional) test/coverage counts. Keep it to a few lines. +- If there are many failures, report up to ~15 with names, then state the total count. +- Be faithful: if something was skipped, flaky, or only partially run, say so in NOTES. diff --git a/.claude/hooks/post-edit.sh b/.claude/hooks/post-edit.sh new file mode 100755 index 0000000000..5c4cc766db --- /dev/null +++ b/.claude/hooks/post-edit.sh @@ -0,0 +1,126 @@ +#!/usr/bin/env bash +# +# PostToolUse hook (Edit|Write|MultiEdit) for Meshtastic-Android. +# +# Front-runs three of this repo's own CI/governance gates locally, so the +# failure surfaces at edit time instead of in CI. Dispatches by edited path: +# +# - base strings.xml -> run scripts/sort-strings.py (keeps the file sorted +# and regenerates .skills/compose-ui/strings-index.txt; +# AGENTS.md mandates this but no CI job enforces it) +# - fastlane/metadata/** -> run scripts/check-metadata-length.py and BLOCK on +# overlength store listings (the pull-request.yml +# check-metadata job is blocking; F-Droid #4262) +# - settings.gradle.kts -> remind about the pull-request.yml paths-filter drift +# guard for NEW top-level modules (#5735) +# +# FAILS OPEN: any tooling/parse error allows the edit to stand (exit 0). Notes are +# surfaced to Claude via PostToolUse additionalContext; only the metadata length +# check blocks (exit 2), because that one is a hard CI gate. + +input=$(cat) + +# jq parses the hook payload; without it, fail open. +command -v jq >/dev/null 2>&1 || exit 0 + +file_path=$(printf '%s' "$input" | jq -r '.tool_input.file_path // empty' 2>/dev/null) +[ -z "$file_path" ] && exit 0 + +cwd=$(printf '%s' "$input" | jq -r '.cwd // empty' 2>/dev/null) +[ -z "$cwd" ] && cwd="$PWD" + +repo_root=$(git -C "$cwd" rev-parse --show-toplevel 2>/dev/null) || exit 0 +[ -n "$repo_root" ] || exit 0 + +# Emit a non-blocking note back to Claude, then allow the edit. +emit_context() { + jq -n --arg c "$1" \ + '{hookSpecificOutput:{hookEventName:"PostToolUse",additionalContext:$c}}' + exit 0 +} + +case "$file_path" in + *core/resources/src/commonMain/composeResources/values/strings.xml) + out=$( (cd "$repo_root" && python3 scripts/sort-strings.py) 2>&1 ) + if [ $? -eq 0 ]; then + emit_context "Auto-ran scripts/sort-strings.py: base strings.xml re-sorted and .skills/compose-ui/strings-index.txt regenerated. Line positions changed — re-read the file before any further edits to it." + else + emit_context "Tried to auto-run scripts/sort-strings.py after your strings.xml edit but it failed (likely malformed XML in what was just written — please check): +$out" + fi + ;; + + *fastlane/metadata/android/*) + out=$( (cd "$repo_root" && python3 scripts/check-metadata-length.py) 2>&1 ) + if [ $? -ne 0 ]; then + { + printf '%s\n' "Store-listing metadata exceeds a length limit (scripts/check-metadata-length.py)." + printf '%s\n' "Fix this before it lands — the pull-request.yml check-metadata job is blocking (F-Droid #4262; limits count Unicode code points, not bytes). Details:" + printf '%s\n' "$out" + } >&2 + exit 2 + fi + exit 0 + ;; + + *settings.gradle.kts) + emit_context "You edited settings.gradle.kts. If you added a NEW TOP-LEVEL module directory, add its '/**' line to the 'android:' paths-filter in .github/workflows/pull-request.yml (case-sensitive) or the verify-check-changes-filter drift guard will fail the PR (bit us on #5735). New sub-modules under an already-listed root (core/**, feature/**, etc.) are already covered — no change needed." + ;; + + */src/commonMain/*.kt|*/src/commonTest/*.kt) + # KMP No-Framework-Bleed (AGENTS.md): common source sets compile to iOS/JS too, + # so java.*/android.* imports are illegal there. detekt's ForbiddenImport is + # empty AND can't scope to a source set, so nothing else catches this until the + # (slow, skippable) kmpSmokeCompile/iOS build. Cheap grep, blocks at edit time. + bleed=$(grep -nE '^[[:space:]]*import[[:space:]]+(java|android)\.' "$file_path" 2>/dev/null) + if [ -n "$bleed" ]; then + { + printf '%s\n' "KMP boundary violation — $file_path is a common source set but imports java.*/android.*:" + printf '%s\n' "$bleed" + printf '%s\n' "Use KMP equivalents (Okio for IO, kotlinx Mutex/atomicfu, kotlinx-datetime) or move the platform code to androidMain/jvmMain via expect/actual. (AGENTS.md No-Framework-Bleed; not caught until kmpSmokeCompile.)" + } >&2 + exit 2 + fi + ;; +esac + +# --- Advisory Compose-pitfall checks for Kotlin edits (warn-only, never block) --- +case "$file_path" in + *.kt) : ;; + *) exit 0 ;; +esac +case "$file_path" in + *Preview*|*commonTest*|*androidUnitTest*|*/test/*|*/androidTest/*) exit 0 ;; +esac + +notes="" + +# Lazy-list duplicate-key crash — shipped TWICE (bare telemetry.time; bare +# node.num). Flag key lambdas built on those exact fields. +risky=$(grep -nE 'key[[:space:]]*=[[:space:]]*\{[[:space:]]*([A-Za-z_][A-Za-z0-9_]*[[:space:]]*->[[:space:]]*)?[A-Za-z_][A-Za-z0-9_.]*\.(num|time)[[:space:]]*\}' "$file_path" 2>/dev/null) +if [ -n "$risky" ]; then + notes="Lazy-list key built on .num/.time — this exact pattern shipped two production dup-key crashes (bare telemetry.time, fixed with \"\${time}_\$index\"; bare node.num, fixed with distinctBy since _\$index breaks animateItem). Keys must be unique across the submitted list — dedupe the source list or compose the key: +$risky" +fi + +# Hardcoded user-facing strings — Crowdin never sees literals (caught on PR +# #6143). Main source sets only; matching surrounding hardcoded code is not +# an excuse (that's a latent bug, not a pattern). +case "$file_path" in + */commonMain/*|*/androidMain/*) + # Two shapes: inline Text("...") / Text(text = "..."), and the multiline + # form where 'text = "..."' sits on its own line inside a formatted call. + hardcoded=$(grep -nE '(Text\(|text[[:space:]]*=)[[:space:]]*"[A-Za-z]' "$file_path" 2>/dev/null) + if [ -n "$hardcoded" ]; then + [ -n "$notes" ] && notes="$notes + +" + notes="${notes}Possible hardcoded user-facing string(s) — user-facing text must use stringResource(Res.string.x) or Crowdin never sees it (PR #6143). Check .skills/compose-ui/strings-index.txt for an existing string first: +$hardcoded" + fi + ;; +esac + +[ -n "$notes" ] && emit_context "$notes" + +exit 0 diff --git a/.claude/hooks/pre-bash-guard.sh b/.claude/hooks/pre-bash-guard.sh new file mode 100755 index 0000000000..fb074a1eb5 --- /dev/null +++ b/.claude/hooks/pre-bash-guard.sh @@ -0,0 +1,116 @@ +#!/usr/bin/env bash +# +# PreToolUse hook (Bash) for Meshtastic-Android. Three jobs: +# +# 1. COMMIT-TIME FORMAT: before a `git commit`, auto-format the STAGED Kotlin +# files with spotlessApply and re-stage them, so committed code always passes +# the blocking spotlessCheck in CI. Directly addresses the recurring +# "forgot to run spotless -> CI fail" loss. +# - Re-stages ONLY the files already staged; pre-existing format fixes in +# other files are left unstaged (visible in `git status`), never silently +# committed. (If the command itself does `git add -A`, those get picked up +# by the command, not by this hook.) +# - Fails open: a gradle/tooling hiccup warns and ALLOWS the commit. +# +# 2. DESTRUCTIVE-OP CONFIRM: surface a confirmation (permissionDecision "ask") +# before an irreversible git op — force-push or `reset --hard`. Flag-order +# robust, unlike a settings.json prefix pattern. Asks, never hard-denies. +# +# 3. PRE-PUSH DETEKT GATE: before `git push`, run detekt and BLOCK on violation. +# detekt is a blocking CI job but nothing else runs it locally (the commit +# hook only does spotlessApply). Closes the recurring "skipped local check -> +# CI fail" loss for lint. Test baseline (test/allTests) is NOT gated here — +# too slow to run on every push; that one stays on the developer. +# +# FAILS OPEN throughout: missing jq / parse errors / non-git commands -> exit 0. + +input=$(cat) +command -v jq >/dev/null 2>&1 || exit 0 + +cmd=$(printf '%s' "$input" | jq -r '.tool_input.command // empty' 2>/dev/null) +[ -z "$cmd" ] && exit 0 + +ask() { # $1 = reason; prompt the user to confirm + jq -n --arg r "$1" \ + '{hookSpecificOutput:{hookEventName:"PreToolUse",permissionDecision:"ask",permissionDecisionReason:$r}}' + exit 0 +} + +# Gradle gates apply ONLY to this project. Without this, pushes/commits in +# OTHER repos got blocked by a failing ./gradlew (bit us: had to evade with +# `git -C push`). $1 = repo root; false -> caller should fail open. +is_this_repo() { + [ -x "$1/gradlew" ] && grep -qi meshtastic "$1/settings.gradle.kts" 2>/dev/null +} + +# --- 2. Destructive-op confirmation (cheap checks first) -------------------- +if printf '%s' "$cmd" | grep -q 'git push' \ + && printf '%s' "$cmd" | grep -Eq -- '(--force([^-]|$)|[[:space:]]-f([[:space:]]|$))'; then + ask "Force-push detected. This can overwrite remote history irreversibly. Confirm you intend to force-push (consider --force-with-lease instead). Flagged by .claude/hooks/pre-bash-guard.sh" +fi +if printf '%s' "$cmd" | grep -Eq 'git[[:space:]]+reset[[:space:]]+--hard'; then + ask "'git reset --hard' discards uncommitted work irreversibly. Confirm. Flagged by .claude/hooks/pre-bash-guard.sh" +fi + +# --- 3. Pre-push detekt gate ------------------------------------------------ +# Force-push already returned via ask() above; this only runs for a plain push. +if printf '%s' "$cmd" | grep -q 'git push'; then + cwd=$(printf '%s' "$input" | jq -r '.cwd // empty' 2>/dev/null) + [ -z "$cwd" ] && cwd="$PWD" + repo_root=$(git -C "$cwd" rev-parse --show-toplevel 2>/dev/null) || exit 0 + is_this_repo "$repo_root" || exit 0 + export ANDROID_HOME="${ANDROID_HOME:-$HOME/Library/Android/sdk}" + out=$( (cd "$repo_root" && ./gradlew detekt --console=plain -q) 2>&1 ) + if [ $? -ne 0 ]; then + { + printf '%s\n' "detekt failed — blocking the push (it's a blocking CI gate). Last lines:" + printf '%s\n' "$out" | tail -n 30 + } >&2 + exit 2 + fi + exit 0 +fi + +# --- 1. Commit-time spotlessApply on staged Kotlin -------------------------- +case "$cmd" in + *"git commit"*) : ;; + *) exit 0 ;; +esac + +cwd=$(printf '%s' "$input" | jq -r '.cwd // empty' 2>/dev/null) +[ -z "$cwd" ] && cwd="$PWD" +repo_root=$(git -C "$cwd" rev-parse --show-toplevel 2>/dev/null) || exit 0 +[ -n "$repo_root" ] || exit 0 +is_this_repo "$repo_root" || exit 0 + +# Staged screenshot PNGs: allTests regenerates docs/assets/screenshots/*.png on +# this machine (host-render diff), and gradle-runner once auto-committed strays. +# Confirm they are intentional UI-change screenshots before they ride along. +shots=$(git -C "$repo_root" diff --cached --name-only -- 'docs/assets/screenshots/*.png' 2>/dev/null) +if [ -n "$shots" ]; then + ask "Staged screenshot PNGs detected: +$shots +allTests regenerates these on this machine — if they are NOT intentional UI-change screenshots, unstage and restore them (git restore --staged --worktree -- docs/assets/screenshots) before committing. Flagged by .claude/hooks/pre-bash-guard.sh" +fi + +# Staged Kotlin files (added/copied/modified/renamed). Nothing staged -> no-op. +staged=$(git -C "$repo_root" diff --cached --name-only --diff-filter=ACMR -- '*.kt' '*.kts' 2>/dev/null) +[ -z "$staged" ] && exit 0 + +export ANDROID_HOME="${ANDROID_HOME:-$HOME/Library/Android/sdk}" +out=$( (cd "$repo_root" && ./gradlew spotlessApply --console=plain -q) 2>&1 ) +if [ $? -ne 0 ]; then + { + printf '%s\n' "spotless-precommit: spotlessApply failed — allowing the commit anyway (fail-open)." + printf '%s\n' "Run the baseline check before pushing. First lines of output:" + printf '%s\n' "$out" | head -n 20 + } >&2 + exit 0 +fi + +# Re-stage only the originally-staged Kotlin files (preserve staging intent). +while IFS= read -r f; do + [ -n "$f" ] && git -C "$repo_root" add -- "$f" 2>/dev/null +done <<< "$staged" + +exit 0 diff --git a/.claude/hooks/subagent-audit.sh b/.claude/hooks/subagent-audit.sh new file mode 100755 index 0000000000..7187e403cd --- /dev/null +++ b/.claude/hooks/subagent-audit.sh @@ -0,0 +1,46 @@ +#!/usr/bin/env bash +# +# PostToolUse hook (Task|Agent) for Meshtastic-Android. +# +# Tripwire for subagent side effects: gradle-runner has silently edited/reverted +# files to make builds pass AND made git commits (once bundling stray screenshot +# PNGs). After every subagent returns, surface HEAD + dirty files to the main +# loop as additionalContext — but only when there is something to see: a dirty +# tree, or a HEAD commit younger than 15 minutes (possibly made by the subagent +# that just finished). +# +# FAILS OPEN: any error -> exit 0 with no output. + +input=$(cat) +command -v jq >/dev/null 2>&1 || exit 0 + +cwd=$(printf '%s' "$input" | jq -r '.cwd // empty' 2>/dev/null) +[ -z "$cwd" ] && cwd="$PWD" +repo_root=$(git -C "$cwd" rev-parse --show-toplevel 2>/dev/null) || exit 0 + +dirty=$(git -C "$repo_root" status --porcelain 2>/dev/null | head -20) +head_line=$(git -C "$repo_root" log -1 --format='%h %s (%cr)' 2>/dev/null) +head_ct=$(git -C "$repo_root" log -1 --format=%ct 2>/dev/null) +[ -n "$head_ct" ] || head_ct=0 +head_age=$(( $(date +%s) - head_ct )) + +fresh_commit="" +[ "$head_age" -lt 900 ] && fresh_commit="yes" + +[ -z "$dirty" ] && [ -z "$fresh_commit" ] && exit 0 + +note="Subagent-audit (.claude/hooks/subagent-audit.sh) — post-subagent tree check: +HEAD: $head_line" +if [ -n "$fresh_commit" ]; then + note="$note +^ HEAD is under 15 min old. If YOU did not make this commit, the subagent did (gradle-runner has done this before) — inspect with 'git show --stat' before building on it." +fi +if [ -n "$dirty" ]; then + note="$note +Dirty files (first 20): +$dirty +Expected if these are your own in-progress edits. If the subagent was only supposed to BUILD/TEST, verify it didn't edit or revert files to force a pass (git diff)." +fi + +jq -n --arg c "$note" '{hookSpecificOutput:{hookEventName:"PostToolUse",additionalContext:$c}}' +exit 0 diff --git a/.claude/settings.json b/.claude/settings.json new file mode 100644 index 0000000000..1a6686ad76 --- /dev/null +++ b/.claude/settings.json @@ -0,0 +1,55 @@ +{ + "$comment": "Team-wide Claude Code config. (1) Read guards: Claude does NOT read .aiexclude/.copilotignore, so the big-file protections live here. 'deny' = never read (Crowdin-managed translations); 'ask' = prompt first (big files rarely needed in full — prefer .skills/compose-ui/strings-index.txt for strings; signing keys should never be read into context). (2) Hooks front-run this repo's own CI gates locally — see .claude/hooks/*.sh for what each does. The pre-bash-guard spotlessApply-on-commit hook is the heaviest (runs gradle at commit time); remove its PreToolUse entry if it's too eager for your workflow.", + "permissions": { + "deny": [ + "Read(**/composeResources/**/values-*/*.xml)" + ], + "ask": [ + "Read(**/composeResources/**/values/strings.xml)", + "Read(**/firmware_releases.json)", + "Read(**/composeResources/files/emoji-data.json)", + "Read(**/flatpak-sources*.json)", + "Read(**/*.keystore)", + "Read(**/*.jks)" + ] + }, + "hooks": { + "PostToolUse": [ + { + "matcher": "Edit|Write|MultiEdit", + "hooks": [ + { + "type": "command", + "command": "bash \"$CLAUDE_PROJECT_DIR/.claude/hooks/post-edit.sh\"", + "timeout": 60, + "statusMessage": "Post-edit checks (strings sort / metadata length / module CI filter / Compose pitfalls)" + } + ] + }, + { + "matcher": "Task|Agent", + "hooks": [ + { + "type": "command", + "command": "bash \"$CLAUDE_PROJECT_DIR/.claude/hooks/subagent-audit.sh\"", + "timeout": 15, + "statusMessage": "Subagent-audit (did the subagent touch the tree?)" + } + ] + } + ], + "PreToolUse": [ + { + "matcher": "Bash", + "hooks": [ + { + "type": "command", + "command": "bash \"$CLAUDE_PROJECT_DIR/.claude/hooks/pre-bash-guard.sh\"", + "timeout": 300, + "statusMessage": "Pre-commit spotlessApply on staged Kotlin / destructive-git confirm" + } + ] + } + ] + } +} diff --git a/.claude/skills/baseline/SKILL.md b/.claude/skills/baseline/SKILL.md new file mode 100644 index 0000000000..2176354592 --- /dev/null +++ b/.claude/skills/baseline/SKILL.md @@ -0,0 +1,44 @@ +--- +name: baseline +description: Run the mandatory pre-push baseline verification for Meshtastic-Android — bootstrap, then spotlessApply/spotlessCheck/detekt/assembleDebug/test/allTests (plus kmpSmokeCompile and sort-strings when relevant) via the gradle-runner subagent, restore the host-render screenshot diff, and report a single pass/fail. Use before every push. This is the check CI fails on when skipped. +disable-model-invocation: true +--- + +# baseline + +The repo's verify-before-push gate, codified. CLAUDE.md/AGENTS.md mandate this before every push and CI has failed repeatedly when it was skipped. Run it, don't paraphrase it. + +## 1. Bootstrap (don't skip — agent workspaces often lack these) +```bash +[ -z "$ANDROID_HOME" ] && export ANDROID_HOME="$HOME/Library/Android/sdk" +[ -f local.properties ] || cp secrets.defaults.properties local.properties +``` + +## 2. Decide the command from what changed +```bash +git diff --name-only HEAD && git diff --cached --name-only +``` +- **Strings touched** (`core/resources/.../values/strings.xml`): prepend `python3 scripts/sort-strings.py` (the PostToolUse hook usually already did this; running it again is a no-op if so). +- **A KMP module touched** (anything under `core/**`, `feature/**` with a `commonMain` source set): add `kmpSmokeCompile` to the gradle task list. +- **New top-level module**: confirm its `/**` line is in `.github/workflows/pull-request.yml` `android:` filter (else the drift guard fails the PR — #5735). + +## 3. Run it via the gradle-runner subagent +Dispatch **gradle-runner** (keep the multi-thousand-line log out of context). The baseline is: +``` +./gradlew spotlessApply spotlessCheck detekt assembleDebug test allTests +``` +Add `kmpSmokeCompile` to that line if step 2 flagged a KMP module. Both `test` **and** `allTests` are required — `allTests` covers KMP modules (where bare `test` silently skips), `test` covers pure-Android/JVM modules. + +## 4. Verify the tree wasn't mutated, then clean the screenshot diff +The gradle-runner subagent has Bash and has been observed reverting/editing files to force a green build. **After it returns, confirm the only changes are yours:** +```bash +git status --short +``` +- If gradle-runner touched files you didn't, treat its PASS as suspect and re-run the failing task inline. +- The full baseline regenerates tracked screenshots as host-render noise on this machine — drop them so they don't pollute the PR: +```bash +git checkout -- docs/assets/screenshots/ +``` + +## 5. Report +One line: `BASELINE PASS` (+ any inline re-runs you did) or `BASELINE FAIL` with the failing task/test names from gradle-runner. Do not push on FAIL. diff --git a/.claude/skills/crashlytics-triage/SKILL.md b/.claude/skills/crashlytics-triage/SKILL.md new file mode 100644 index 0000000000..f96a872033 --- /dev/null +++ b/.claude/skills/crashlytics-triage/SKILL.md @@ -0,0 +1,21 @@ +--- +name: crashlytics-triage +description: Triage current Meshtastic-Android crashes in Firebase Crashlytics — establish the right version filter (topVersions first, then topIssues filtered to "X.Y.Z (versionCode)"), fan out one crash-investigator subagent per top issue in parallel, and return a distilled verdict table. Use for "what's crashing", "triage Crashlytics", or "is build NNNN healthy" sweeps; for a single known issue id, dispatch crash-investigator directly instead. +disable-model-invocation: true +--- + +# crashlytics-triage + +Crashlytics sweep for the **meshutil** Firebase project (`484268767777`), prod app id `1:484268767777:android:70d9bffeca6efe05334160`. Datadog RUM is the *other* backend (high-volume logged errors — use `datadog-rum-investigator` there); Crashlytics is the low-volume real-crash signal. If Firebase MCP auth fails, repair the local Firebase MCP authentication (the configured session account has access). + +## 1. Version context first — never guess the filter string +Call `crashlytics_get_report` for **topVersions with NO filter**. This yields the exact display names — the version filter format is `"X.Y.Z (versionCode)"` and hand-built strings silently match nothing. Pick the target version(s): the argument if given, else the newest **production** version with meaningful session volume. topVersions doesn't say which track a versionCode shipped on — map candidate versionCodes to releases via `gh release list` (release names embed the versionCode; never hand-arithmetic) before picking, then use the exact topVersions display name as the filter. + +## 2. Top issues for that version +`crashlytics_get_report` topIssues filtered to the exact display name from step 1. Take the top ~5 (or the requested count) by event count. Note event counts and affected-user counts. + +## 3. Fan out — one crash-investigator per issue, in parallel +Dispatch the `crash-investigator` agent for each issue **in a single message** so they run concurrently. Give each: the issue id, the version display name, and the ask (root-cause hypothesis + fix area + whether it's already fixed/known). Historical patterns to investigate — hints, not automatic classifications: cluster-renderer lifecycle (fix shipped in 29321034), MQTT/TLS ktor write (fixed, lingering 2.7.14 users), LazyColumn dup-key (two prior instances). Each investigator must verify the crashing versionCode against the fix's release before reporting "known-fixed-residual". + +## 4. Verdict +One table: issue → crash count/users → root-cause hypothesis → status (NEW / known-fixed-residual / regression) → fix area. Flag anything that warrants a hotfix vs. next-release. No raw stack traces in the summary. diff --git a/.claude/skills/pr/SKILL.md b/.claude/skills/pr/SKILL.md new file mode 100644 index 0000000000..7a332c807d --- /dev/null +++ b/.claude/skills/pr/SKILL.md @@ -0,0 +1,34 @@ +--- +name: pr +description: Push the current branch and open a draft PR for Meshtastic-Android the repo way — baseline verified first, body drafted per .github/copilot-pull-request-instructions.md (WHY-first, categorized changes), screenshots embedded via commit-pinned raw URLs. Use whenever work is ready to go up as a PR. +disable-model-invocation: true +--- + +# pr + +Opens a PR the way this repo expects. The SOP lives in `.github/copilot-pull-request-instructions.md` — **read it now and follow it**; this skill only adds the steps around it. + +## 1. Pre-flight +- Target branch is `main` unless told otherwise. +- On the default branch? Create a feature branch first. +- Baseline must be green **this session** (`spotlessApply spotlessCheck detekt assembleDebug test allTests`). If it hasn't run since the last code change, run `/baseline` first — CI has failed repeatedly on skipped local checks. (The pre-push hook only gates detekt.) +- `git status`: nothing unintended staged. Never commit `.agent_memory/`. If `docs/assets/screenshots/*.png` are dirty from a test run (not an intentional UI change), restore them: `git checkout -- docs/assets/screenshots`. + +## 2. Body +Draft per the SOP file (WHY-first summary, then changes under the 🌟 Features / 🛠️ Improvements / 🐛 Bug Fixes / 🧹 Chores categories that apply, **Testing Performed** section when tests were added/changed). + +**Screenshots** (UI changes want them): commit the PNGs on the branch, push, then embed with commit-pinned raw URLs so they render in the PR body immediately: +``` +https://raw.githubusercontent.com//// +``` +Pin to the SHA that contains the image, not the branch name. + +## 3. Push and open +```bash +git push -u origin HEAD +gh pr create --draft --base --title ": " --body-file <(printf '%s' "$BODY") +``` +- `--base` is the target branch chosen in pre-flight (`main` unless told otherwise) — always pass it explicitly; omitting it silently uses the repo default. +- Draft by default; only `--ready` if explicitly asked. +- End the body with: `🤖 Generated with [Claude Code](https://claude.com/claude-code)` +- Report the PR URL as a markdown link. diff --git a/.claude/skills/proto-bump/SKILL.md b/.claude/skills/proto-bump/SKILL.md new file mode 100644 index 0000000000..d2c1367a79 --- /dev/null +++ b/.claude/skills/proto-bump/SKILL.md @@ -0,0 +1,88 @@ +--- +name: proto-bump +description: Change the org.meshtastic:protobufs pin for Meshtastic-Android — either bump to a tagged release (mergeable) or track develop-SNAPSHOT for a preview (draft), adding/removing the transitive resolution-force hack as appropriate. Verifies with test/allTests (not just compile) via the gradle-runner subagent, audits the new field surface, and opens a PR. Use to consume new proto changes or to re-pin a SNAPSHOT draft onto a tag. +disable-model-invocation: true +--- + +# proto-bump + +Changes the upstream Meshtastic protobufs Maven pin. Protobuf models are **not** generated in this repo — they come from `org.meshtastic:protobufs` (Square Wire–built KMP models), pinned in `gradle/libs.versions.toml`. A bump is a catalog edit + verification, never a hand-edit of generated proto. + +> **Renovate already watches the catalog** for new *tagged* `org.meshtastic:protobufs` releases and opens the bump PR for you. Use this skill to **drive/finish** such a bump (verify, audit, adapt call sites), or — the part Renovate can't do — to track an unreleased `develop-SNAPSHOT`. + +## Two modes + +| | Mode A — tagged release | Mode B — develop-SNAPSHOT | +|---|---|---| +| Version | `X.Y.Z` | `develop-SNAPSHOT` | +| Transitive force block | **absent** (remove if present) | **present** (add it) | +| PR state | mergeable | **draft** (un-block when a tag ships) | +| When | a release carries what you need | the change only exists on protobufs `develop` (precedent: #5790, #5834, lockdown re-port) | + +Pick the mode from the argument / context. If the change you need isn't in any tag yet, it's Mode B. + +## Mode A — bump to a tagged release (mergeable) + +1. Read the current pin: `meshtastic-protobufs = "<…>"` in `gradle/libs.versions.toml`. Confirm the target is a real tag at https://github.com/meshtastic/protobufs/releases (or Maven Central), not a SNAPSHOT. +2. Set `meshtastic-protobufs = "X.Y.Z"`. +3. **Re-pin cleanup:** if the transitive force block (see below) is present in the root `build.gradle.kts`, **remove it** — a tagged protobufs is ordered correctly against transitive pins, so the force is unnecessary and misleading once on a release. Also re-check: is takpacket/mqtt now republished against this protobufs? If still pinning an older one, you may need to keep the force (note it in the PR). +4. **Verify** (step "Verification" below) — including `test`/`allTests`. +5. **Audit** the new additive surface (below). +6. Open a normal (non-draft) PR. + +## Mode B — track develop-SNAPSHOT (draft only) + +1. Set `meshtastic-protobufs = "develop-SNAPSHOT"`. +2. **Add the transitive force block** to the bottom of the root `build.gradle.kts` (exact text below). No repository change is needed — `settings.gradle.kts` already declares the Sonatype maven-snapshots repo (`snapshotsOnly()`) and JitPack (`https://jitpack.io`), which is where `develop-SNAPSHOT` resolves from. +3. **Verify** — including `test`/`allTests` (this is the mode where skipping them bites; see below). +4. **Audit** the new additive surface. +5. Open the PR **as a draft**, stating the un-block condition: *"merge once protobufs `vX.Y.Z` is tagged; switch to Mode A (set the tag + remove the force block) first."* + +## The transitive force block (why it exists, exact code) + +`takpacket-sdk` (and the MQTT client) transitively pin a **tagged** `protobufs` (e.g. `2.7.25`). Gradle ranks `2.7.25` **above** `develop-SNAPSHOT` (a numeric component outranks the `develop` string qualifier), so the *test runtime* classpath silently downgrades to the tagged proto while the *common-metadata compile* uses the snapshot. The mismatch throws `NoSuchFieldError`/`NoSuchMethodError` on proto-generated classes **at test runtime** — and crucially `assembleDebug`/`detekt` do **not** catch it; only `test`/`allTests` do. The block forces every `org.meshtastic:protobufs*` variant to the snapshot so compile and runtime agree: + +```kotlin +// ─── TEMPORARY: protobufs develop-SNAPSHOT preview (PR #NNNN) ───────────────────────────────────── +// We track the unreleased protobufs develop-SNAPSHOT. takpacket-sdk-jvm transitively pins a tagged +// protobufs, and Gradle ranks the tag > develop-SNAPSHOT (a numeric part outranks the "develop" +// string qualifier). That downgrades the test *runtime* classpath to the tag while the common-metadata +// *compile* uses the snapshot, yielding NoSuchFieldError/NoSuchMethodError on the proto-generated +// classes at test runtime (assembleDebug/detekt don't catch it; test/allTests do). Force every +// protobufs* variant to the snapshot so compile and runtime agree. Safe while atak.proto is unchanged, +// so takpacket's own message ABI stays compatible with the newer protobufs. +// REMOVE once protobufs is tagged (Mode A) / takpacket + mqtt are republished against it. +allprojects { + configurations.all { + resolutionStrategy.eachDependency { + if (requested.group == "org.meshtastic" && requested.name.startsWith("protobufs")) { + useVersion("develop-SNAPSHOT") + because("preview #NNNN: override takpacket transitive protobufs pin") + } + } + } +} +``` + +Update `#NNNN` to the current PR. Remove this entire block when returning to a tagged release (Mode A step 3). + +## Verification (don't skip test/allTests) + +Dispatch the **gradle-runner** subagent (keep the heavy log out of context). The downgrade failure mode above is a *runtime* classpath problem invisible to compilation, so verification MUST exercise tests: + +- `kmpSmokeCompile` + a broad compile of proto-consuming modules (`core:*`, `feature:*`) — catches *compile-time* breaking changes (renamed/removed fields, changed oneof shapes). +- **`test` and `allTests`** — the only gate that catches the transitive runtime downgrade. Treat a green compile as necessary-but-not-sufficient. + +Triage each compile/test failure: adapt this repo's call sites, or, if a break is unexpected, stop and report rather than papering over it. + +## Audit the new additive surface (recommended) + +New proto versions usually add fields/messages the app doesn't consume yet. Diff the new surface against current usage and list what became implementable. **Caveat from prior audits: verify each reference directly** — automated gap-lists for this repo have been wrong as often as right. Treat the list as candidates, not facts; don't implement them in this PR unless asked. + +## PR + guardrails + +- Write the PR per `.github/copilot-pull-request-instructions.md`: WHY-first; 🛠️ (or 🌟 if it unlocks a user-facing feature); link the upstream release notes (real URL only); "Testing Performed" = the gradle-runner run including `allTests`. +- Never hand-edit or vendor generated proto — this repo consumes the Maven artifact only. +- Keep the change minimal: a bump PR is a catalog edit + the force block (Mode B) + necessary call-site adaptations, not a feature. +- Branch off `main` (the 2.8.0 line) unless told otherwise. +- After a successful change, update the relevant memory pointer (protobufs-sdk-alignment / lora-region-preset-map) so the next session knows the new baseline. diff --git a/.coderabbit.yaml b/.coderabbit.yaml new file mode 100644 index 0000000000..ca16dbafb5 --- /dev/null +++ b/.coderabbit.yaml @@ -0,0 +1,328 @@ +# yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json +# CodeRabbit config — see https://docs.coderabbit.ai/getting-started/yaml-configuration +language: en-US + +reviews: + # chill = fewer nitpicks. CI already gates detekt/spotless/tests, and the + # maintainers are experienced — we want CodeRabbit for substance, not lint noise. + profile: chill + high_level_summary: true + poem: false + # Don't burn reviews on WIP. This repo opens lots of draft PRs; review on "ready". + # Skip Renovate dependency updates — CI gates dependencies; we review for substance, not every bump. + auto_review: + enabled: true + drafts: false + # Review once when the PR goes ready, then on demand via `@coderabbitai review`. + # Re-reviewing every push turned 6-commit PRs into 8 review rounds, because each + # fix commit reopened a full pass. Batch the fixes, push, then ask for one re-review. + auto_incremental_review: false + ignore_usernames: + - renovate + - renovate[bot] + # Workflow-authored PRs (changelog updates, scheduled firmware/hardware/ + # translation bumps) — machine-generated content, nothing to review. + - github-actions + - github-actions[bot] + ignore_title_keywords: + - "chore: Scheduled updates" + # Stop reviewing once a PR is closed. + abort_on_close: true + + path_filters: + # Generated / huge / non-source — don't review, just noise + token burn. + - "!**/build/**" + - "!**/*.png" + - "!**/*.webp" + - "!**/firmware_releases.json" + - "!**/emoji-data.json" + - "!**/flatpak-sources.json" + # Crowdin-managed translations — owned upstream, not hand-edited here. + - "!**/values-*/strings.xml" + # Spec Kit scaffolding — vendored tooling, not hand-maintained here. + - "!.specify/**" + + path_instructions: + # Global review contract. Everything below this entry is area-specific detail; + # this is what "a finding" means in this repo at all. + - path: "**" + instructions: >- + Report problems only. Every comment must name a concrete defect with evidence in the diff. + No praise, no style preferences, no speculative design feedback, no "consider extracting this" + on code that works. + + + FLAG these categories: + + 1. Bugs — logic errors, off-by-one, null dereference, a missing `await`/`join`, dropped + cancellation, incorrect disposal order. + + 2. Security — credential or key exposure, injection, insecure defaults, PII/location/crypto-key + material reaching a log sink or analytics payload. + + 3. Correctness — behaviour that contradicts the PR description, the linked issue, or an existing + contract; a breaking public-API change with no justification. + + 4. Behavioural contract changes — when a type is replaced, removed, or refactored, diff the OLD + implementation against the NEW one. Look for a removed `override`, a property that used to throw + on invalid access and now returns a default, an exception type that changed, and call sites that + depended on the removed type's specific behaviour. + + 5. Weakened invariants — validation quietly relaxed during a refactor. Kotlin shapes: + `single()`/`first()` (throws) swapped for `firstOrNull()` (silently picks nothing); + a deleted `require`/`check`/`error`; `!!` replaced by `?: ` so a broken state becomes a + plausible value; an exhaustive `when` gaining an `else ->` branch that swallows new cases. + + 6. Missing error handling at system boundaries — unvalidated input from the radio, a peer, an + MQTT broker, a deep link, or an intent extra. Do NOT flag missing null checks the Kotlin type + system already guarantees. + + 7. Performance regressions — allocation in a hot path or a recomposition scope, N+1 database + queries, `runBlocking`/`Thread.sleep`/`.get()` on a coroutine or UI path, blocking + `getString()` on `Dispatchers.Default`, an unstable Compose parameter type that defeats skipping. + + 8. Concurrency — unguarded shared mutable state, a read-decide-write sequence spanning a suspend + boundary, a mutex held across a suspending call, deadlock and lock-ordering risk. + + 9. Temporal coupling and initialisation safety — `lateinit var` paired with a separate + `initialize()`/`start()` that a caller must remember, DI registrations that only work in one + resolution order, any pattern where a forgotten call is a runtime crash with no compile-time + signal. + + 10. Resource leaks — a `CoroutineScope` created and never cancelled, a `Closeable`/`AutoCloseable` + outside `use {}`, a registered receiver/listener/callback with no matching unregister, a BLE or + socket connection not closed on every exit path. Flag these even when the pattern was moved in + from elsewhere. + + 11. Dead code and stale comments — a comment describing behaviour the code no longer has, an + unused local or parameter, a materialising call whose result is never consumed. + + 12. Repository convention violations — see AGENTS.md and .skills/. Highest value: `java.*` or + `android.*` in `commonMain`, hardcoded user-facing strings, `runCatching` in a suspend context + instead of `safeCatching`, OkHttp instead of Ktor, hand-edits to generated or Crowdin-owned files. + + 13. Comment problems — a comment that contradicts the code, a workaround with no tracking link, a + parser or protocol handler that omits the raw wire shape needed to read the edge cases, and + privacy- or security-sensitive behaviour whose comment fails to explain scope and WHY. Do not ask + for comments on obvious code. + + + Do NOT flag: style already enforced by detekt, spotless, or .editorconfig; missing KDoc, unless a + new public API is entirely undocumented; refactoring suggestions for code the PR did not touch; + generated output (Wire protos, Crowdin locale files, baseline profiles, docs screenshots); + missing tests for docs-only, comment-only, or mechanical-rename changes. + + + Moved or extracted code counts as newly written. Review it on its merits and flag pre-existing + defects that came along with it, labelled "pre-existing — good opportunity to fix during this + refactor" so the author can weigh scope. + + + One problem per comment. Cite the exact line, symbol, or condition. Give a fix direction — a + snippet when the fix is not obvious. Never restate a finding already raised in an existing + review thread. + - path: "**/commonMain/**" + instructions: > + KMP common code. Flag any import of java.* or android.* — these break non-Android targets. Expect KMP equivalents instead (Okio, kotlinx Mutex/atomicfu, NumberFormatter.format() for floats). + - path: "**/*.kt" + instructions: > + Flag leftover // ... existing code ... placeholders, and any logging of PII, location, or cryptographic keys. + - path: "**/src/**/strings.xml" + instructions: > + New string resources must be alphabetically sorted (scripts/sort-strings.py). Flag out-of-order additions. + - path: baselineprofile/ + instructions: Keep baseline profile generation tied to the `google` flavor and connected devices/emulators, and commit the generated profile output to `androidApp/src/google/generated/baselineProfiles/baseline-prof.txt`. + - path: docs/ + instructions: Treat non-English locale folders as Crowdin-managed output; edit the English sources under `docs/en/` and register new pages through `feature/docs/` instead of hand-editing translated locale directories. + - path: screenshot-tests/ + instructions: When updating docs screenshots, keep `docs-screenshots-manifest.txt` and `docs-screenshot-aliases.properties` in sync with the generated files, and rerun `copyDocsScreenshots` after regenerating screenshots. + - path: docs-screenshots/ + instructions: Keep this module generate-only for documentation screenshots; do not add it to the CI validation gate that is reserved for `screenshot-tests`. + - path: desktopApp/ + instructions: Keep desktop release ProGuard rules aligned with `androidApp/proguard-rules.pro`, and preserve the desktop-specific runtime wiring needed for `Dispatchers.Main` on JVM. + - path: androidApp/ + instructions: Keep the Android app’s `MeshService` declaration and manifest wiring in sync with the implementation that lives in `core:service`. + - path: core/service/ + instructions: Keep `RadioControllerImpl` composed from its sub-controllers via interface delegation; admin sends are fire-and-forget, and any config mutation must go through `editSettings { }` transactions. + - path: feature/docs/ + instructions: Treat the Compose resources under `src/commonMain/composeResources/files/` as generated output from `/docs/en/**` and translated docs sync tasks; do not hand-edit those copied files. + - path: feature/map/ + instructions: Route map access through the injected `CompositionLocal` provider contracts; do not depend directly on Google Maps or osmdroid from feature code. + - path: feature/car/ + instructions: Run unit tests with `./gradlew :feature:car:testGoogleDebugUnitTest`, and keep Robolectric pinned to SDK 36 for this module. + - path: core/database/ + instructions: >- + Review focus: schema compatibility. A `@Database` version bump must ship a new + `core/database/schemas/.json` AND an (n-1)→n test under `androidHostTest` that inserts rows at + the old version, migrates, and asserts row count and column values survive — not merely that the + migration executes. A column going nullable must assert both that pre-existing values are retained + and that the new NULL state is reachable. Flag `@Insert`+`@Update` pairs that should be `@Upsert`, + single-row queries missing `LIMIT 1`, and N+1 patterns (a loop issuing single-row queries) that + should be a chunked `WHERE IN`. + - path: core/network/ + instructions: >- + Review focus: request lifecycle. Timeouts and base URLs come from `HttpClientDefaults`; flag + hardcoded timeouts or absolute URLs in callers. Check every failure path for cancellation + propagation (`safeCatching`, not `runCatching`) and for a response body that is closed on error as + well as success. + - path: .github/workflows/ + instructions: >- + Review focus: unintended side effects. Flag a change that widens a trigger (especially + `pull_request_target` and anything granting write scopes to fork-authored code), a secret exposed + to an untrusted context, a cache key that lets one job poison another, and an unpinned third-party + action. Job-level `continue-on-error` or a removed `--fail`-style guard silently converts a broken + gate into a green tick — flag it. + - path: "**/*.gradle.kts" + instructions: >- + Review focus: build correctness over cleanliness. Several idioms here are load-bearing and look + redundant — do not suggest removing an explicit dependency declaration, a duplicated exclusion, or + an apparently no-op configuration block without evidence from the diff that it is dead. Flag + changes that alter variant/flavor wiring, drop a keep rule, or make a task's inputs/outputs + untracked (which silently disables caching and up-to-date checks). + + # Every CodeRabbit tool is enabled by default, so this block only ever needs to + # turn things OFF or configure them. detekt is off because CI owns it (Zero Lint + # Tolerance gate) and duplicate comments were the noise we removed. The scanners + # CI doesn't run — gitleaks, shellcheck, actionlint, zizmor, semgrep, trivy, + # presidio (PII), buf (protobuf) — are already on by default; don't re-list them. + tools: + detekt: + enabled: false + # Custom AST rules mechanically enforce the recurring defect classes that prose + # can't. See .coderabbit/ast-grep-rules/ and .skills/code-review/SKILL.md. + # essential_rules stays on (default) — these are additive. + ast-grep: + rule_dirs: + - ".coderabbit/ast-grep-rules" + + # Auto-generated docstrings/tests/autofix are noisy for a repo with strict + # human-authored KDoc and KMP-aware tests; leave finishing touches off. + finishing_touches: + docstrings: + enabled: false + unit_tests: + enabled: false + + # No KDoc-coverage mandate in this repo; the default warning-at-80% check + # would nag every PR. PR titles are already linted by CI + # (.github/workflows/pull-request-target.yml), so no title check here either. + pre_merge_checks: + docstrings: + mode: "off" + # Defect classes that survive review-by-prose because they are about what is + # ABSENT from a diff — a sibling call site left unfixed, a test that would still + # pass with the fix reverted, a regression nobody covered, a behaviour quietly + # dropped while code moved files. A per-file reviewer never sees these; a + # whole-PR check does. Warning, not error: all four are judgment calls and a + # false positive must not block a merge. + custom_checks: + - name: "Sibling call sites and presence semantics" + mode: "warning" + instructions: >- + When a diff changes how an absent value is represented — making a field nullable, + removing a zero-guard, or adding a presence check — verify EVERY call site of that + field was updated, not just the one the bug was reported against. Ambient temperature + was fixed in NodeItem.kt while its sibling NodeItemCompact.kt kept the zero-guard. + Name any unfixed sibling explicitly. Also flag a new field defaulting to 0 where 0 is + a physically reachable value on that scale (RSSI, temperature, current, voltage, + particulate concentration). Two exceptions, do NOT flag either: humidity, where 0 %RH + is unreachable and the guard is intentional and tested; and the proto `rx_snr`, which + has no presence upstream, so its 0f ambiguity cannot be fixed app-side. An app-level + SNR field that IS nullable is still in scope. + - name: "Tests prove the path, not the end state" + mode: "warning" + instructions: >- + For each added or changed test, decide whether it would still pass if the production + code it covers were reverted. Flag tests that seed a fake's backing store and then + assert the value comes back, tests that assert only a collection's size rather than + which items survived, and tests asserting emission ORDER under Dispatchers.Unconfined + (not a stable contract). A test must assert the side effect only the intended path + produces — a call counter, a request issued, a cache written. + - name: "Regression coverage for changed behavior" + mode: "warning" + instructions: >- + Do not stop at "tests pass" or "there are tests". For each non-trivial production change in + the diff, work through four steps and report only the gaps. + + + 1. Changed behaviour — name the concrete code path, function, or configuration key from the + diff whose behaviour changed. + + 2. Observable surfaces — which of these can see the change: public API, the mesh/radio + protocol handling, persisted database rows, DataStore preferences, Compose UI state, + navigation, notifications, the foreground service lifecycle, BLE/serial/TCP transport, + MQTT, widgets, Android Auto, the desktop app, or R8/ProGuard-shaped release behaviour. + + 3. Regression risks — the specific ways this could break a working scenario: ordering and + timing changes, reconnect and retry paths, process death and state restore, schema + compatibility for rows written by an older build, cross-module call sites, flavor + differences (google vs fdroid), and platform differences (Android vs JVM vs iOS targets). + + 4. Coverage gap — name the test that should exist and does not. + + + A bug fix needs a test that FAILS without the fix. A test that only exercises the happy path, + or a regenerated snapshot/golden file, does not prove a behaviour change. Flag a PR whose only + test evidence is an updated screenshot golden, an updated Room schema JSON, or a regenerated + baseline profile when the change is behavioural. + + + State each finding as: impacted code path, the regression risk, the missing test shape. Be + specific enough that the author can write the test from the comment. + + + Do NOT ask for tests for: documentation-only or comment-only changes, mechanical renames, + dependency version bumps, or refactors the diff shows to be behaviour-preserving. Do not + demand a test category for a surface the change cannot reach — a `commonMain` formatting + helper does not need a transport test. + - name: "Moved code diffed against its original" + mode: "warning" + instructions: >- + Applies when the diff deletes a type/function in one file and adds something similar + elsewhere, or extracts code into a new file or module. Treat the moved code as newly written + and compare the OLD implementation against the NEW one line by line. + + + Flag any of the following that the move introduced silently: + + - a removed `override`, or an interface member the new type no longer implements; + + - a changed exception contract — something that threw now returns a default, or vice versa; + + - a dropped `require`/`check`/`init` block validation, or a narrowed visibility widened; + + - a default parameter value that changed, which alters every call site that omitted it; + + - a nullability change on a numeric field, which is the presence-vs-sentinel-zero class; + + - a lost `@Serializable`/`@Parcelize`/Koin annotation, or a scope change (`@Single` to + `@Factory`) that alters instance lifetime; + + - a coroutine scope, dispatcher, or `SharingStarted` policy that changed during the move. + + + Then check the call sites of the removed declaration: every caller that relied on the old + behaviour must still be correct. Name any caller the PR left on the old assumption. + + + Pre-existing defects carried into the new location are in scope — label them "pre-existing — + good opportunity to fix during this refactor" so the author can decide on scope. Do not flag + a move that the diff shows to be genuinely mechanical. + +knowledge_base: + # Learnings are how a confirmed finding stops recurring on the next PR. Pin the + # scope to this repo: the default `auto` already resolves to `local` for public + # repos, but being explicit keeps it from shifting if visibility ever changes. + learnings: + scope: local + # Feed CodeRabbit the same guidance human/AI contributors follow, including + # the repo-specific .skills/ modules and Copilot path instructions it + # wouldn't pick up by default. + code_guidelines: + enabled: true + filePatterns: + - "AGENTS.md" + - "CLAUDE.md" + - ".skills/**/SKILL.md" + - ".github/copilot-instructions.md" + - ".github/instructions/*.instructions.md" diff --git a/.coderabbit/ast-grep-rules/no-float-metric-zero-sentinel.yml b/.coderabbit/ast-grep-rules/no-float-metric-zero-sentinel.yml new file mode 100644 index 0000000000..04485c055a --- /dev/null +++ b/.coderabbit/ast-grep-rules/no-float-metric-zero-sentinel.yml @@ -0,0 +1,27 @@ +# Recurring defect class A — see "Presence vs sentinel zero" in .skills/code-review/SKILL.md. +# +# The protobuf models are Wire-generated, so presence IS nullability: EnvironmentMetrics.temperature and friends are +# declared `Float? = null` and there is no hasX() accessor. A `(x ?: 0f) != 0f` guard collapses "absent" and +# "measured zero" into one state and silently discards a real reading. Use `x?.let { ... }` instead — see +# `gatherSensors` in core/ui/.../NodeItem.kt for the reference pattern. +# +# Humidity is the deliberate exception: 0 %RH is not physically reachable, so relative_humidity / co2_humidity keep +# their zero-guards and are not listed here. Same for barometric_pressure (0 hPa is a vacuum). +id: no-float-metric-zero-sentinel +language: kotlin +severity: error +message: "0 is a real reading on this scale — use `?.let { }` instead of a zero-guard." +# Every spelling of the zero-float literal is listed: a reformat from `0f` to `0.0f` must not slip past the rule. +rule: + any: + - pattern: "($X ?: 0f) != 0f" + - pattern: "($X ?: 0f) == 0f" + - pattern: "($X ?: 0F) != 0F" + - pattern: "($X ?: 0F) == 0F" + - pattern: "($X ?: 0.0f) != 0.0f" + - pattern: "($X ?: 0.0f) == 0.0f" + - pattern: "($X ?: 0.0F) != 0.0F" + - pattern: "($X ?: 0.0F) == 0.0F" +constraints: + X: + regex: "(temperature|voltage|current|soil_moisture)$" diff --git a/.coderabbit/ast-grep-rules/no-rssi-zero-default.yml b/.coderabbit/ast-grep-rules/no-rssi-zero-default.yml new file mode 100644 index 0000000000..0d1f437f03 --- /dev/null +++ b/.coderabbit/ast-grep-rules/no-rssi-zero-default.yml @@ -0,0 +1,20 @@ +# Recurring defect class A — see "Presence vs sentinel zero" in .skills/code-review/SKILL.md. +# +# 0 dBm is the STRONGEST value on the RSSI scale, so defaulting a missing reading to 0 renders an unknown signal as an +# excellent one. Keep the value nullable end-to-end and let `MetricFormatter.rssi(null)` render an em dash. +# +# Scope: this rule targets DEFAULTING a live reading (`?: 0`), not comparing a stored one. A `rssi == 0` test against +# persisted data can be legitimate migration handling — `Reaction.kt` reads pre-schema-51 rows that stored 0 where the +# column is now nullable, so there a 0 really is indistinguishable from "no reading". Broadening this rule to `== 0` +# would flag that documented exception and nothing else, so it deliberately stops at the `?: 0` form. +id: no-rssi-zero-default +language: kotlin +severity: error +message: "0 dBm is the strongest RSSI, not \"unknown\" — keep the value nullable." +rule: + any: + - pattern: "$X ?: 0" + - pattern: "($X ?: 0) != 0" +constraints: + X: + regex: "(?i)rssi$" diff --git a/.coderabbit/ast-grep-rules/presence-vs-sentinel-zero-float.yml b/.coderabbit/ast-grep-rules/presence-vs-sentinel-zero-float.yml new file mode 100644 index 0000000000..f11268adab --- /dev/null +++ b/.coderabbit/ast-grep-rules/presence-vs-sentinel-zero-float.yml @@ -0,0 +1,19 @@ +# Class A of "Recurring Defect Classes" in .skills/code-review/SKILL.md, enforced mechanically. +# 0 is a real reading on these scales, so a zero-guard silently drops a genuine measurement. +# Humidity is deliberately excluded: 0 %RH is not physically reachable, and +# EnvironmentMetricsForGraphingTest.humidity_zeroFilteredOut asserts that guard on purpose. +id: presence-vs-sentinel-zero-float +language: kotlin +severity: warning +message: >- + Zero-guard on a zero-inclusive scale drops real readings. 0 is valid for temperature, + current, voltage and soil moisture, so `(x ?: 0f) != 0f` hides a genuine measurement and + conflates it with "not reported". Use a null check — `x?.let { }` — as NodeItem.kt does for + ambient temperature. Humidity is the one legitimate exception and is excluded from this rule. +note: >- + See .skills/code-review/SKILL.md, "Recurring Defect Classes", class A. +rule: + pattern: "($FIELD ?: 0f) != 0f" +constraints: + FIELD: + regex: "\\.(temperature|soil_temperature|co2_temperature|current|voltage|soil_moisture)$" diff --git a/.coderabbit/ast-grep-rules/presence-vs-sentinel-zero-signal.yml b/.coderabbit/ast-grep-rules/presence-vs-sentinel-zero-signal.yml new file mode 100644 index 0000000000..758a783ae4 --- /dev/null +++ b/.coderabbit/ast-grep-rules/presence-vs-sentinel-zero-signal.yml @@ -0,0 +1,33 @@ +# Class A of "Recurring Defect Classes" in .skills/code-review/SKILL.md, enforced mechanically. +# Presence in this repo is nullability: the models are Wire-generated, so there is no hasX() +# accessor — an optional field is simply a nullable type. The proto `rx_rssi` only becomes `Int?` +# in protobufs 2.7.26.138 (PR #6498, still open at time of writing); on the pinned 2.7.26.130 it +# is `Int = 0`. So this rule currently guards APP-level nullable rssi (parameters, state, BLE +# advertisement values) and becomes a proto regression guard once #6498 lands. +# +# INT LITERALS ONLY, deliberately. RSSI is an integer dBm value everywhere in this repo — +# proto `rx_rssi: Int`, `Node.rssi: Int`, `MetricFormatter.rssi(value: Int)` — and there is no +# Float-typed rssi declaration anywhere. Adding `?: 0f` / `takeIf { it != 0f }` variants here +# would be dead patterns. Float sentinels on genuinely-Float metrics are the float rule's job. +# RSSI only, deliberately NOT snr: rx_snr still has no proto presence upstream, so a 0f guard there +# is genuinely ambiguous and unfixable app-side — flagging it would be pure noise on code nobody can +# correct. Also excludes `replyId ?: 0` and `packet.from.takeIf { it != 0 }`, which are legitimate +# because 0 there means "unset", not "a measurement of zero". +id: presence-vs-sentinel-zero-signal +language: kotlin +severity: warning +message: >- + Zero-default on a signal metric conflates "not reported" with a real reading. 0 dBm is a + valid RSSI (SX126x reports exactly 0, SX127x can go positive) and on a signal-strength scale + it renders as the STRONGEST value — so an unknown signal displays as excellent. Keep the type + nullable and branch on null, rendering absence explicitly rather than substituting a number. +note: >- + See .skills/code-review/SKILL.md, "Recurring Defect Classes", class A. rx_snr still has no + proto presence upstream, so its 0f ambiguity cannot be fixed app-side — do not re-raise that. +rule: + any: + - pattern: "$RECV.takeIf { it != 0 }" + - pattern: "$RECV ?: 0" +constraints: + RECV: + regex: "(?i)rssi$" diff --git a/.copilotignore b/.copilotignore index 02ec3ad1d4..f53a45d63c 100644 --- a/.copilotignore +++ b/.copilotignore @@ -1,27 +1,54 @@ -# Ignore build artifacts and generated files from Copilot indexing +# Meshtastic Android - GitHub Copilot Ignore List # This saves context window tokens and prevents Copilot from hallucinating off of minified code. -# Build directories +# ── Build & Generated ───────────────────────────────────────────────────────── **/build/** .gradle/ -.idea/ - -# Android generated files +.kotlin/ **/generated/** .cxx/ .externalNativeBuild/ -# Git history & worktrees -.git/ -.worktrees/ - -# Protobuf (Prevents Copilot from suggesting raw protobuf byte buffers) -core/proto/ - -# Environment and secrets +# ── IDE & Environment ───────────────────────────────────────────────────────── +.idea/ +.run/ +.claude/ +.gemini/ +.jdk local.properties secrets.properties *.jks +.DS_Store -# Agent References (Prevents pollution of project space with external code) +# ── Agent Artifacts (Large volumes of logs/images) ─────────────────────────── +.agent_artifacts/ +# Note: .agent_plans/ is NOT ignored to maintain implementation context. .agent_refs/ +tmp/ +*.log + +# ── Binary Assets & Media ───────────────────────────────────────────────────── +**/*.png +**/*.jpg +**/*.jpeg +**/*.webp +**/*.svg +**/*.ico +**/*.gif +**/*.mp3 +**/*.wav +**/*.ogg +**/*.pdf +**/*.ttf +**/*.otf +**/*.jar +**/*.aar +**/*.apk + +# ── External & Submodules ───────────────────────────────────────────────────── +core/proto/ + +# ── Resources ──────────────────────────────────────────────────────────────── +# Ignore translations (reduces churn and indexing tokens) +**/values-*/strings.xml +**/composeResources/**/values*/*.xml diff --git a/.gitattributes b/.gitattributes new file mode 100644 index 0000000000..f0f137f254 --- /dev/null +++ b/.gitattributes @@ -0,0 +1,13 @@ +# Mark only generated/derived XML as linguist-generated to reduce Copilot PR summary costs. +# Hand-edited resources (layouts, string values) are intentionally excluded so they remain +# visible in diffs and code review. +**/composeResources/**/values-*/*.xml linguist-generated=true +.skills/compose-ui/strings-index.txt linguist-generated=true + +# Ensure assets are treated as binary +*.png binary +*.jpg binary +*.webp binary +*.mp3 binary +*.wav binary +*.ogg binary diff --git a/.github/actions/gradle-setup/action.yml b/.github/actions/gradle-setup/action.yml index a429591902..f0e4e90a24 100644 --- a/.github/actions/gradle-setup/action.yml +++ b/.github/actions/gradle-setup/action.yml @@ -7,9 +7,37 @@ inputs: jdk_distribution: description: 'JDK distribution (temurin or jetbrains)' default: 'temurin' + install_jetbrains_jdk: + description: 'Also install JetBrains JDK 25 for Compose Desktop toolchain resolution' + default: 'false' gradle_encryption_key: description: 'Encryption key for Gradle remote cache' required: false + develocity_access_key: + description: 'Access key for the OSS Community Develocity Instance (Build Scan publishing and remote cache writes)' + required: false + job_summary_pr_comment: + description: | + Job summary as a PR comment: never | always | on-failure. Needs `pull-requests: write`; + no-ops on fork PRs (read-only token). + default: 'never' + dependency_graph: + description: | + Dependency graph mode: disabled | generate | generate-and-submit | generate-and-upload | + download-and-submit. Submit needs `contents: write`. Never combine with + cache_configuration_cache — a CC-hit build generates NO graph. + default: 'disabled' + cache_configuration_cache: + description: | + Persist .gradle/configuration-cache. Opt-in: only pays off when config inputs are + commit-stable (the VERSION_CODE-pinned jobs). Real-versionCode jobs would restore, + miss, and never re-save. + default: 'false' + cache_key_suffix: + description: | + Extra CC-key discriminator for matrix legs running different task graphs (test-shards). + Runner-only matrices don't need it — os/arch are already in the key. + default: '' runs: using: composite steps: @@ -20,21 +48,108 @@ runs: - name: Validate Gradle Wrapper uses: gradle/actions/wrapper-validation@v6 - - name: Set up JDK 21 + - name: Set up JDK 25 uses: actions/setup-java@v5 with: - java-version: '21' + java-version: '25' distribution: ${{ inputs.jdk_distribution }} token: ${{ github.token }} + - name: Restore cached JetBrains JDK 25 + if: inputs.install_jetbrains_jdk == 'true' + id: cache-jbr + uses: actions/cache@v6 + with: + path: ${{ runner.tool_cache }}/Java_JetBrains_jdk + key: jbr-25-${{ runner.os }}-${{ runner.arch }} + + - name: Set up JetBrains JDK 25 (for Compose Desktop) + if: inputs.install_jetbrains_jdk == 'true' && steps.cache-jbr.outputs.cache-hit != 'true' + id: setup-jbr + continue-on-error: true + uses: actions/setup-java@v5 + with: + java-version: '25' + distribution: 'jetbrains' + check-latest: false + token: ${{ github.token }} + + - name: JBR setup skipped or failed — Gradle will auto-provision via Foojay + if: inputs.install_jetbrains_jdk == 'true' && steps.cache-jbr.outputs.cache-hit != 'true' && steps.setup-jbr.outcome == 'failure' + shell: bash + run: echo "::warning::JBR setup-java failed (likely GitHub API rate limit). Gradle will auto-provision JBR via Foojay toolchain resolver." + + # Kotlin/Native lives in ~/.konan, OUTSIDE the Gradle home that + # setup-gradle caches — without this every job re-downloads the K/N + # toolchain (downloadKotlinNativeDistribution) and the iOS-target compile + # tasks it feeds miss the build cache, while the identical tasks hit + # FROM-CACHE locally. Keyed on the version catalog: over-invalidates on + # unrelated bumps, but konan re-downloads are exactly what it prevents. + - name: Cache Kotlin/Native toolchain + uses: actions/cache@v6 + with: + path: ~/.konan + key: konan-${{ runner.os }}-${{ runner.arch }}-${{ hashFiles('gradle/libs.versions.toml') }} + + # Robolectric downloads its android-all-instrumented jars to the local Maven + # repo, which lives outside the Gradle User Home and so isn't covered by + # setup-gradle's caching. (This was previously attempted via + # gradle-home-cache-includes, but those entries resolve relative to the + # Gradle User Home — the `~/.m2/...` line expanded to the literal path + # `/~/.m2/...` and never matched anything.) Keyed on the + # version catalog, which pins the Robolectric version. + - name: Cache Robolectric android-all jars + uses: actions/cache@v6 + with: + path: ~/.m2/repository/org/robolectric + key: robolectric-${{ runner.os }}-${{ runner.arch }}-${{ hashFiles('gradle/libs.versions.toml') }} + restore-keys: | + robolectric-${{ runner.os }}-${{ runner.arch }}- + - name: Setup Gradle uses: gradle/actions/setup-gradle@v6 with: cache-read-only: ${{ inputs.cache_read_only }} cache-encryption-key: ${{ inputs.gradle_encryption_key }} - cache-cleanup: on-success + develocity-access-key: ${{ inputs.develocity_access_key }} + # Cache cleanup is broken on Windows with setup-gradle v6's "enhanced + # caching" provider: it deletes the whole Gradle User Home instead of + # just unused entries (1739 files "excluded from the cache" here vs 25 + # on Linux), so every subsequent save fails path validation and the + # Windows cache is never written. Upstream: gradle/actions#1013. + # Skip cleanup there — a slightly larger cache beats no cache at all. + cache-cleanup: ${{ runner.os == 'Windows' && 'never' || 'on-success' }} add-job-summary: always + add-job-summary-as-pr-comment: ${{ inputs.job_summary_pr_comment }} + dependency-graph: ${{ inputs.dependency_graph }} gradle-home-cache-includes: | caches notifications - ~/.m2/repository/org/robolectric \ No newline at end of file + # `caches` sweeps in caches/build-cache-1, the LOCAL build cache — which since the + # Develocity onboarding (#6531) duplicates the remote cache at + # community.develocity.cloud. Shipping both means paying tarball upload/download + # for entries the remote already serves, on every one of the ~14 jobs that call + # this action, against a 10 GB repo-wide Actions cache quota that evicts under + # pressure. Dependency and transform caches (the expensive part) still ship. + # Effect is measurable in the Build Scan cache-performance view; revert if remote + # cache latency turns out to cost more than the restore it replaced. + gradle-home-cache-excludes: | + caches/build-cache-1 + + # CC entries live in the project dir; setup-gradle only caches the Gradle User Home — + # Develocity measured 100% CC miss (~52s config/build) before this. Runs after Setup + # Gradle so GRADLE_ENCRYPTION_KEY is exported. No sha in the key: unchanged build files + # hit exactly and skip the save; restore-keys is the self-heal for stale entries. + # The wrapper hash is its own restore-key segment: a stale same-version entry is a + # graceful CC miss, but an entry written by a DIFFERENT Gradle version can crash + # fingerprint deserialization outright (seen on 9.6.1 entries under 9.7.0). + - name: Cache Gradle configuration-cache + # Gates: opted in; key present (undecryptable otherwise — keeps keyless fork PRs off + # these entries); not merge_group (throwaway cache scope). + if: inputs.cache_configuration_cache == 'true' && inputs.gradle_encryption_key != '' && github.event_name != 'merge_group' + uses: actions/cache@v6 + with: + path: ${{ github.workspace }}/.gradle/configuration-cache + key: gradle-cc-${{ runner.os }}-${{ runner.arch }}-${{ github.job }}${{ inputs.cache_key_suffix && format('-{0}', inputs.cache_key_suffix) || '' }}-${{ hashFiles('gradle/wrapper/gradle-wrapper.properties') }}-${{ hashFiles('settings.gradle.kts', '**/build.gradle.kts', 'build-logic/**', 'gradle/*.gradle', 'gradle/libs.versions.toml', 'gradle.properties', 'config.properties', '.github/ci-gradle.properties') }} + restore-keys: | + gradle-cc-${{ runner.os }}-${{ runner.arch }}-${{ github.job }}${{ inputs.cache_key_suffix && format('-{0}', inputs.cache_key_suffix) || '' }}-${{ hashFiles('gradle/wrapper/gradle-wrapper.properties') }}- diff --git a/.github/agents/speckit.agent-governance.refresh.agent.md b/.github/agents/speckit.agent-governance.refresh.agent.md new file mode 100644 index 0000000000..0916b88e5c --- /dev/null +++ b/.github/agents/speckit.agent-governance.refresh.agent.md @@ -0,0 +1,48 @@ +--- +description: Generate or update the active agent governance file +--- + + + + +# Agent Governance Generate/Update + +## Input + +$ARGUMENTS + +## Output + +- Active agent platform governance file. +- Managed `SPECKIT GOVERNANCE` section. +- `.specify/memory/agent-governance.md`: internal cache. + +## Procedure + +1. Require `.specify/`. +2. Resolve target: + - `.specify/init-options.json` `context_file` + - `.specify/integration.json` `default_integration` or `integration` + - `AGENTS.md` +3. Create internal cache when missing. +4. Generate target file when missing. +5. Update only the managed section when target exists. +6. Use existing managed section as refresh source. +7. Distill detected repository areas into action rules. + - depth: 2 + - include hidden and cache directories +8. Preserve content outside managed markers. +9. Preserve managed markers verbatim. +10. Run: + + ```bash + uv run python .specify/extensions/agent-governance/scripts/refresh_agent_governance.py + ``` + +## Report + +- target governance file +- generated or updated +- review target +- internal cache status +- captured evidence when cache is created \ No newline at end of file diff --git a/.github/agents/speckit.analyze.agent.md b/.github/agents/speckit.analyze.agent.md new file mode 100644 index 0000000000..1237e1065a --- /dev/null +++ b/.github/agents/speckit.analyze.agent.md @@ -0,0 +1,249 @@ +--- +description: Perform a non-destructive cross-artifact consistency and quality analysis across spec.md, plan.md, and tasks.md after task generation. +--- + +## User Input + +```text +$ARGUMENTS +``` + +You **MUST** consider the user input before proceeding (if not empty). + +## Pre-Execution Checks + +**Check for extension hooks (before analysis)**: +- Check if `.specify/extensions.yml` exists in the project root. +- If it exists, read it and look for entries under the `hooks.before_analyze` key +- If the YAML cannot be parsed or is invalid, skip hook checking silently and continue normally +- Filter out hooks where `enabled` is explicitly `false`. Treat hooks without an `enabled` field as enabled by default. +- For each remaining hook, do **not** attempt to interpret or evaluate hook `condition` expressions: + - If the hook has no `condition` field, or it is null/empty, treat the hook as executable + - If the hook defines a non-empty `condition`, skip the hook and leave condition evaluation to the HookExecutor implementation +- For each executable hook, output the following based on its `optional` flag: + - **Optional hook** (`optional: true`): + ``` + ## Extension Hooks + + **Optional Pre-Hook**: {extension} + Command: `/{command}` + Description: {description} + + Prompt: {prompt} + To execute: `/{command}` + ``` + - **Mandatory hook** (`optional: false`): + ``` + ## Extension Hooks + + **Automatic Pre-Hook**: {extension} + Executing: `/{command}` + EXECUTE_COMMAND: {command} + + Wait for the result of the hook command before proceeding to the Goal. + ``` +- If no hooks are registered or `.specify/extensions.yml` does not exist, skip silently + +## Goal + +Identify inconsistencies, duplications, ambiguities, and underspecified items across the three core artifacts (`spec.md`, `plan.md`, `tasks.md`) before implementation. This command MUST run only after `/speckit.tasks` has successfully produced a complete `tasks.md`. + +## Operating Constraints + +**STRICTLY READ-ONLY**: Do **not** modify any files. Output a structured analysis report. Offer an optional remediation plan (user must explicitly approve before any follow-up editing commands would be invoked manually). + +**Constitution Authority**: The project constitution (`.specify/memory/constitution.md`) is **non-negotiable** within this analysis scope. Constitution conflicts are automatically CRITICAL and require adjustment of the spec, plan, or tasks—not dilution, reinterpretation, or silent ignoring of the principle. If a principle itself needs to change, that must occur in a separate, explicit constitution update outside `/speckit.analyze`. + +## Execution Steps + +### 1. Initialize Analysis Context + +Run `.specify/scripts/bash/check-prerequisites.sh --json --require-tasks --include-tasks` once from repo root and parse JSON for FEATURE_DIR and AVAILABLE_DOCS. Derive absolute paths: + +- SPEC = FEATURE_DIR/spec.md +- PLAN = FEATURE_DIR/plan.md +- TASKS = FEATURE_DIR/tasks.md + +Abort with an error message if any required file is missing (instruct the user to run missing prerequisite command). +For single quotes in args like "I'm Groot", use escape syntax: e.g 'I'\''m Groot' (or double-quote if possible: "I'm Groot"). + +### 2. Load Artifacts (Progressive Disclosure) + +Load only the minimal necessary context from each artifact: + +**From spec.md:** + +- Overview/Context +- Functional Requirements +- Success Criteria (measurable outcomes — e.g., performance, security, availability, user success, business impact) +- User Stories +- Edge Cases (if present) + +**From plan.md:** + +- Architecture/stack choices +- Data Model references +- Phases +- Technical constraints + +**From tasks.md:** + +- Task IDs +- Descriptions +- Phase grouping +- Parallel markers [P] +- Referenced file paths + +**From constitution:** + +- Load `.specify/memory/constitution.md` for principle validation + +### 3. Build Semantic Models + +Create internal representations (do not include raw artifacts in output): + +- **Requirements inventory**: For each Functional Requirement (FR-###) and Success Criterion (SC-###), record a stable key. Use the explicit FR-/SC- identifier as the primary key when present, and optionally also derive an imperative-phrase slug for readability (e.g., "User can upload file" → `user-can-upload-file`). Include only Success Criteria items that require buildable work (e.g., load-testing infrastructure, security audit tooling), and exclude post-launch outcome metrics and business KPIs (e.g., "Reduce support tickets by 50%"). +- **User story/action inventory**: Discrete user actions with acceptance criteria +- **Task coverage mapping**: Map each task to one or more requirements or stories (inference by keyword / explicit reference patterns like IDs or key phrases) +- **Constitution rule set**: Extract principle names and MUST/SHOULD normative statements + +### 4. Detection Passes (Token-Efficient Analysis) + +Focus on high-signal findings. Limit to 50 findings total; aggregate remainder in overflow summary. + +#### A. Duplication Detection + +- Identify near-duplicate requirements +- Mark lower-quality phrasing for consolidation + +#### B. Ambiguity Detection + +- Flag vague adjectives (fast, scalable, secure, intuitive, robust) lacking measurable criteria +- Flag unresolved placeholders (TODO, TKTK, ???, ``, etc.) + +#### C. Underspecification + +- Requirements with verbs but missing object or measurable outcome +- User stories missing acceptance criteria alignment +- Tasks referencing files or components not defined in spec/plan + +#### D. Constitution Alignment + +- Any requirement or plan element conflicting with a MUST principle +- Missing mandated sections or quality gates from constitution + +#### E. Coverage Gaps + +- Requirements with zero associated tasks +- Tasks with no mapped requirement/story +- Success Criteria requiring buildable work (performance, security, availability) not reflected in tasks + +#### F. Inconsistency + +- Terminology drift (same concept named differently across files) +- Data entities referenced in plan but absent in spec (or vice versa) +- Task ordering contradictions (e.g., integration tasks before foundational setup tasks without dependency note) +- Conflicting requirements (e.g., one requires Next.js while other specifies Vue) + +### 5. Severity Assignment + +Use this heuristic to prioritize findings: + +- **CRITICAL**: Violates constitution MUST, missing core spec artifact, or requirement with zero coverage that blocks baseline functionality +- **HIGH**: Duplicate or conflicting requirement, ambiguous security/performance attribute, untestable acceptance criterion +- **MEDIUM**: Terminology drift, missing non-functional task coverage, underspecified edge case +- **LOW**: Style/wording improvements, minor redundancy not affecting execution order + +### 6. Produce Compact Analysis Report + +Output a Markdown report (no file writes) with the following structure: + +## Specification Analysis Report + +| ID | Category | Severity | Location(s) | Summary | Recommendation | +|----|----------|----------|-------------|---------|----------------| +| A1 | Duplication | HIGH | spec.md:L120-134 | Two similar requirements ... | Merge phrasing; keep clearer version | + +(Add one row per finding; generate stable IDs prefixed by category initial.) + +**Coverage Summary Table:** + +| Requirement Key | Has Task? | Task IDs | Notes | +|-----------------|-----------|----------|-------| + +**Constitution Alignment Issues:** (if any) + +**Unmapped Tasks:** (if any) + +**Metrics:** + +- Total Requirements +- Total Tasks +- Coverage % (requirements with >=1 task) +- Ambiguity Count +- Duplication Count +- Critical Issues Count + +### 7. Provide Next Actions + +At end of report, output a concise Next Actions block: + +- If CRITICAL issues exist: Recommend resolving before `/speckit.implement` +- If only LOW/MEDIUM: User may proceed, but provide improvement suggestions +- Provide explicit command suggestions: e.g., "Run /speckit.specify with refinement", "Run /speckit.plan to adjust architecture", "Manually edit tasks.md to add coverage for 'performance-metrics'" + +### 8. Offer Remediation + +Ask the user: "Would you like me to suggest concrete remediation edits for the top N issues?" (Do NOT apply them automatically.) + +### 9. Check for extension hooks + +After reporting, check if `.specify/extensions.yml` exists in the project root. +- If it exists, read it and look for entries under the `hooks.after_analyze` key +- If the YAML cannot be parsed or is invalid, skip hook checking silently and continue normally +- Filter out hooks where `enabled` is explicitly `false`. Treat hooks without an `enabled` field as enabled by default. +- For each remaining hook, do **not** attempt to interpret or evaluate hook `condition` expressions: + - If the hook has no `condition` field, or it is null/empty, treat the hook as executable + - If the hook defines a non-empty `condition`, skip the hook and leave condition evaluation to the HookExecutor implementation +- For each executable hook, output the following based on its `optional` flag: + - **Optional hook** (`optional: true`): + ``` + ## Extension Hooks + + **Optional Hook**: {extension} + Command: `/{command}` + Description: {description} + + Prompt: {prompt} + To execute: `/{command}` + ``` + - **Mandatory hook** (`optional: false`): + ``` + ## Extension Hooks + + **Automatic Hook**: {extension} + Executing: `/{command}` + EXECUTE_COMMAND: {command} + ``` +- If no hooks are registered or `.specify/extensions.yml` does not exist, skip silently + +## Operating Principles + +### Context Efficiency + +- **Minimal high-signal tokens**: Focus on actionable findings, not exhaustive documentation +- **Progressive disclosure**: Load artifacts incrementally; don't dump all content into analysis +- **Token-efficient output**: Limit findings table to 50 rows; summarize overflow +- **Deterministic results**: Rerunning without changes should produce consistent IDs and counts + +### Analysis Guidelines + +- **NEVER modify files** (this is read-only analysis) +- **NEVER hallucinate missing sections** (if absent, report them accurately) +- **Prioritize constitution violations** (these are always CRITICAL) +- **Use examples over exhaustive rules** (cite specific instances, not generic patterns) +- **Report zero issues gracefully** (emit success report with coverage statistics) + +## Context + +$ARGUMENTS diff --git a/.github/agents/speckit.brownfield.bootstrap.agent.md b/.github/agents/speckit.brownfield.bootstrap.agent.md new file mode 100644 index 0000000000..f5949cbf47 --- /dev/null +++ b/.github/agents/speckit.brownfield.bootstrap.agent.md @@ -0,0 +1,113 @@ +--- +description: Generate spec-kit configuration tailored to the existing codebase +--- + + + + +# Bootstrap Spec-Kit + +Generate a customized spec-kit configuration for an existing codebase. Uses the project profile from `/speckit.brownfield.scan` (or performs a scan if none exists) to create a constitution, templates, and agent configuration that match the project's actual architecture, tech stack, and conventions. + +## User Input + +```text +$ARGUMENTS +``` + +You **MUST** consider the user input before proceeding (if not empty). The user may specify preferences (e.g., "strict TDD", "minimal constitution"), a target directory for a monorepo module, or request specific template customizations. + +## Prerequisites + +1. Verify the current directory is a git repository +2. Verify a spec-kit project exists by checking for `.specify/` directory (run `specify init` first if missing) +3. Check if a project profile exists from a previous scan — if not, run a scan first + +## Outline + +1. **Load or generate project profile**: Check if `/speckit.brownfield.scan` has been run: + - If a project profile exists, use it + - If not, perform an inline scan to gather tech stack, architecture, and conventions + - Confirm the profile with the user before proceeding + +2. **Generate constitution**: Create `.specify/memory/constitution.md` tailored to the project: + + The constitution **MUST** include: + - **Project identity**: Name, purpose, primary language(s), architecture pattern + - **Code boundaries**: Which directories contain which types of code (e.g., "frontend code lives in `client/`, backend in `server/`") + - **Naming conventions**: File naming, variable naming, branch naming as detected + - **Testing requirements**: Test framework, test location, coverage expectations + - **Dependency rules**: How modules depend on each other, what imports are allowed + - **Quality gates**: Linting, formatting, CI checks that must pass + + The constitution **MUST NOT**: + - Override existing project standards without user confirmation + - Invent conventions that don't exist in the codebase + - Include generic boilerplate unrelated to the actual project + +3. **Customize spec template**: Modify `.specify/templates/spec-template.md` to reflect the project: + - Add project-specific sections (e.g., "Database Migrations" for projects with ORMs) + - Include architecture-aware requirements (e.g., "Frontend Requirements" and "API Requirements" for full-stack projects) + - Reference actual module paths instead of generic placeholders + +4. **Customize plan template**: Modify `.specify/templates/plan-template.md` to reflect the project: + - Include module-aware implementation sections (e.g., separate phases for frontend/backend) + - Reference actual test frameworks and build tools + - Include project-specific complexity factors + +5. **Customize tasks template**: Modify `.specify/templates/tasks-template.md` to reflect the project: + - Task phases should map to the project's actual module structure + - Include project-specific setup tasks (e.g., database migration, dependency install) + - Reference actual test commands (e.g., `npm test`, `pytest`, `go test ./...`) + +6. **Generate AGENTS.md** (if multi-module): For monorepos and multi-module projects: + - Define agent boundaries per module + - Specify which agent owns which directories + - Set up inter-agent communication rules + +7. **Present changes**: Show the user what will be created or modified: + + ```markdown + # Bootstrap Plan + + | File | Action | Description | + |------|--------|-------------| + | `.specify/memory/constitution.md` | Create | Project-specific constitution with detected conventions | + | `.specify/templates/spec-template.md` | Modify | Add project-specific sections (Database Migrations, API Contract) | + | `.specify/templates/plan-template.md` | Modify | Add module-aware phases (frontend, backend, shared) | + | `.specify/templates/tasks-template.md` | Modify | Add actual test commands and build steps | + | `AGENTS.md` | Create | Agent boundaries for frontend and backend modules | + + Proceed with bootstrap? (confirm before writing) + ``` + +8. **Execute bootstrap**: After user confirmation, write all files. + +9. **Report**: + + ```markdown + # Bootstrap Complete + + | Artifact | Status | + |----------|--------| + | Constitution | ✅ Created — 12 rules from detected conventions | + | Spec template | ✅ Customized — added Database Migrations, API Contract sections | + | Plan template | ✅ Customized — frontend/backend phase split | + | Tasks template | ✅ Customized — actual test commands included | + | AGENTS.md | ✅ Created — 2 agents (frontend, backend) | + + ## Next Steps + - Review `.specify/memory/constitution.md` and adjust any rules + - Run `/speckit.brownfield.validate` to verify configuration matches project + - Run `/speckit.brownfield.migrate` to reverse-engineer specs for existing features + - Start new features with `/speckit.specify` — templates are now project-aware + ``` + +## Rules + +- **Always confirm before writing** — show the bootstrap plan and wait for approval +- **Never overwrite without asking** — if constitution or templates already exist, show a diff and ask +- **Derive from reality** — every constitution rule must trace to something detected in the codebase +- **No invented conventions** — if the project has no consistent pattern for something, say so instead of guessing +- **Respect existing spec-kit setup** — if `.specify/` already has customizations, merge rather than replace +- **Module-aware** — for monorepos, generate configuration that respects module boundaries \ No newline at end of file diff --git a/.github/agents/speckit.brownfield.migrate.agent.md b/.github/agents/speckit.brownfield.migrate.agent.md new file mode 100644 index 0000000000..3a6bfa6791 --- /dev/null +++ b/.github/agents/speckit.brownfield.migrate.agent.md @@ -0,0 +1,128 @@ +--- +description: Incrementally adopt SDD for existing features with reverse-engineered + specs +--- + + + + +# Migrate Existing Features + +Reverse-engineer spec-kit artifacts (spec.md, plan.md, tasks.md) for features that were built before spec-kit was adopted. This brings existing work into the SDD workflow so teams can track, refine, and extend features using spec-kit commands. + +## User Input + +```text +$ARGUMENTS +``` + +You **MUST** consider the user input before proceeding (if not empty). The user may specify a feature or module to migrate (e.g., "auth system", "payments module"), a branch name, or "all" to migrate everything. + +## Prerequisites + +1. Verify a spec-kit project exists by checking for `.specify/` directory +2. Verify git is available and the project is a git repository +3. Verify the project has existing source code to migrate (not an empty project) +4. Verify constitution exists (recommend running `/speckit.brownfield.bootstrap` first if missing) + +## Outline + +1. **Identify migration targets**: Determine what to migrate based on user input: + + | Input | Action | + |-------|--------| + | Specific feature name | Locate the feature in the codebase by searching for related files, modules, or directories | + | Specific branch name | Analyze the branch's commits and changed files to identify the feature scope | + | Module path | Treat the entire module as a single feature to migrate | + | `all` | List all identifiable features and let the user select which to migrate | + | No input | Show a list of detected features and ask the user to pick one | + +2. **Detect feature boundaries**: For each migration target, determine its scope: + - **Files**: Which source files implement this feature + - **Tests**: Which test files cover this feature + - **Dependencies**: What other modules or services this feature depends on + - **API surface**: Endpoints, functions, or interfaces exposed by this feature + - **Database**: Migrations, models, or schema changes related to this feature + +3. **Reverse-engineer spec.md**: Analyze the code to reconstruct what the feature does: + - **User scenarios**: Infer from test cases, route handlers, and UI components + - **Requirements**: Extract from code behavior, validation rules, and error handling + - **Success criteria**: Derive from test assertions and acceptance patterns + - **Assumptions**: Note any hardcoded values, environment dependencies, or implicit requirements + - Mark the spec as `status: migrated` to distinguish from specs created through the normal workflow + +4. **Reverse-engineer plan.md**: Reconstruct the implementation approach: + - **Technical context**: Actual frameworks, libraries, and patterns used + - **Project structure**: Where the feature's code lives in the project + - **Complexity assessment**: Based on file count, line count, and dependency depth + +5. **Reverse-engineer tasks.md**: Create a task list reflecting what was actually built: + - Each major component or module becomes a task group + - Mark all tasks as `[x]` (completed) since the feature already exists + - Include test tasks based on actual test files found + - Note any gaps: code without tests, features without error handling + +6. **Create feature branch and artifacts**: For each migrated feature: + - Create a feature directory: `specs/{feature-name}/` + - Write `spec.md`, `plan.md`, and `tasks.md` into the feature directory + - Do **not** create a git branch — the feature already exists on its branch or main + +7. **Present migration plan**: Show what will be created before writing: + + ```markdown + # Migration Plan: User Authentication + + ## Detected Scope + | Category | Files | Lines | + |----------|-------|-------| + | Source | 8 files | ~420 lines | + | Tests | 3 files | ~180 lines | + | Migrations | 2 files | ~45 lines | + + ## Artifacts to Generate + | File | Content | + |------|---------| + | `specs/user-auth/spec.md` | 4 user scenarios, 12 requirements, 6 success criteria | + | `specs/user-auth/plan.md` | 3 implementation phases, 8 technical decisions | + | `specs/user-auth/tasks.md` | 14 tasks (all completed), 2 gaps identified | + + ## Gaps Found + - ⚠️ No error handling tests for expired tokens + - ⚠️ No rate limiting on login endpoint + + Proceed with migration? + ``` + +8. **Execute migration**: After user confirmation, write all artifacts. + +9. **Report**: + + ```markdown + # Migration Complete: User Authentication + + | Artifact | Status | + |----------|--------| + | spec.md | ✅ Created — 4 scenarios, 12 requirements | + | plan.md | ✅ Created — 3 phases | + | tasks.md | ✅ Created — 14/14 tasks complete | + + ## Identified Gaps + 1. No error handling tests for expired tokens → consider `/speckit.specify` for a follow-up feature + 2. No rate limiting on login endpoint → consider `/speckit.bugfix.report` to track + + ## Next Steps + - Review generated artifacts in `specs/user-auth/` + - Use `/speckit.refine.update` to adjust any inaccurate specs + - Use `/speckit.specify` for new features — they'll follow the same SDD workflow + - Run `/speckit.brownfield.migrate` again for additional features + ``` + +## Rules + +- **Always confirm before writing** — show the migration plan and wait for user approval +- **Honest assessment** — if the code is unclear or poorly documented, say so in the spec rather than inventing explanations +- **Mark as migrated** — all migrated specs must include `status: migrated` to distinguish from fresh specs +- **Identify gaps** — actively look for missing tests, error handling, or documentation and report them +- **Non-destructive** — never modify existing source code, only create spec artifacts +- **One feature at a time** — for "all" input, migrate features sequentially with confirmation between each +- **Respect constitution** — generated artifacts must follow the project's constitution rules \ No newline at end of file diff --git a/.github/agents/speckit.brownfield.scan.agent.md b/.github/agents/speckit.brownfield.scan.agent.md new file mode 100644 index 0000000000..3fbeefe548 --- /dev/null +++ b/.github/agents/speckit.brownfield.scan.agent.md @@ -0,0 +1,121 @@ +--- +description: Auto-discover project structure, tech stack, frameworks, and architecture + patterns +--- + + + + +# Scan Project + +Analyze an existing codebase to discover its technology stack, architecture patterns, module structure, and coding conventions. This produces a project profile that the bootstrap command uses to generate tailored spec-kit configuration. + +## User Input + +```text +$ARGUMENTS +``` + +You **MUST** consider the user input before proceeding (if not empty). The user may specify a subdirectory to scan (e.g., "backend/"), a focus area (e.g., "only frontend"), or request a specific depth of analysis. + +## Prerequisites + +1. Verify the current directory is a git repository +2. Verify this is an existing project with source code (not an empty repo) + +## Outline + +1. **Detect tech stack**: Identify languages, frameworks, and tools by scanning: + + | Signal | What to Check | + |--------|--------------| + | **Languages** | File extensions (`.py`, `.ts`, `.go`, `.java`, `.rs`, etc.) and their relative proportions | + | **Package managers** | `package.json`, `requirements.txt`, `pyproject.toml`, `go.mod`, `Cargo.toml`, `pom.xml`, `build.gradle` | + | **Frameworks** | Dependencies in package files (React, Django, Spring, Express, Rails, etc.) | + | **Build tools** | `Makefile`, `webpack.config.js`, `vite.config.ts`, `Dockerfile`, `docker-compose.yml` | + | **CI/CD** | `.github/workflows/`, `.gitlab-ci.yml`, `.circleci/`, `Jenkinsfile` | + | **Testing** | Test directories, test frameworks in dependencies (`jest`, `pytest`, `go test`, `JUnit`) | + +2. **Analyze architecture**: Identify the project's structural patterns: + + | Pattern | Indicators | + |---------|-----------| + | **Monolith** | Single source tree, one entry point, shared database config | + | **Monorepo** | Multiple `package.json`/`go.mod` files, workspace config, `packages/` or `apps/` directories | + | **Microservices** | Multiple Dockerfiles, service directories, API gateway config | + | **Frontend + Backend** | Separate `client/`/`server/` or `frontend/`/`backend/` directories | + | **Library/Package** | `setup.py`, `lib/` directory, published package config | + | **MVC** | `models/`, `views/`, `controllers/` directories | + | **Layered** | `domain/`, `application/`, `infrastructure/`, `presentation/` directories | + +3. **Map module structure**: For monorepos and multi-module projects: + - Identify each module/package/service and its purpose + - Detect inter-module dependencies (imports, shared types) + - Note module boundaries (what code belongs where) + - Identify shared libraries or utilities + +4. **Extract conventions**: Detect existing coding patterns: + - **Naming**: File naming (camelCase, kebab-case, snake_case), directory naming + - **Branching**: Existing branch names and patterns from `git branch -a` + - **Commit style**: Recent commit message patterns from `git log --oneline -20` + - **Testing**: Test file location (`__tests__/`, `*_test.go`, `test_*.py`), test naming + - **Documentation**: README structure, inline docs, API docs + +5. **Detect existing governance**: Check for files that indicate existing project standards: + - `CONTRIBUTING.md`, `ARCHITECTURE.md`, `ADR/` (Architecture Decision Records) + - `.editorconfig`, linter configs (`.eslintrc`, `.flake8`, `rustfmt.toml`) + - `CLAUDE.md`, `AGENTS.md`, `.specify/` (existing spec-kit setup) + +6. **Output project profile**: + + ```markdown + # Project Profile + + ## Tech Stack + | Category | Detected | + |----------|----------| + | **Primary language** | TypeScript (68%), Python (32%) | + | **Frontend** | React 18, Vite, TailwindCSS | + | **Backend** | FastAPI, SQLAlchemy, PostgreSQL | + | **Testing** | Jest (frontend), pytest (backend) | + | **CI/CD** | GitHub Actions | + | **Package manager** | npm (frontend), pip (backend) | + + ## Architecture + - **Pattern**: Frontend + Backend (separated) + - **Frontend**: `client/` — React SPA + - **Backend**: `server/` — FastAPI REST API + - **Database**: PostgreSQL (via SQLAlchemy ORM) + + ## Module Map + | Module | Path | Purpose | Dependencies | + |--------|------|---------|-------------| + | Frontend | `client/` | React SPA | Backend API | + | Backend | `server/` | REST API | Database | + | Shared | `shared/` | Type definitions | — | + + ## Conventions + - **File naming**: kebab-case (frontend), snake_case (backend) + - **Branch pattern**: `feat/*`, `fix/*`, `chore/*` + - **Commit style**: Conventional Commits + - **Test location**: `__tests__/` (frontend), `tests/` (backend) + + ## Existing Governance + - ✅ CONTRIBUTING.md + - ✅ .eslintrc.json + - ❌ ARCHITECTURE.md + - ❌ .specify/ (no spec-kit setup) + + ## Recommendations + - Run `/speckit.brownfield.bootstrap` to generate tailored spec-kit configuration + - Constitution should enforce: kebab-case files (frontend), snake_case (backend) + - Feature specs should map to the frontend/backend split + ``` + +## Rules + +- **Read-only** — this command never modifies any files +- **Respect .gitignore** — never scan `node_modules/`, `vendor/`, `dist/`, `.venv/`, or other ignored directories +- **Proportional analysis** — report language percentages based on actual file counts or line counts +- **No assumptions** — only report what is actually detected in the codebase +- **Handle empty results** — if a category has nothing detected, say "Not detected" rather than guessing \ No newline at end of file diff --git a/.github/agents/speckit.brownfield.validate.agent.md b/.github/agents/speckit.brownfield.validate.agent.md new file mode 100644 index 0000000000..ec39e2bf5a --- /dev/null +++ b/.github/agents/speckit.brownfield.validate.agent.md @@ -0,0 +1,94 @@ +--- +description: Verify bootstrap output matches actual project structure and conventions +--- + + + + +# Validate Bootstrap + +Verify that the spec-kit configuration generated by `/speckit.brownfield.bootstrap` accurately reflects the actual project structure, conventions, and architecture. Reports mismatches and suggests corrections. + +## User Input + +```text +$ARGUMENTS +``` + +You **MUST** consider the user input before proceeding (if not empty). The user may specify a focus area (e.g., "only constitution", "only templates") or request verbose output. + +## Prerequisites + +1. Verify a spec-kit project exists by checking for `.specify/` directory +2. Verify git is available and the project is a git repository +3. Verify at least one bootstrap artifact exists (constitution, customized templates, or AGENTS.md) + +## Outline + +1. **Validate constitution**: Check `.specify/memory/constitution.md` against the actual codebase: + + | Check | How | + |-------|-----| + | **Language references** | Verify mentioned languages actually exist in the codebase | + | **Directory references** | Verify all referenced paths (`client/`, `server/`, etc.) exist | + | **Framework references** | Verify mentioned frameworks are in dependency files | + | **Naming conventions** | Sample 20 files and check if naming rules match reality | + | **Test location** | Verify test directories mentioned in constitution exist | + | **Branch pattern** | Check if branch naming rules match actual branches in `git branch -a` | + +2. **Validate templates**: Check customized spec/plan/tasks templates: + + | Check | How | + |-------|-----| + | **Module references** | Verify template sections reference actual modules/directories | + | **Test commands** | Verify test commands in tasks template actually work | + | **Build commands** | Verify build commands reference real scripts from package files | + | **Section relevance** | Flag template sections that reference non-existent project aspects | + +3. **Validate AGENTS.md** (if exists): Check agent configuration: + + | Check | How | + |-------|-----| + | **Directory ownership** | Verify each agent's directories exist | + | **No overlaps** | Check that no directory is owned by multiple agents | + | **No orphans** | Check that all source directories are covered by at least one agent | + +4. **Detect drift**: Check if the project has changed since bootstrap: + - New directories or modules added since constitution was generated + - Dependencies added or removed since bootstrap + - New branch patterns that don't match constitution rules + +5. **Output validation report**: + + ```markdown + # Validation Report + + ## Constitution + | Rule | Status | Detail | + |------|--------|--------| + | Primary language: TypeScript | ✅ Pass | 68% of source files | + | Frontend in `client/` | ✅ Pass | Directory exists, contains React code | + | Backend in `server/` | ✅ Pass | Directory exists, contains FastAPI code | + | Test location: `__tests__/` | ⚠️ Drift | Also found tests in `tests/` (not mentioned) | + | Branch pattern: `feat/*` | ✅ Pass | 8/10 recent branches match | + + ## Templates + | Template | Status | Detail | + |----------|--------|--------| + | Spec template | ✅ Pass | All custom sections map to real project aspects | + | Plan template | ⚠️ Drift | References `shared/` module — directory renamed to `common/` | + | Tasks template | ✅ Pass | Test commands verified | + + ## Summary + - **Checks passed**: 9/11 + - **Drift detected**: 2 items + - **Action needed**: Update plan template (`shared/` → `common/`), add `tests/` to constitution + ``` + +## Rules + +- **Read-only** — this command never modifies any files +- **Evidence-based** — every pass/fail must cite specific files or directories as evidence +- **Actionable output** — for every failure or drift, suggest the specific fix +- **Non-blocking** — drift warnings don't mean the configuration is broken, just that it could be improved +- **Respect .gitignore** — never scan ignored directories when validating \ No newline at end of file diff --git a/.github/agents/speckit.checklist.agent.md b/.github/agents/speckit.checklist.agent.md new file mode 100644 index 0000000000..93ec785da7 --- /dev/null +++ b/.github/agents/speckit.checklist.agent.md @@ -0,0 +1,361 @@ +--- +description: Generate a custom checklist for the current feature based on user requirements. +--- + +## Checklist Purpose: "Unit Tests for English" + +**CRITICAL CONCEPT**: Checklists are **UNIT TESTS FOR REQUIREMENTS WRITING** - they validate the quality, clarity, and completeness of requirements in a given domain. + +**NOT for verification/testing**: + +- ❌ NOT "Verify the button clicks correctly" +- ❌ NOT "Test error handling works" +- ❌ NOT "Confirm the API returns 200" +- ❌ NOT checking if code/implementation matches the spec + +**FOR requirements quality validation**: + +- ✅ "Are visual hierarchy requirements defined for all card types?" (completeness) +- ✅ "Is 'prominent display' quantified with specific sizing/positioning?" (clarity) +- ✅ "Are hover state requirements consistent across all interactive elements?" (consistency) +- ✅ "Are accessibility requirements defined for keyboard navigation?" (coverage) +- ✅ "Does the spec define what happens when logo image fails to load?" (edge cases) + +**Metaphor**: If your spec is code written in English, the checklist is its unit test suite. You're testing whether the requirements are well-written, complete, unambiguous, and ready for implementation - NOT whether the implementation works. + +## User Input + +```text +$ARGUMENTS +``` + +You **MUST** consider the user input before proceeding (if not empty). + +## Pre-Execution Checks + +**Check for extension hooks (before checklist generation)**: +- Check if `.specify/extensions.yml` exists in the project root. +- If it exists, read it and look for entries under the `hooks.before_checklist` key +- If the YAML cannot be parsed or is invalid, skip hook checking silently and continue normally +- Filter out hooks where `enabled` is explicitly `false`. Treat hooks without an `enabled` field as enabled by default. +- For each remaining hook, do **not** attempt to interpret or evaluate hook `condition` expressions: + - If the hook has no `condition` field, or it is null/empty, treat the hook as executable + - If the hook defines a non-empty `condition`, skip the hook and leave condition evaluation to the HookExecutor implementation +- For each executable hook, output the following based on its `optional` flag: + - **Optional hook** (`optional: true`): + ``` + ## Extension Hooks + + **Optional Pre-Hook**: {extension} + Command: `/{command}` + Description: {description} + + Prompt: {prompt} + To execute: `/{command}` + ``` + - **Mandatory hook** (`optional: false`): + ``` + ## Extension Hooks + + **Automatic Pre-Hook**: {extension} + Executing: `/{command}` + EXECUTE_COMMAND: {command} + + Wait for the result of the hook command before proceeding to the Execution Steps. + ``` +- If no hooks are registered or `.specify/extensions.yml` does not exist, skip silently + +## Execution Steps + +1. **Setup**: Run `.specify/scripts/bash/check-prerequisites.sh --json` from repo root and parse JSON for FEATURE_DIR and AVAILABLE_DOCS list. + - All file paths must be absolute. + - For single quotes in args like "I'm Groot", use escape syntax: e.g 'I'\''m Groot' (or double-quote if possible: "I'm Groot"). + +2. **Clarify intent (dynamic)**: Derive up to THREE initial contextual clarifying questions (no pre-baked catalog). They MUST: + - Be generated from the user's phrasing + extracted signals from spec/plan/tasks + - Only ask about information that materially changes checklist content + - Be skipped individually if already unambiguous in `$ARGUMENTS` + - Prefer precision over breadth + + Generation algorithm: + 1. Extract signals: feature domain keywords (e.g., auth, latency, UX, API), risk indicators ("critical", "must", "compliance"), stakeholder hints ("QA", "review", "security team"), and explicit deliverables ("a11y", "rollback", "contracts"). + 2. Cluster signals into candidate focus areas (max 4) ranked by relevance. + 3. Identify probable audience & timing (author, reviewer, QA, release) if not explicit. + 4. Detect missing dimensions: scope breadth, depth/rigor, risk emphasis, exclusion boundaries, measurable acceptance criteria. + 5. Formulate questions chosen from these archetypes: + - Scope refinement (e.g., "Should this include integration touchpoints with X and Y or stay limited to local module correctness?") + - Risk prioritization (e.g., "Which of these potential risk areas should receive mandatory gating checks?") + - Depth calibration (e.g., "Is this a lightweight pre-commit sanity list or a formal release gate?") + - Audience framing (e.g., "Will this be used by the author only or peers during PR review?") + - Boundary exclusion (e.g., "Should we explicitly exclude performance tuning items this round?") + - Scenario class gap (e.g., "No recovery flows detected—are rollback / partial failure paths in scope?") + + Question formatting rules: + - If presenting options, generate a compact table with columns: Option | Candidate | Why It Matters + - Limit to A–E options maximum; omit table if a free-form answer is clearer + - Never ask the user to restate what they already said + - Avoid speculative categories (no hallucination). If uncertain, ask explicitly: "Confirm whether X belongs in scope." + + Defaults when interaction impossible: + - Depth: Standard + - Audience: Reviewer (PR) if code-related; Author otherwise + - Focus: Top 2 relevance clusters + + Output the questions (label Q1/Q2/Q3). After answers: if ≥2 scenario classes (Alternate / Exception / Recovery / Non-Functional domain) remain unclear, you MAY ask up to TWO more targeted follow‑ups (Q4/Q5) with a one-line justification each (e.g., "Unresolved recovery path risk"). Do not exceed five total questions. Skip escalation if user explicitly declines more. + +3. **Understand user request**: Combine `$ARGUMENTS` + clarifying answers: + - Derive checklist theme (e.g., security, review, deploy, ux) + - Consolidate explicit must-have items mentioned by user + - Map focus selections to category scaffolding + - Infer any missing context from spec/plan/tasks (do NOT hallucinate) + +4. **Load feature context**: Read from FEATURE_DIR: + - spec.md: Feature requirements and scope + - plan.md (if exists): Technical details, dependencies + - tasks.md (if exists): Implementation tasks + + **Context Loading Strategy**: + - Load only necessary portions relevant to active focus areas (avoid full-file dumping) + - Prefer summarizing long sections into concise scenario/requirement bullets + - Use progressive disclosure: add follow-on retrieval only if gaps detected + - If source docs are large, generate interim summary items instead of embedding raw text + +5. **Generate checklist** - Create "Unit Tests for Requirements": + - Create `FEATURE_DIR/checklists/` directory if it doesn't exist + - Generate unique checklist filename: + - Use short, descriptive name based on domain (e.g., `ux.md`, `api.md`, `security.md`) + - Format: `[domain].md` + - File handling behavior: + - If file does NOT exist: Create new file and number items starting from CHK001 + - If file exists: Append new items to existing file, continuing from the last CHK ID (e.g., if last item is CHK015, start new items at CHK016) + - Never delete or replace existing checklist content - always preserve and append + + **CORE PRINCIPLE - Test the Requirements, Not the Implementation**: + Every checklist item MUST evaluate the REQUIREMENTS THEMSELVES for: + - **Completeness**: Are all necessary requirements present? + - **Clarity**: Are requirements unambiguous and specific? + - **Consistency**: Do requirements align with each other? + - **Measurability**: Can requirements be objectively verified? + - **Coverage**: Are all scenarios/edge cases addressed? + + **Category Structure** - Group items by requirement quality dimensions: + - **Requirement Completeness** (Are all necessary requirements documented?) + - **Requirement Clarity** (Are requirements specific and unambiguous?) + - **Requirement Consistency** (Do requirements align without conflicts?) + - **Acceptance Criteria Quality** (Are success criteria measurable?) + - **Scenario Coverage** (Are all flows/cases addressed?) + - **Edge Case Coverage** (Are boundary conditions defined?) + - **Non-Functional Requirements** (Performance, Security, Accessibility, etc. - are they specified?) + - **Dependencies & Assumptions** (Are they documented and validated?) + - **Ambiguities & Conflicts** (What needs clarification?) + + **HOW TO WRITE CHECKLIST ITEMS - "Unit Tests for English"**: + + ❌ **WRONG** (Testing implementation): + - "Verify landing page displays 3 episode cards" + - "Test hover states work on desktop" + - "Confirm logo click navigates home" + + ✅ **CORRECT** (Testing requirements quality): + - "Are the exact number and layout of featured episodes specified?" [Completeness] + - "Is 'prominent display' quantified with specific sizing/positioning?" [Clarity] + - "Are hover state requirements consistent across all interactive elements?" [Consistency] + - "Are keyboard navigation requirements defined for all interactive UI?" [Coverage] + - "Is the fallback behavior specified when logo image fails to load?" [Edge Cases] + - "Are loading states defined for asynchronous episode data?" [Completeness] + - "Does the spec define visual hierarchy for competing UI elements?" [Clarity] + + **ITEM STRUCTURE**: + Each item should follow this pattern: + - Question format asking about requirement quality + - Focus on what's WRITTEN (or not written) in the spec/plan + - Include quality dimension in brackets [Completeness/Clarity/Consistency/etc.] + - Reference spec section `[Spec §X.Y]` when checking existing requirements + - Use `[Gap]` marker when checking for missing requirements + + **EXAMPLES BY QUALITY DIMENSION**: + + Completeness: + - "Are error handling requirements defined for all API failure modes? [Gap]" + - "Are accessibility requirements specified for all interactive elements? [Completeness]" + - "Are mobile breakpoint requirements defined for responsive layouts? [Gap]" + + Clarity: + - "Is 'fast loading' quantified with specific timing thresholds? [Clarity, Spec §NFR-2]" + - "Are 'related episodes' selection criteria explicitly defined? [Clarity, Spec §FR-5]" + - "Is 'prominent' defined with measurable visual properties? [Ambiguity, Spec §FR-4]" + + Consistency: + - "Do navigation requirements align across all pages? [Consistency, Spec §FR-10]" + - "Are card component requirements consistent between landing and detail pages? [Consistency]" + + Coverage: + - "Are requirements defined for zero-state scenarios (no episodes)? [Coverage, Edge Case]" + - "Are concurrent user interaction scenarios addressed? [Coverage, Gap]" + - "Are requirements specified for partial data loading failures? [Coverage, Exception Flow]" + + Measurability: + - "Are visual hierarchy requirements measurable/testable? [Acceptance Criteria, Spec §FR-1]" + - "Can 'balanced visual weight' be objectively verified? [Measurability, Spec §FR-2]" + + **Scenario Classification & Coverage** (Requirements Quality Focus): + - Check if requirements exist for: Primary, Alternate, Exception/Error, Recovery, Non-Functional scenarios + - For each scenario class, ask: "Are [scenario type] requirements complete, clear, and consistent?" + - If scenario class missing: "Are [scenario type] requirements intentionally excluded or missing? [Gap]" + - Include resilience/rollback when state mutation occurs: "Are rollback requirements defined for migration failures? [Gap]" + + **Traceability Requirements**: + - MINIMUM: ≥80% of items MUST include at least one traceability reference + - Each item should reference: spec section `[Spec §X.Y]`, or use markers: `[Gap]`, `[Ambiguity]`, `[Conflict]`, `[Assumption]` + - If no ID system exists: "Is a requirement & acceptance criteria ID scheme established? [Traceability]" + + **Surface & Resolve Issues** (Requirements Quality Problems): + Ask questions about the requirements themselves: + - Ambiguities: "Is the term 'fast' quantified with specific metrics? [Ambiguity, Spec §NFR-1]" + - Conflicts: "Do navigation requirements conflict between §FR-10 and §FR-10a? [Conflict]" + - Assumptions: "Is the assumption of 'always available podcast API' validated? [Assumption]" + - Dependencies: "Are external podcast API requirements documented? [Dependency, Gap]" + - Missing definitions: "Is 'visual hierarchy' defined with measurable criteria? [Gap]" + + **Content Consolidation**: + - Soft cap: If raw candidate items > 40, prioritize by risk/impact + - Merge near-duplicates checking the same requirement aspect + - If >5 low-impact edge cases, create one item: "Are edge cases X, Y, Z addressed in requirements? [Coverage]" + + **🚫 ABSOLUTELY PROHIBITED** - These make it an implementation test, not a requirements test: + - ❌ Any item starting with "Verify", "Test", "Confirm", "Check" + implementation behavior + - ❌ References to code execution, user actions, system behavior + - ❌ "Displays correctly", "works properly", "functions as expected" + - ❌ "Click", "navigate", "render", "load", "execute" + - ❌ Test cases, test plans, QA procedures + - ❌ Implementation details (frameworks, APIs, algorithms) + + **✅ REQUIRED PATTERNS** - These test requirements quality: + - ✅ "Are [requirement type] defined/specified/documented for [scenario]?" + - ✅ "Is [vague term] quantified/clarified with specific criteria?" + - ✅ "Are requirements consistent between [section A] and [section B]?" + - ✅ "Can [requirement] be objectively measured/verified?" + - ✅ "Are [edge cases/scenarios] addressed in requirements?" + - ✅ "Does the spec define [missing aspect]?" + +6. **Structure Reference**: Generate the checklist following the canonical template in `.specify/templates/checklist-template.md` for title, meta section, category headings, and ID formatting. If template is unavailable, use: H1 title, purpose/created meta lines, `##` category sections containing `- [ ] CHK### ` lines with globally incrementing IDs starting at CHK001. + +7. **Report**: Output full path to checklist file, item count, and summarize whether the run created a new file or appended to an existing one. Summarize: + - Focus areas selected + - Depth level + - Actor/timing + - Any explicit user-specified must-have items incorporated + +**Important**: Each `/speckit.checklist` command invocation uses a short, descriptive checklist filename and either creates a new file or appends to an existing one. This allows: + +- Multiple checklists of different types (e.g., `ux.md`, `test.md`, `security.md`) +- Simple, memorable filenames that indicate checklist purpose +- Easy identification and navigation in the `checklists/` folder + +To avoid clutter, use descriptive types and clean up obsolete checklists when done. + +## Example Checklist Types & Sample Items + +**UX Requirements Quality:** `ux.md` + +Sample items (testing the requirements, NOT the implementation): + +- "Are visual hierarchy requirements defined with measurable criteria? [Clarity, Spec §FR-1]" +- "Is the number and positioning of UI elements explicitly specified? [Completeness, Spec §FR-1]" +- "Are interaction state requirements (hover, focus, active) consistently defined? [Consistency]" +- "Are accessibility requirements specified for all interactive elements? [Coverage, Gap]" +- "Is fallback behavior defined when images fail to load? [Edge Case, Gap]" +- "Can 'prominent display' be objectively measured? [Measurability, Spec §FR-4]" + +**API Requirements Quality:** `api.md` + +Sample items: + +- "Are error response formats specified for all failure scenarios? [Completeness]" +- "Are rate limiting requirements quantified with specific thresholds? [Clarity]" +- "Are authentication requirements consistent across all endpoints? [Consistency]" +- "Are retry/timeout requirements defined for external dependencies? [Coverage, Gap]" +- "Is versioning strategy documented in requirements? [Gap]" + +**Performance Requirements Quality:** `performance.md` + +Sample items: + +- "Are performance requirements quantified with specific metrics? [Clarity]" +- "Are performance targets defined for all critical user journeys? [Coverage]" +- "Are performance requirements under different load conditions specified? [Completeness]" +- "Can performance requirements be objectively measured? [Measurability]" +- "Are degradation requirements defined for high-load scenarios? [Edge Case, Gap]" + +**Security Requirements Quality:** `security.md` + +Sample items: + +- "Are authentication requirements specified for all protected resources? [Coverage]" +- "Are data protection requirements defined for sensitive information? [Completeness]" +- "Is the threat model documented and requirements aligned to it? [Traceability]" +- "Are security requirements consistent with compliance obligations? [Consistency]" +- "Are security failure/breach response requirements defined? [Gap, Exception Flow]" + +## Anti-Examples: What NOT To Do + +**❌ WRONG - These test implementation, not requirements:** + +```markdown +- [ ] CHK001 - Verify landing page displays 3 episode cards [Spec §FR-001] +- [ ] CHK002 - Test hover states work correctly on desktop [Spec §FR-003] +- [ ] CHK003 - Confirm logo click navigates to home page [Spec §FR-010] +- [ ] CHK004 - Check that related episodes section shows 3-5 items [Spec §FR-005] +``` + +**✅ CORRECT - These test requirements quality:** + +```markdown +- [ ] CHK001 - Are the number and layout of featured episodes explicitly specified? [Completeness, Spec §FR-001] +- [ ] CHK002 - Are hover state requirements consistently defined for all interactive elements? [Consistency, Spec §FR-003] +- [ ] CHK003 - Are navigation requirements clear for all clickable brand elements? [Clarity, Spec §FR-010] +- [ ] CHK004 - Is the selection criteria for related episodes documented? [Gap, Spec §FR-005] +- [ ] CHK005 - Are loading state requirements defined for asynchronous episode data? [Gap] +- [ ] CHK006 - Can "visual hierarchy" requirements be objectively measured? [Measurability, Spec §FR-001] +``` + +**Key Differences:** + +- Wrong: Tests if the system works correctly +- Correct: Tests if the requirements are written correctly +- Wrong: Verification of behavior +- Correct: Validation of requirement quality +- Wrong: "Does it do X?" +- Correct: "Is X clearly specified?" + +## Post-Execution Checks + +**Check for extension hooks (after checklist generation)**: +Check if `.specify/extensions.yml` exists in the project root. +- If it exists, read it and look for entries under the `hooks.after_checklist` key +- If the YAML cannot be parsed or is invalid, skip hook checking silently and continue normally +- Filter out hooks where `enabled` is explicitly `false`. Treat hooks without an `enabled` field as enabled by default. +- For each remaining hook, do **not** attempt to interpret or evaluate hook `condition` expressions: + - If the hook has no `condition` field, or it is null/empty, treat the hook as executable + - If the hook defines a non-empty `condition`, skip the hook and leave condition evaluation to the HookExecutor implementation +- For each executable hook, output the following based on its `optional` flag: + - **Optional hook** (`optional: true`): + ``` + ## Extension Hooks + + **Optional Hook**: {extension} + Command: `/{command}` + Description: {description} + + Prompt: {prompt} + To execute: `/{command}` + ``` + - **Mandatory hook** (`optional: false`): + ``` + ## Extension Hooks + + **Automatic Hook**: {extension} + Executing: `/{command}` + EXECUTE_COMMAND: {command} + ``` +- If no hooks are registered or `.specify/extensions.yml` does not exist, skip silently diff --git a/.github/agents/speckit.clarify.agent.md b/.github/agents/speckit.clarify.agent.md new file mode 100644 index 0000000000..9de13e17e5 --- /dev/null +++ b/.github/agents/speckit.clarify.agent.md @@ -0,0 +1,247 @@ +--- +description: Identify underspecified areas in the current feature spec by asking up to 5 highly targeted clarification questions and encoding answers back into the spec. +handoffs: + - label: Build Technical Plan + agent: speckit.plan + prompt: Create a plan for the spec. I am building with... +--- + +## User Input + +```text +$ARGUMENTS +``` + +You **MUST** consider the user input before proceeding (if not empty). + +## Pre-Execution Checks + +**Check for extension hooks (before clarification)**: +- Check if `.specify/extensions.yml` exists in the project root. +- If it exists, read it and look for entries under the `hooks.before_clarify` key +- If the YAML cannot be parsed or is invalid, skip hook checking silently and continue normally +- Filter out hooks where `enabled` is explicitly `false`. Treat hooks without an `enabled` field as enabled by default. +- For each remaining hook, do **not** attempt to interpret or evaluate hook `condition` expressions: + - If the hook has no `condition` field, or it is null/empty, treat the hook as executable + - If the hook defines a non-empty `condition`, skip the hook and leave condition evaluation to the HookExecutor implementation +- For each executable hook, output the following based on its `optional` flag: + - **Optional hook** (`optional: true`): + ``` + ## Extension Hooks + + **Optional Pre-Hook**: {extension} + Command: `/{command}` + Description: {description} + + Prompt: {prompt} + To execute: `/{command}` + ``` + - **Mandatory hook** (`optional: false`): + ``` + ## Extension Hooks + + **Automatic Pre-Hook**: {extension} + Executing: `/{command}` + EXECUTE_COMMAND: {command} + + Wait for the result of the hook command before proceeding to the Outline. + ``` +- If no hooks are registered or `.specify/extensions.yml` does not exist, skip silently + +## Outline + +Goal: Detect and reduce ambiguity or missing decision points in the active feature specification and record the clarifications directly in the spec file. + +Note: This clarification workflow is expected to run (and be completed) BEFORE invoking `/speckit.plan`. If the user explicitly states they are skipping clarification (e.g., exploratory spike), you may proceed, but must warn that downstream rework risk increases. + +Execution steps: + +1. Run `.specify/scripts/bash/check-prerequisites.sh --json --paths-only` from repo root **once** (combined `--json --paths-only` mode / `-Json -PathsOnly`). Parse minimal JSON payload fields: + - `FEATURE_DIR` + - `FEATURE_SPEC` + - (Optionally capture `IMPL_PLAN`, `TASKS` for future chained flows.) + - If JSON parsing fails, abort and instruct user to re-run `/speckit.specify` or verify feature branch environment. + - For single quotes in args like "I'm Groot", use escape syntax: e.g 'I'\''m Groot' (or double-quote if possible: "I'm Groot"). + +2. Load the current spec file. Perform a structured ambiguity & coverage scan using this taxonomy. For each category, mark status: Clear / Partial / Missing. Produce an internal coverage map used for prioritization (do not output raw map unless no questions will be asked). + + Functional Scope & Behavior: + - Core user goals & success criteria + - Explicit out-of-scope declarations + - User roles / personas differentiation + + Domain & Data Model: + - Entities, attributes, relationships + - Identity & uniqueness rules + - Lifecycle/state transitions + - Data volume / scale assumptions + + Interaction & UX Flow: + - Critical user journeys / sequences + - Error/empty/loading states + - Accessibility or localization notes + + Non-Functional Quality Attributes: + - Performance (latency, throughput targets) + - Scalability (horizontal/vertical, limits) + - Reliability & availability (uptime, recovery expectations) + - Observability (logging, metrics, tracing signals) + - Security & privacy (authN/Z, data protection, threat assumptions) + - Compliance / regulatory constraints (if any) + + Integration & External Dependencies: + - External services/APIs and failure modes + - Data import/export formats + - Protocol/versioning assumptions + + Edge Cases & Failure Handling: + - Negative scenarios + - Rate limiting / throttling + - Conflict resolution (e.g., concurrent edits) + + Constraints & Tradeoffs: + - Technical constraints (language, storage, hosting) + - Explicit tradeoffs or rejected alternatives + + Terminology & Consistency: + - Canonical glossary terms + - Avoided synonyms / deprecated terms + + Completion Signals: + - Acceptance criteria testability + - Measurable Definition of Done style indicators + + Misc / Placeholders: + - TODO markers / unresolved decisions + - Ambiguous adjectives ("robust", "intuitive") lacking quantification + + For each category with Partial or Missing status, add a candidate question opportunity unless: + - Clarification would not materially change implementation or validation strategy + - Information is better deferred to planning phase (note internally) + +3. Generate (internally) a prioritized queue of candidate clarification questions (maximum 5). Do NOT output them all at once. Apply these constraints: + - Maximum of 5 total questions across the whole session. + - Each question must be answerable with EITHER: + - A short multiple‑choice selection (2–5 distinct, mutually exclusive options), OR + - A one-word / short‑phrase answer (explicitly constrain: "Answer in <=5 words"). + - Only include questions whose answers materially impact architecture, data modeling, task decomposition, test design, UX behavior, operational readiness, or compliance validation. + - Ensure category coverage balance: attempt to cover the highest impact unresolved categories first; avoid asking two low-impact questions when a single high-impact area (e.g., security posture) is unresolved. + - Exclude questions already answered, trivial stylistic preferences, or plan-level execution details (unless blocking correctness). + - Favor clarifications that reduce downstream rework risk or prevent misaligned acceptance tests. + - If more than 5 categories remain unresolved, select the top 5 by (Impact * Uncertainty) heuristic. + +4. Sequential questioning loop (interactive): + - Present EXACTLY ONE question at a time. + - For multiple‑choice questions: + - **Analyze all options** and determine the **most suitable option** based on: + - Best practices for the project type + - Common patterns in similar implementations + - Risk reduction (security, performance, maintainability) + - Alignment with any explicit project goals or constraints visible in the spec + - Present your **recommended option prominently** at the top with clear reasoning (1-2 sentences explaining why this is the best choice). + - Format as: `**Recommended:** Option [X] - ` + - Then render all options as a Markdown table: + + | Option | Description | + |--------|-------------| + | A |