From 788add8f4da7569e3d2ab98d3c0b5000964848b3 Mon Sep 17 00:00:00 2001 From: Ovi Trif Date: Mon, 4 May 2026 18:33:02 +0200 Subject: [PATCH 1/4] docs: improve pr qa instructions --- {.claude => .agents}/commands/pr.md | 62 +++++++++++++++++------- {.claude => .agents}/commands/release.md | 0 .claude/commands | 1 + AGENTS.md | 6 ++- 4 files changed, 51 insertions(+), 18 deletions(-) rename {.claude => .agents}/commands/pr.md (75%) rename {.claude => .agents}/commands/release.md (100%) create mode 120000 .claude/commands diff --git a/.claude/commands/pr.md b/.agents/commands/pr.md similarity index 75% rename from .claude/commands/pr.md rename to .agents/commands/pr.md index d99c0eed9..5b35e24b8 100644 --- a/.claude/commands/pr.md +++ b/.agents/commands/pr.md @@ -122,31 +122,59 @@ This PR adds support for... When the user provides custom instructions after `--`: - Parse any referenced commit SHAs and read their full messages - Focus the description content on areas the user emphasizes -- Structure QA Notes according to user's specific testing instructions +- Structure QA Notes according to user's specific manual testing instructions - Custom instructions take priority over default generation rules for sections they address -- Preserve exact testing steps provided by the user (don't summarize or omit details) - -**QA Notes / Testing Scenarios:** -- Structure with numbered headings and steps -- Make steps easily referenceable -- Be specific about what to test and expected outcomes +- Preserve exact manual testing steps provided by the user (don't summarize or omit details) +- If custom instructions include automated checks, keep them out of the PR body and mention them only in the assistant's chat summary + +**QA Notes / Manual Tests:** +- QA Notes are only for actionable human QA instructions. +- Always use this structure: + ```md + ### QA Notes + #### Manual Tests + ``` +- Do not include automated verification entries in PR bodies. +- Do not mention CI checks, local verification commands, Gradle tasks, detekt, lint, unit tests, build passes, cargo test, cargo clippy, npm test, typecheck, or similar automated checks in PR-body QA notes. +- If automated checks were run, mention them only in the assistant's chat summary, not in the PR description. +- If no actionable manual validation exists, write `N/A` under `#### Manual Tests`. +- Write manual tests using this template: + ```md + - [ ] **{numbering}.** {optional_condition + →} {screen_action} → {next_screen_action}: expectation + ``` +- Use a list of unchecked checkboxes for each individual test. +- Use a numbered prefix for each test, in bold, for example `**1.**`, `**2.**`. +- Use `regression:` for regression checks, positioned after the numbering. +- Use sub-lists for variations of the same test. +- Use letter suffixes in numbering for each variation when a test has a sub-list, for example `**3a.**`, `**3b.**`. +- Always use `→` to denote navigation, for example `Send → Amount`. +- Use screen names from code, formatted as separate words without the `Screen` suffix, for example `SendAmountScreen` becomes `Send Amount`. +- Use short-form wording like `in-sheet` for sheet screens, `nav` for navigation, `back` for back nav, and `LN` for Lightning Network. **For library repos (has `bindings/` directory or `Cargo.toml`):** -Structure QA Notes around testing and integration: +Structure QA Notes around manual integration validation only. Automated checks still belong in the assistant's chat summary. Example: ``` ### QA Notes +#### Manual Tests +- [ ] **1.** Consumer app → exercise updated binding flow: behavior matches previous release. +- [ ] **2.** `regression:` Android integration screen → trigger changed API path: no crash or stale data. +``` -#### Testing -- [ ] `cargo test` passes -- [ ] `cargo clippy` clean -- [ ] Android bindings: `./build_android.sh` -- [ ] iOS bindings: `./build_ios.sh` - -#### Integration -- Tested in: [bitkit-android#XXX](link) -- Or N/A if internal refactor with no API changes +Concrete style target: +```md +### QA Notes +#### Manual Tests +- [ ] **1.** No usable channels/spending balance → scan LN invoice: error shows immediately, not after 15s. +- [ ] **2.** Scanner → scan fixed amount LN invoice: Send Confirm or QuickPay opens directly. +- [ ] **3a.** `regression:` Send → scanner/paste fixed amount LN invoice: in-sheet nav to Confirm or QuickPay. + - [ ] **3b.** `regression:` Variable amount LN invoice/LNURL-pay: lands on Amount view. +- [ ] **4a.** Activity Detail of LN transfer → tap Connection: lands on Channel Detail. + - [ ] **4b.** back: returns to Activity Detail. +- [ ] **5a.** Settings → Lightning Connections → tap channel: still opens Channel Detail. + - [ ] **5b.** back: returns to Connections List. +- [ ] **6.** `regression:` Channel Detail → tap Close Connection: works. ``` **Preview Section (conditional):** diff --git a/.claude/commands/release.md b/.agents/commands/release.md similarity index 100% rename from .claude/commands/release.md rename to .agents/commands/release.md diff --git a/.claude/commands b/.claude/commands new file mode 120000 index 000000000..1ea3574a1 --- /dev/null +++ b/.claude/commands @@ -0,0 +1 @@ +../.agents/commands \ No newline at end of file diff --git a/AGENTS.md b/AGENTS.md index 585ea46e8..3e003df28 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -1,6 +1,10 @@ # CLAUDE.md -This file provides guidance to AI agents like Cursor/Claude Code/Codex/WARP when working with code in this repository. +This file provides guidance to Codex, Claude Code, and Cursor when working with code in this repository. + +## Agent Commands + +Durable shared agent command specs live in `.agents/commands/`. For PR creation, follow `.agents/commands/pr.md`; `.claude/commands` is a compatibility symlink to the same files. ## Build Commands From c3919c4b587ee1a953e0ba3b89b1aa7bbc914c5d Mon Sep 17 00:00:00 2001 From: Ovi Trif Date: Tue, 5 May 2026 13:23:07 +0200 Subject: [PATCH 2/4] docs: add automated pr test notes --- .agents/commands/pr.md | 25 +++++++++++++++++-------- 1 file changed, 17 insertions(+), 8 deletions(-) diff --git a/.agents/commands/pr.md b/.agents/commands/pr.md index 5b35e24b8..b81462c68 100644 --- a/.agents/commands/pr.md +++ b/.agents/commands/pr.md @@ -122,22 +122,24 @@ This PR adds support for... When the user provides custom instructions after `--`: - Parse any referenced commit SHAs and read their full messages - Focus the description content on areas the user emphasizes -- Structure QA Notes according to user's specific manual testing instructions +- Structure QA Notes according to user's specific manual testing instructions and automated coverage notes - Custom instructions take priority over default generation rules for sections they address - Preserve exact manual testing steps provided by the user (don't summarize or omit details) -- If custom instructions include automated checks, keep them out of the PR body and mention them only in the assistant's chat summary +- If custom instructions include automated checks, place them under `#### Automated Tests` -**QA Notes / Manual Tests:** -- QA Notes are only for actionable human QA instructions. +**QA Notes / Tests:** +- QA Notes separate actionable human QA instructions from automated verification coverage. - Always use this structure: ```md ### QA Notes #### Manual Tests + #### Automated Tests ``` -- Do not include automated verification entries in PR bodies. -- Do not mention CI checks, local verification commands, Gradle tasks, detekt, lint, unit tests, build passes, cargo test, cargo clippy, npm test, typecheck, or similar automated checks in PR-body QA notes. -- If automated checks were run, mention them only in the assistant's chat summary, not in the PR description. +- Keep local verification commands, Gradle tasks, detekt, lint, unit tests, build passes, cargo test, cargo clippy, npm test, typecheck, CI coverage, or similar automated checks out of `#### Manual Tests`; list them under `#### Automated Tests`. +- Use `#### Automated Tests` to list automated checks that were run locally and any automated coverage added or updated with the change, such as unit tests, integration tests, or CI checks. +- For workflow behavior validation, include `(after merge)` in the automated test item because workflow changes only take effect for PRs opened after the workflow update merges. - If no actionable manual validation exists, write `N/A` under `#### Manual Tests`. +- If no automated checks were run and no automated coverage changed, write `N/A` under `#### Automated Tests`. - Write manual tests using this template: ```md - [ ] **{numbering}.** {optional_condition + →} {screen_action} → {next_screen_action}: expectation @@ -152,7 +154,7 @@ When the user provides custom instructions after `--`: - Use short-form wording like `in-sheet` for sheet screens, `nav` for navigation, `back` for back nav, and `LN` for Lightning Network. **For library repos (has `bindings/` directory or `Cargo.toml`):** -Structure QA Notes around manual integration validation only. Automated checks still belong in the assistant's chat summary. +Structure manual QA around integration validation only. Automated checks belong under `#### Automated Tests`. Example: ``` @@ -160,6 +162,9 @@ Example: #### Manual Tests - [ ] **1.** Consumer app → exercise updated binding flow: behavior matches previous release. - [ ] **2.** `regression:` Android integration screen → trigger changed API path: no crash or stale data. +#### Automated Tests +- [x] cargo test +- [x] Android binding integration tests ``` Concrete style target: @@ -175,6 +180,10 @@ Concrete style target: - [ ] **5a.** Settings → Lightning Connections → tap channel: still opens Channel Detail. - [ ] **5b.** back: returns to Connections List. - [ ] **6.** `regression:` Channel Detail → tap Close Connection: works. +#### Automated Tests +- [x] ./gradlew compileDevDebugKotlin +- [x] ./gradlew testDevDebugUnitTest +- [x] ./gradlew detekt ``` **Preview Section (conditional):** From e810f4439451fc05f6bc18226eda5395cf16a2a4 Mon Sep 17 00:00:00 2001 From: Ovi Trif Date: Tue, 5 May 2026 13:27:21 +0200 Subject: [PATCH 3/4] docs: rename automated pr checks --- .agents/commands/pr.md | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/.agents/commands/pr.md b/.agents/commands/pr.md index b81462c68..91163112f 100644 --- a/.agents/commands/pr.md +++ b/.agents/commands/pr.md @@ -125,21 +125,21 @@ When the user provides custom instructions after `--`: - Structure QA Notes according to user's specific manual testing instructions and automated coverage notes - Custom instructions take priority over default generation rules for sections they address - Preserve exact manual testing steps provided by the user (don't summarize or omit details) -- If custom instructions include automated checks, place them under `#### Automated Tests` +- If custom instructions include automated checks, place them under `#### Automated Checks` -**QA Notes / Tests:** +**QA Notes / Validation:** - QA Notes separate actionable human QA instructions from automated verification coverage. - Always use this structure: ```md ### QA Notes #### Manual Tests - #### Automated Tests + #### Automated Checks ``` -- Keep local verification commands, Gradle tasks, detekt, lint, unit tests, build passes, cargo test, cargo clippy, npm test, typecheck, CI coverage, or similar automated checks out of `#### Manual Tests`; list them under `#### Automated Tests`. -- Use `#### Automated Tests` to list automated checks that were run locally and any automated coverage added or updated with the change, such as unit tests, integration tests, or CI checks. -- For workflow behavior validation, include `(after merge)` in the automated test item because workflow changes only take effect for PRs opened after the workflow update merges. +- Keep local verification commands, Gradle tasks, detekt, lint, unit tests, build passes, cargo test, cargo clippy, npm test, typecheck, CI coverage, or similar automated checks out of `#### Manual Tests`; list them under `#### Automated Checks`. +- Use `#### Automated Checks` to list automated checks that were run locally and any automated coverage added or updated with the change, such as unit tests, integration tests, or CI checks. +- For workflow behavior validation, include `(after merge)` in the automated check item because workflow changes only take effect for PRs opened after the workflow update merges. - If no actionable manual validation exists, write `N/A` under `#### Manual Tests`. -- If no automated checks were run and no automated coverage changed, write `N/A` under `#### Automated Tests`. +- If no automated checks were run and no automated coverage changed, write `N/A` under `#### Automated Checks`. - Write manual tests using this template: ```md - [ ] **{numbering}.** {optional_condition + →} {screen_action} → {next_screen_action}: expectation @@ -154,7 +154,7 @@ When the user provides custom instructions after `--`: - Use short-form wording like `in-sheet` for sheet screens, `nav` for navigation, `back` for back nav, and `LN` for Lightning Network. **For library repos (has `bindings/` directory or `Cargo.toml`):** -Structure manual QA around integration validation only. Automated checks belong under `#### Automated Tests`. +Structure manual QA around integration validation only. Automated checks belong under `#### Automated Checks`. Example: ``` @@ -162,7 +162,7 @@ Example: #### Manual Tests - [ ] **1.** Consumer app → exercise updated binding flow: behavior matches previous release. - [ ] **2.** `regression:` Android integration screen → trigger changed API path: no crash or stale data. -#### Automated Tests +#### Automated Checks - [x] cargo test - [x] Android binding integration tests ``` @@ -180,7 +180,7 @@ Concrete style target: - [ ] **5a.** Settings → Lightning Connections → tap channel: still opens Channel Detail. - [ ] **5b.** back: returns to Connections List. - [ ] **6.** `regression:` Channel Detail → tap Close Connection: works. -#### Automated Tests +#### Automated Checks - [x] ./gradlew compileDevDebugKotlin - [x] ./gradlew testDevDebugUnitTest - [x] ./gradlew detekt From 0ac9ad4d26f1505be465d8adfdb390e37c00b0e9 Mon Sep 17 00:00:00 2001 From: Ovi Trif Date: Wed, 6 May 2026 12:31:46 +0200 Subject: [PATCH 4/4] chore: refine pr qa notes --- .agents/commands/pr.md | 20 ++++++++++++-------- 1 file changed, 12 insertions(+), 8 deletions(-) diff --git a/.agents/commands/pr.md b/.agents/commands/pr.md index 91163112f..c986b8fde 100644 --- a/.agents/commands/pr.md +++ b/.agents/commands/pr.md @@ -125,7 +125,7 @@ When the user provides custom instructions after `--`: - Structure QA Notes according to user's specific manual testing instructions and automated coverage notes - Custom instructions take priority over default generation rules for sections they address - Preserve exact manual testing steps provided by the user (don't summarize or omit details) -- If custom instructions include automated checks, place them under `#### Automated Checks` +- If custom instructions include automated checks or coverage notes, place them under `#### Automated Checks` **QA Notes / Validation:** - QA Notes separate actionable human QA instructions from automated verification coverage. @@ -135,8 +135,11 @@ When the user provides custom instructions after `--`: #### Manual Tests #### Automated Checks ``` -- Keep local verification commands, Gradle tasks, detekt, lint, unit tests, build passes, cargo test, cargo clippy, npm test, typecheck, CI coverage, or similar automated checks out of `#### Manual Tests`; list them under `#### Automated Checks`. -- Use `#### Automated Checks` to list automated checks that were run locally and any automated coverage added or updated with the change, such as unit tests, integration tests, or CI checks. +- Keep local verification commands, Gradle tasks, detekt, lint, unit tests, build passes, cargo test, cargo clippy, npm test, typecheck, CI coverage, or similar automated checks out of `#### Manual Tests`; summarize them under `#### Automated Checks` when they add useful context. +- Use `#### Automated Checks` to summarize automated verification evidence, prioritizing coverage added, modified, or removed with file paths and a short explanation. +- For removed automated coverage, state why it was removed. +- Do not list standard CI or PR bot commands as checkbox items just because they run for every PR. If standard CI coverage is worth mentioning, summarize it in one sentence. +- List raw commands only when they were run locally, are non-standard, use special flags or environment values, validate workflow behavior, or explain a meaningful verification gap. - For workflow behavior validation, include `(after merge)` in the automated check item because workflow changes only take effect for PRs opened after the workflow update merges. - If no actionable manual validation exists, write `N/A` under `#### Manual Tests`. - If no automated checks were run and no automated coverage changed, write `N/A` under `#### Automated Checks`. @@ -163,8 +166,8 @@ Example: - [ ] **1.** Consumer app → exercise updated binding flow: behavior matches previous release. - [ ] **2.** `regression:` Android integration screen → trigger changed API path: no crash or stale data. #### Automated Checks -- [x] cargo test -- [x] Android binding integration tests +- Binding tests added: cover updated Android API path in `bindings/android/...`. +- CI: standard cargo and binding checks run by the PR bot. ``` Concrete style target: @@ -181,9 +184,10 @@ Concrete style target: - [ ] **5b.** back: returns to Connections List. - [ ] **6.** `regression:` Channel Detail → tap Close Connection: works. #### Automated Checks -- [x] ./gradlew compileDevDebugKotlin -- [x] ./gradlew testDevDebugUnitTest -- [x] ./gradlew detekt +- Unit tests added: cover invoice timeout handling in `app/src/test/.../SendInvoiceTest.kt`. +- Unit tests modified: update channel navigation assertions in `app/src/test/.../ChannelDetailTest.kt`. +- Test coverage removed: delete stale mock-only assertions from `app/src/test/.../OldFlowTest.kt` because the flow no longer exists. +- CI: standard compile, unit test, and detekt checks run by the PR bot. ``` **Preview Section (conditional):**