From 05a2f135528151ce8f159cdb811c3ce69321ead7 Mon Sep 17 00:00:00 2001 From: Brandon McAnsh Date: Fri, 25 Sep 2026 12:14:59 -0400 Subject: [PATCH] chore(claude): give agent prompts a finish line and drop needless stops Several agents and skills described steps but no check that the result works, and a few stopped mid-run to ask about local, reversible edits. Finish lines: - test-gap-finder: generated tests pass testDebugUnitTest - module-scaffolder: the module (and the app, for features) compiles - dep-review: the recommended test plan passes after applying changes - fetch-protos: re-run the build after domain-model edits and scaffolding; Step 4 ran before them - triage: the brief names a root cause with .kt:NN evidence, or states confidence and what is missing; steps may be skipped, and the 2-hop cap on call-chain reading is gone - CLAUDE.md: a default definition of done for code changes Confirmation stops removed: - module-scaffolder no longer asks for dependencies; a subagent can't reach the user, so it returned early - fetch-protos applies domain-model edits, defaults new UserFlags fields to read-only, and scaffolds only when invoked with `scaffold` - dep-review applies changes in a worktree instead of asking first, and no longer runs `gh pr checkout` in the main checkout Push, merge, commit, and release gates are unchanged. --- .claude/agents/module-scaffolder.md | 4 +++- .claude/agents/test-gap-finder.md | 2 ++ .claude/skills/dep-review/SKILL.md | 14 +++++++------- .claude/skills/fetch-protos/SKILL.md | 25 +++++++++++++++---------- .claude/skills/triage/SKILL.md | 10 ++++++++-- CLAUDE.md | 4 ++++ 6 files changed, 39 insertions(+), 20 deletions(-) diff --git a/.claude/agents/module-scaffolder.md b/.claude/agents/module-scaffolder.md index c9b3952d37..621d7d3772 100644 --- a/.claude/agents/module-scaffolder.md +++ b/.claude/agents/module-scaffolder.md @@ -97,6 +97,8 @@ If Compose is needed, use `flipcash.android.library.compose` instead. - Add an `annotatedEntry` to the `appEntryProvider` in `apps/flipcash/app/src/main/kotlin/com/flipcash/app/internal/ui/navigation/AppScreenContent.kt` - If deeplink-reachable: add URL pattern to `AppRouter` in `apps/flipcash/shared/router/` +Done when `./gradlew ::compileDebugKotlin` succeeds, and for feature modules also `./gradlew :apps:flipcash:app:compileDebugKotlin`. + ## File Templates ### Screen (public entry point) @@ -166,4 +168,4 @@ internal fun ScreenContent( - Use `internal` visibility for everything except the public Screen composable - Follow the existing naming conventions exactly (check similar modules if unsure) - Hyphenated module names use the hyphenated form in paths and camelCase in packages (e.g., module `currency-selection` → package `currencyselection`) -- Ask the user what dependencies the module needs if not specified +- If dependencies aren't specified, add none beyond the convention plugin, and list likely additions in your report diff --git a/.claude/agents/test-gap-finder.md b/.claude/agents/test-gap-finder.md index f191b2ba21..09ba52f55a 100644 --- a/.claude/agents/test-gap-finder.md +++ b/.claude/agents/test-gap-finder.md @@ -10,6 +10,8 @@ You are a test coverage analyst and test author for a 100+ module Android projec Given a target (file, module, or feature area), analyze what has test coverage and what doesn't, then generate test stubs or full tests as needed. +Done when the generated tests pass `./gradlew ::testDebugUnitTest`. List any stub left with a TODO and why. + ## Project Test Patterns ### Frameworks & Tools diff --git a/.claude/skills/dep-review/SKILL.md b/.claude/skills/dep-review/SKILL.md index 8912f0b83b..f07a08d315 100644 --- a/.claude/skills/dep-review/SKILL.md +++ b/.claude/skills/dep-review/SKILL.md @@ -159,14 +159,15 @@ If no code changes are needed, state: "No code changes required — safe to merg If the verdict is **APPROVE WITH CHANGES**: -1. Show the user each required change with a before/after diff -2. Ask the user if they want to apply the changes -3. If confirmed, check out the PR branch and apply the modifications: +1. Check out the PR branch in a worktree (not the main checkout — `gh pr checkout` + there switches your branch) and apply the changes: ```bash - gh pr checkout + git fetch origin + git worktree add -b ../dep-review- origin/ ``` -4. Make the code changes using Edit -5. Offer to commit with: +2. Make the code changes using Edit, and show each one as a before/after diff +3. Run the Recommended Test Plan; done when it passes +4. Offer to commit with: ``` fix(deps): adapt to API changes ``` @@ -174,7 +175,6 @@ If the verdict is **APPROVE WITH CHANGES**: ## Never - Auto-merge a PR without user approval -- Make code changes without showing them first - Skip the release notes check — even for patch bumps, behavioral changes happen - Dismiss a major version bump as low risk without evidence - Push changes to the PR branch without explicit user confirmation diff --git a/.claude/skills/fetch-protos/SKILL.md b/.claude/skills/fetch-protos/SKILL.md index 4469a97bd6..5af389c66c 100644 --- a/.claude/skills/fetch-protos/SKILL.md +++ b/.claude/skills/fetch-protos/SKILL.md @@ -2,9 +2,9 @@ name: fetch-protos description: > Bump a client-protocol artifact, summarize the contract changes it carries, - and scaffold new service stubs. Usage: /fetch-protos [flipcash|opencode] [version] + and scaffold new service stubs. Usage: /fetch-protos [flipcash|opencode] [version] [scaffold] user-invocable: true -argument-hint: "[flipcash|opencode] [version]" +argument-hint: "[flipcash|opencode] [version] [scaffold]" allowed-tools: - Bash - Read @@ -39,12 +39,14 @@ Parse `$ARGUMENTS` to determine targets and an optional version. **Rules:** - Known targets: `flipcash`, `opencode` - If no targets specified, check **both** -- A semver-looking string as the last argument is the version to move to; without - one, use the latest release +- A semver-looking string is the version to move to; without one, use the latest + release +- The word `scaffold` anywhere in the arguments opts in to Step 6 scaffolding - Examples: - `/fetch-protos` → check both artifacts for newer releases - `/fetch-protos flipcash` → flipcash only, latest release - `/fetch-protos opencode 0.2.0` → opencode at 0.2.0 + - `/fetch-protos flipcash scaffold` → flipcash latest, scaffolding new RPCs ## Steps @@ -164,8 +166,9 @@ mapped — follow the same pattern. ##### UserFlags-specific chain When `UserFlags` fields change, the following files form a chain that must all be -updated together. Ask the user whether the new field should be **read-only** (display -only) or **editable** (overridable via the debug editor). +updated together. Default a new field to **read-only** (display only) rather than +**editable** (overridable via the debug editor); flag it in the summary if it looks +like a flag you'd want to override. | # | File | What to update | |---|------|----------------| @@ -187,12 +190,13 @@ For **read-only** fields (e.g., booleans like `enablePhoneNumberSend`): - In `UserFlagsViewModel`, add to `readOnlyEntries` with a string resource label - No changes needed in `Overrides`, `Field.kt`, or `UserFlagsCoordinator` -Present a report of domain model updates needed and apply them after user confirmation. +Apply the domain model updates and include them in the Step 7 summary. ### Step 6 — Scaffold new service stubs -For RPCs marked as needing scaffolding, ask the user if they want to scaffold them. -If confirmed, generate code following the patterns below. +Scaffold only if the invocation asks for it (e.g. `/fetch-protos flipcash scaffold`); +otherwise list the RPCs that need scaffolding in the Step 7 summary. When scaffolding, +generate code following the patterns below. #### Api method pattern @@ -304,6 +308,8 @@ the corresponding Hilt module (`FlipcashModule.kt` or `OpenCodeModule.kt`). ### Step 7 — Review and commit +Re-run the Step 4 build; done when it passes with the new code in place. + Show the user a summary of all changes (proto updates + any scaffolded code). Offer to commit with a conventional commit message: @@ -325,4 +331,3 @@ feat(): scaffold service stubs for new RPCs - Try to edit the generated protobuf code — it lives in the published artifact - Commit without user approval - Skip build verification -- Scaffold service code without asking the user first diff --git a/.claude/skills/triage/SKILL.md b/.claude/skills/triage/SKILL.md index 5a099cc7e3..33fc2803bd 100644 --- a/.claude/skills/triage/SKILL.md +++ b/.claude/skills/triage/SKILL.md @@ -20,7 +20,12 @@ allowed-tools: # Triage: daily Bugsnag issue investigation You are a senior Android engineer performing daily Bugsnag triage for the -Flipcash Android app. Follow the steps below exactly. +Flipcash Android app. + +Done when `.claude/plans/triage-.md` follows the template and names a +root cause backed by `.kt:NN` evidence, or states your confidence and what +evidence is missing. The steps below are the usual path; skip any the evidence +makes unnecessary. ## Step 1 — Fetch the issue @@ -159,7 +164,8 @@ Collect: Using the evidence from Step 4: 1. Read the source files identified in the stack trace. -2. Follow the call chain — read callers and callees within 2 hops. +2. Follow the call chain — read callers and callees until the hypothesis is + confirmed or ruled out against the logs and breadcrumbs. 3. Check for known patterns: null-safety violations, lifecycle issues, threading bugs, uncaught coroutine exceptions, missing error handling. 4. Form a hypothesis and verify it against the logs and breadcrumbs. diff --git a/CLAUDE.md b/CLAUDE.md index 170a698b52..74258b47c7 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -85,6 +85,10 @@ The feature plugin automatically includes `:libs:logging`, `:ui:core`, `:ui:comp - **Protobuf models**: Backend models come from the published `com.flipcash:{ocp,flipcash2}-client-protocol` artifacts, not from protos in this repo; the contracts are generated in their own repos - **Dark mode only**: App forces `MODE_NIGHT_YES` +## Definition of Done + +A code change is done when the touched modules' unit tests pass and `./gradlew :apps:flipcash:app:compileDebugKotlin` succeeds. + ## Namespaces - App: `com.flipcash.app.android` (no `applicationIdSuffix` — debug and release share the ID, so installing one replaces the other)