docs(qa): release-review skill and six GUI A/B regression scenarios - #952
docs(qa): release-review skill and six GUI A/B regression scenarios#952Claudius-Maginificent wants to merge 2 commits into
Conversation
Captures a repeatable nightly/weekly QA procedure (verified changelog, scoped code review, true old-vs-new binary GUI regression pass, visual changelog) as a reusable, environment-independent skill, plus the version-agnostic GUI scenarios it authored for the current diff-touched risk areas (identity key recovery, wallet Max-send/asset-lock, DPNS registration, error banners/Identity Home actions, shielded availability after sync, and DAPI budget resilience across repeated resyncs). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
📝 WalkthroughWalkthroughAdds a ChangesRelease review campaign
Estimated code review effort: 3 (Moderate) | ~20 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
⛔ Blockers found — Sonnet deferred (commit d327b55) |
thepastaclaw
left a comment
thepastaclaw
left a comment
There was a problem hiding this comment.
Preliminary review — Codex only
The documentation adds a useful release-QA framework, but four confirmed process defects can select the wrong revisions, leave the GUI fixtures uninitialized, or compare builds against materially different live-network state. Three additional documentation issues weaken revision provenance, regression attribution, and validation testing, so the procedure does not yet provide the advertised reliable, version-agnostic A/B campaign.
Source: reviewers codex/general=gpt-5.6-sol(completed); verifier=codex/verifier=gpt-5.6-sol(completed); coordinator=openclaw-agent/cliproxy/gpt-5.6-sol(orchestration-only).
Validated blockers were found in the Codex precheck. Sonnet is deferred until a fresh Codex revalidation clears the blocker gate.
Review provenance
- Codex reviewers:
gpt-5.6-sol— general (completed) - Verifier:
gpt-5.6-sol— verifier - Sonnet: not run (deferred by blocker gate)
🔴 4 blocking | 🟡 3 suggestion(s)
🤖 Prompt for all review comments with AI agents
These findings are from an automated code review. Verify each finding against the current code and only fix it if needed.
In `.claude/skills/release-review/SKILL.md`:
- [BLOCKING] .claude/skills/release-review/SKILL.md:86-96: Resolve the canonical repository instead of assuming `origin`
The release lookup explicitly targets `dashpay/dash-evo-tool`, but the following tag, branch, and `ls-remote` commands unconditionally use `origin`. That does not work in a standard fork checkout: at this exact head, `origin` fetches `thepastaclaw/dash-evo-tool`, while `upstream` fetches the canonical repository. The published `v1.0.0-weekly.20260721` tag is absent from `origin`, and `origin/v1.0-dev` resolves to `478b816b99b9f73c721670a0118e6dff8c46204f` instead of canonical `upstream/v1.0-dev` at `2ee9fa64898fcb34d382f11ccea91eecdfe4439f`. Identify a remote whose fetch URL matches `dashpay/dash-evo-tool`, or fetch the canonical URL directly, and use that verified source consistently for tags, the development branch, and remote checks.
- [BLOCKING] .claude/skills/release-review/SKILL.md:139-142: Build the verified development commit instead of symbolic `HEAD`
Phase 1 selects a canonical development-branch revision and explicitly rejects the invoking session's feature branch, but Phase 2b creates the new worktree at `HEAD`. When the skill is invoked from a PR or feature checkout, this builds that checkout rather than the development revision used for the changelog and diff. Resolve the canonical development ref to a commit SHA during Phase 1, record it in `qa-run-state.md`, and use that exact SHA for the diff, worktree, binary, screenshots, and final report.
- [BLOCKING] .claude/skills/release-review/SKILL.md:55-64: Add real GUI fixture provisioning instead of relying on unused variables
The preflight says `.env.example` contains the fixture names currently in use, but that file contains no `E2E_*` entries. `E2E_MN_PROTX_HASH` and `E2E_IDENTITY_ID` have no implementation elsewhere in the repository, while `E2E_WALLET_MNEMONIC` is read by the backend-E2E harness, which explicitly restores the wallet; the GUI binary does not consume it. Each scenario then launches a fresh data directory without selecting Testnet, importing the wallet, discovering or importing identities, or seeding legacy migration data. Fresh GUI settings default to Mainnet, so the documented setup cannot satisfy the scenarios' prerequisites. Document the actual secret-safe fixture source and add explicit GUI provisioning steps for network selection, wallet restoration, identity discovery/import, masternode linkage, and legacy-state seeding where required.
- [BLOCKING] .claude/skills/release-review/SKILL.md:198-205: Give baseline and development runs equivalent starting state
Separate local data directories isolate files, but they do not preserve equivalent live-network inputs. The scenarios mutate shared fixtures: Max sends change wallet balances and UTXOs, deposits and asset locks are consumed, identity top-ups and withdrawals move credits, and a DPNS name cannot be registered twice. Running baseline first and development second against the same wallet, identities, deposits, and names therefore gives the two binaries different starting conditions and can create false regressions or false passes. Define a pairing strategy using independently and equivalently funded fixtures, together with equivalent seeded data-directory snapshots where applicable, and record each side's pre-run balances, UTXOs, deposits, identities, and DPNS names.
- [SUGGESTION] .claude/skills/release-review/SKILL.md:222-227: Do not infer pre-existing behavior from an untouched suspect file
A zero-result `git log <baseline>..<head> -- <suspect-file>` proves only that the file was not edited directly. A changed caller, shared model, persisted-data format, network configuration, feature gate, or dependency can still introduce a regression observed in that file. Use old-versus-new reproduction as the primary attribution evidence, then inspect the transitive change path or bisect the release range before claiming a defect is pre-existing.
In `docs/gui-testing/scenarios/identity-key-recovery-migration.md`:
- [SUGGESTION] docs/gui-testing/scenarios/identity-key-recovery-migration.md:16-19: Remove release-specific refs from the reusable scenarios
The README and PR description call these scenarios version-agnostic, but all six files hardcode baseline `v1.0.0-weekly.20260721` and development ref `origin/v1.0-dev`. The baseline will become obsolete after another published release, and `origin` is already a contributor fork in this checkout. Refer to the baseline and development binaries selected by the current campaign, and record their exact SHAs only in that campaign's artifacts. Apply the same correction to the corresponding header in every new scenario file.
In `docs/gui-testing/scenarios/error-banners-identity-home-actions.md`:
- [SUGGESTION] docs/gui-testing/scenarios/error-banners-identity-home-actions.md:68-72: Compare validation limits only for the same field type
This step presents an overlong DPNS name and a keyword or label as fields that should reject at the same limit, but they intentionally have different constraints. The current validators allow 63 ASCII characters for a DPNS registration name, 50 characters for a contract keyword, and 100 characters for a DashPay account label. Following the example literally would classify correct field-specific behavior as inconsistent. Test the same field through two callers that accept that field, or test each field independently at its own documented boundary.
… shared-state gaps in release-review Addresses review feedback on PR #952: - Never assume `origin` is the canonical repo — resolve the remote whose fetch URL actually matches it, since a fork checkout's `origin` can point elsewhere (confirmed: it silently diverged in one checkout). - Resolve the development branch to a commit SHA once in Phase 1 and use that SHA everywhere downstream; Phase 2b no longer builds a bare `HEAD`, which was ambiguous (and wrong) when this procedure runs from a feature checkout rather than the development branch itself. - Phase 0's fixture section no longer claims `.env.example` defines the E2E_* variable names (it doesn't) or that the GUI reads them (it doesn't either — confirmed via `AppSettings::default()` == Mainnet, and no `E2E_` references anywhere in `src/`). Reframed as a scenario-doc naming convention plus an explicit GUI provisioning sequence (network selection, wallet import, identity discovery) that a fresh data directory needs. - Phase 4 now calls out that isolated data directories don't give the two builds equivalent live-network starting state — fund-moving/name- registering steps need independently-funded fixtures or recorded before/after state, not a shared wallet run sequentially. - Softened the pre-existing-vs-diff-introduced check: an untouched file proves only that it wasn't directly edited, not that nothing upstream of it changed — old-vs-new reproduction is now framed as the primary evidence, the git-log check as corroboration only. - Removed hardcoded baseline-tag/branch references from all six scenario files, which contradicted their own "version-agnostic" framing and would go stale after the next release. - Fixed a scenario step that implied a DPNS name, a contract keyword, and a DashPay account label share one length limit — they're 63/50/100 chars respectively by design; the step now compares one field type at a time. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 12
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.claude/skills/release-review/SKILL.md:
- Around line 260-266: Update the “Not blocking” guidance in the Blocker rule to
exempt only non-user-visible timing noise and behavior reproduced identically on
both builds. Ensure new concurrency or timing races that break the happy flow,
crash the application, or cause data loss are classified as blocking and follow
the existing reproduction requirement.
- Around line 31-37: Update the baseline re-verification in the release review
workflow to compare the canonical peeled remote tag commit from git ls-remote
with the local baseline SHA resolved by git rev-parse <tag>^{commit} before
reusing any worktree or binary artifacts. Fail closed and rebuild or
re-establish the baseline when the SHAs differ, including the corresponding
logic at the other baseline-check location.
- Around line 31-37: Update the release-review workflow to use the canonical
repository for all GitHub CLI commands, including the baseline lookup and
changelog PR inspection, by adding the explicit dashpay/dash-evo-tool repository
target. In the remote validation logic, inspect the fetch remote URL
specifically and require an exact comparison with the canonical repository URL
rather than matching arbitrary remote output or substrings; keep git fetch
aligned with that validated fetch URL.
- Around line 339-351: Update the GUI testing secret-handling procedure around
the existing remediation guidance to require preventive controls before capture
or artifact generation: use protected secret input, prevent secrets in command
arguments, environment dumps, logs, transcripts, screenshots, and inline HTML,
redact generated artifacts, apply restrictive permissions, and scan artifacts
before embedding. Explicitly state that .gitignore does not protect directories
outside the repository, and apply these requirements to the related guidance at
the referenced sections.
- Around line 247-258: Update the guidance for mutating A/B scenarios so
equivalent preconditions are mandatory: require each build and reproduction to
use an independently funded equivalent fixture or a restored snapshot, rather
than merely recording post-baseline differences. Preserve the exception for
read-only steps, and revise the existing alternatives in this section to remove
accounting for changed state as a valid option.
- Around line 118-128: Update the branch-resolution step in the release-review
procedure to fetch the canonical remote's active development branch before
running git rev-parse and recording the development SHA. Keep the existing
contribution-doc discovery, single-SHA reuse, and tag-to-development-SHA range
behavior unchanged.
- Around line 181-183: Update the shell command code fence in the release-review
instructions to use the shell language identifier, preserving the existing build
command unchanged.
In `@docs/gui-testing/README.md`:
- Around line 256-261: Make the release-review rules version-agnostic and
consistent across all listed scenarios: in docs/gui-testing/README.md lines
256-261, define the development artifact once and allow non-blocking outcomes
only for cosmetic differences or behavior identical to baseline; in
docs/gui-testing/scenarios/identity-key-recovery-migration.md lines 117-120,
compare feature presence and behavior between the selected builds without
referring to changes introduced by this diff; in
docs/gui-testing/scenarios/dapi-budget-resilience-after-resync.md lines 91-97,
remove the assumption that baseline failure and development success are
required; in docs/gui-testing/scenarios/dpns-registration-flow.md lines 97-109,
establish that intermittent wrong-identity results remain blocking despite
timing sensitivity; in
docs/gui-testing/scenarios/error-banners-identity-home-actions.md lines 104-108
and docs/gui-testing/scenarios/platform-shielded-availability-after-sync.md
lines 79-82, remove references assuming this PR fixes the behavior.
In `@docs/gui-testing/scenarios/dapi-budget-resilience-after-resync.md`:
- Around line 44-48: Rename the procedure step heading from “Baseline Platform
Info check” to “Initial Platform Info check” while preserving its existing
Platform Info instructions and observations.
- Around line 63-66: Update the log-checking instructions in
docs/gui-testing/scenarios/dapi-budget-resilience-after-resync.md lines 63-66 to
explicitly search the guaranteed $LOG path, or document the verified DET log
path, for DapiAllAddressesExhausted markers. Also update
docs/gui-testing/scenarios/error-banners-identity-home-actions.md lines 65-68 to
specify the actual path used to locate crash and panic output, ensuring both
procedures inspect the process-output destination.
In `@docs/gui-testing/scenarios/identity-key-recovery-migration.md`:
- Around line 129-134: Update the residual note in the identity key recovery
migration scenario so same-numbered voting-key collisions remain blocking and
are not waived as expected HEAD behavior. In the guidance around Step 8,
instruct testers to compare observed behavior across the selected build SHAs and
record any confirmed exception in campaign artifacts before treating it as an
established residual.
- Around line 16-20: In
docs/gui-testing/scenarios/identity-key-recovery-migration.md lines 16-20, add
the parent skill’s fixture-restoration requirement: every mutating scenario must
start each baseline and development run with equivalent funded identity/wallet
state, or restore recorded starting balances, deposits, and identities before
comparison. Apply the same requirement in
docs/gui-testing/scenarios/dapi-budget-resilience-after-resync.md lines 16-18,
docs/gui-testing/scenarios/dpns-registration-flow.md lines 18-20,
docs/gui-testing/scenarios/error-banners-identity-home-actions.md lines 17-19,
docs/gui-testing/scenarios/platform-shielded-availability-after-sync.md lines
14-16, and docs/gui-testing/scenarios/wallet-max-send-asset-lock.md lines 18-21.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: cca68127-563f-4e40-b040-495398373c54
📒 Files selected for processing (8)
.claude/skills/release-review/SKILL.mddocs/gui-testing/README.mddocs/gui-testing/scenarios/dapi-budget-resilience-after-resync.mddocs/gui-testing/scenarios/dpns-registration-flow.mddocs/gui-testing/scenarios/error-banners-identity-home-actions.mddocs/gui-testing/scenarios/identity-key-recovery-migration.mddocs/gui-testing/scenarios/platform-shielded-availability-after-sync.mddocs/gui-testing/scenarios/wallet-max-send-asset-lock.md
| 1. **Re-verify the baseline is still current** (`gh release list` — a newer pre-release may have | ||
| shipped since the prior run). If unchanged, the prior run's worktree/binary for it is still | ||
| valid — check for an existing `git worktree` and a previously-built binary under your scratch | ||
| root before rebuilding anything. | ||
| 2. **Only rebuild what moved.** If the baseline is unchanged and only HEAD advanced, reuse the | ||
| baseline binary as-is; `git fetch` + `git checkout <new-sha>` the HEAD worktree in place and | ||
| rebuild only that binary. This turns a two-cold-build Phase 2b into one incremental build. |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Fail closed when the baseline tag target changes.
The rerun path reuses the old binary when the release list is unchanged. The initial tag check runs git ls-remote but never compares its result with git rev-parse <tag>^{commit}. Compare the canonical peeled tag commit with the recorded baseline SHA before reusing artifacts.
Proposed verification
remote_sha="$(git ls-remote <canonical-remote> 'refs/tags/<tag>^{}' | awk '{print $1}')"
local_sha="$(git rev-parse '<tag>^{commit}')"
test "$remote_sha" = "$local_sha"Also applies to: 112-117
🧰 Tools
🪛 SkillSpector (2.4.4)
[warning] 174: [RA2] Session Persistence: Skill establishes unauthorized persistence across sessions via cron jobs, startup scripts, or state files. Session persistence allows an attacker to maintain access beyond the current interaction.
Remediation: Remove any persistence mechanisms (cron jobs, startup scripts, state files). Skills should not maintain state across sessions without explicit user consent.
(Rogue Agent (RA2))
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.claude/skills/release-review/SKILL.md around lines 31 - 37, Update the
baseline re-verification in the release review workflow to compare the canonical
peeled remote tag commit from git ls-remote with the local baseline SHA resolved
by git rev-parse <tag>^{commit} before reusing any worktree or binary artifacts.
Fail closed and rebuild or re-establish the baseline when the SHAs differ,
including the corresponding logic at the other baseline-check location.
🗄️ Data Integrity & Integration | 🟠 Major
Use the canonical repository for every lookup.
The rerun path calls gh release list without --repo, and the changelog step calls gh pr view <n> without --repo. In a fork checkout, either command can read the wrong repository. The git remote -v | grep ... check can also match a push URL or a non-exact substring, while git fetch uses the fetch URL. Compare fetch URLs exactly and pass --repo dashpay/dash-evo-tool to every GitHub CLI command.
Also applies to: 98-106, 129-133
🧰 Tools
🪛 SkillSpector (2.4.4)
[warning] 174: [RA2] Session Persistence: Skill establishes unauthorized persistence across sessions via cron jobs, startup scripts, or state files. Session persistence allows an attacker to maintain access beyond the current interaction.
Remediation: Remove any persistence mechanisms (cron jobs, startup scripts, state files). Skills should not maintain state across sessions without explicit user consent.
(Rogue Agent (RA2))
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.claude/skills/release-review/SKILL.md around lines 31 - 37, Update the
release-review workflow to use the canonical repository for all GitHub CLI
commands, including the baseline lookup and changelog PR inspection, by adding
the explicit dashpay/dash-evo-tool repository target. In the remote validation
logic, inspect the fetch remote URL specifically and require an exact comparison
with the canonical repository URL rather than matching arbitrary remote output
or substrings; keep git fetch aligned with that validated fetch URL.
| 4. **Determine the active development branch and resolve it to a commit SHA — record that SHA, don't | ||
| carry a branch name or bare `HEAD` forward.** Check the repo's own contribution docs (e.g. | ||
| `CLAUDE.md`, `CONTRIBUTING.md`) rather than assuming `main`/`master`; this project may use a | ||
| dedicated long-lived dev branch instead. Resolve it once: | ||
| `git rev-parse <canonical-remote>/<base-branch>`, and treat that output as **the development SHA** | ||
| for the rest of this run — every later phase (the diff range, Phase 2b's second worktree, | ||
| screenshots, the final report) uses this exact SHA. Never substitute a bare `HEAD` for it (that's | ||
| the invoking checkout's HEAD, not necessarily the development branch's — wrong whenever this | ||
| procedure is run from a feature/PR checkout, which is common) and never re-resolve the branch ref | ||
| later in the run, since it can move while you're still working. Compute the range: | ||
| `git log <tag>..<development-sha> --oneline`. |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Fetch the development branch before resolving its SHA.
Phase 1 fetches the tag but not the development branch. git rev-parse <canonical-remote>/<base-branch> therefore reads a potentially stale local tracking ref. Fetch the exact branch from the canonical remote before resolving and recording the development SHA.
Proposed fix
+git fetch <canonical-remote> "+refs/heads/<base-branch>:refs/remotes/<canonical-remote>/<base-branch>"
git rev-parse <canonical-remote>/<base-branch>📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| 4. **Determine the active development branch and resolve it to a commit SHA — record that SHA, don't | |
| carry a branch name or bare `HEAD` forward.** Check the repo's own contribution docs (e.g. | |
| `CLAUDE.md`, `CONTRIBUTING.md`) rather than assuming `main`/`master`; this project may use a | |
| dedicated long-lived dev branch instead. Resolve it once: | |
| `git rev-parse <canonical-remote>/<base-branch>`, and treat that output as **the development SHA** | |
| for the rest of this run — every later phase (the diff range, Phase 2b's second worktree, | |
| screenshots, the final report) uses this exact SHA. Never substitute a bare `HEAD` for it (that's | |
| the invoking checkout's HEAD, not necessarily the development branch's — wrong whenever this | |
| procedure is run from a feature/PR checkout, which is common) and never re-resolve the branch ref | |
| later in the run, since it can move while you're still working. Compute the range: | |
| `git log <tag>..<development-sha> --oneline`. | |
| 4. **Determine the active development branch and resolve it to a commit SHA — record that SHA, don't | |
| carry a branch name or bare `HEAD` forward.** Check the repo's own contribution docs (e.g. | |
| `CLAUDE.md`, `CONTRIBUTING.md`) rather than assuming `main`/`master`; this project may use a | |
| dedicated long-lived dev branch instead. Resolve it once: | |
| `git fetch <canonical-remote> "+refs/heads/<base-branch>:refs/remotes/<canonical-remote>/<base-branch>"` | |
| `git rev-parse <canonical-remote>/<base-branch>`, and treat that output as **the development SHA** | |
| for the rest of this run — every later phase (the diff range, Phase 2b's second worktree, | |
| screenshots, the final report) uses this exact SHA. Never substitute a bare `HEAD` for it (that's | |
| the invoking checkout's HEAD, not necessarily the development branch's — wrong whenever this | |
| procedure is run from a feature/PR checkout, which is common) and never re-resolve the branch ref | |
| later in the run, since it can move while you're still working. Compute the range: | |
| `git log <tag>..<development-sha> --oneline`. |
🧰 Tools
🪛 SkillSpector (2.4.4)
[warning] 174: [RA2] Session Persistence: Skill establishes unauthorized persistence across sessions via cron jobs, startup scripts, or state files. Session persistence allows an attacker to maintain access beyond the current interaction.
Remediation: Remove any persistence mechanisms (cron jobs, startup scripts, state files). Skills should not maintain state across sessions without explicit user consent.
(Rogue Agent (RA2))
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.claude/skills/release-review/SKILL.md around lines 118 - 128, Update the
branch-resolution step in the release-review procedure to fetch the canonical
remote's active development branch before running git rev-parse and recording
the development SHA. Keep the existing contribution-doc discovery, single-SHA
reuse, and tag-to-development-SHA range behavior unchanged.
| ``` | ||
| CARGO_TARGET_DIR=<scratch-root>/<name>-target cargo build --bin dash-evo-tool --manifest-path <worktree>/Cargo.toml | ||
| ``` |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Add a language identifier to the shell fence.
The build command fence has no language identifier. markdownlint reports MD040 for this block. Use shell.
Proposed fix
-```
+```shell
CARGO_TARGET_DIR=<scratch-root>/<name>-target cargo build --bin dash-evo-tool --manifest-path <worktree>/Cargo.toml
</details>
<!-- suggestion_start -->
<details>
<summary>📝 Committable suggestion</summary>
> ‼️ **IMPORTANT**
> Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
```suggestion
🧰 Tools
🪛 markdownlint-cli2 (0.23.1)
[warning] 181-181: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🪛 SkillSpector (2.4.4)
[warning] 174: [RA2] Session Persistence: Skill establishes unauthorized persistence across sessions via cron jobs, startup scripts, or state files. Session persistence allows an attacker to maintain access beyond the current interaction.
Remediation: Remove any persistence mechanisms (cron jobs, startup scripts, state files). Skills should not maintain state across sessions without explicit user consent.
(Rogue Agent (RA2))
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.claude/skills/release-review/SKILL.md around lines 181 - 183, Update the
shell command code fence in the release-review instructions to use the shell
language identifier, preserving the existing build command unchanged.
Source: Linters/SAST tools
| **Isolated data directories give the two builds separate local state, but not separate live-network | ||
| state — plan for that separately.** A scenario that spends a UTXO, consumes a deposit, registers a | ||
| DPNS name, or moves credits mutates shared on-chain/live-network fixture state; running the baseline | ||
| binary through it and then the development binary through the identical flow means the second run | ||
| starts from state the first run already changed (an already-consumed deposit, a spent UTXO set, a | ||
| name that's no longer available), which can produce a false regression or a false pass. For any | ||
| scenario step that mutates fund-moving or name-registering state, do one of: give each build its | ||
| own independently-funded/equivalent fixture (separate wallet or identity per side) rather than | ||
| sharing one; or explicitly record each side's starting balances/UTXOs/deposits/identities/DPNS names | ||
| before that step and account for the difference when judging the result. Steps that only read state | ||
| (navigation, display checks) aren't affected — reserve this for anything that actually spends or | ||
| registers something. |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major
Make equivalent fixtures mandatory for mutating A/B scenarios.
Recording the difference after the old run does not preserve equivalent preconditions. The old run can spend a UTXO, consume a deposit, register a name, or move credits before the new run starts. The new run can then fail before reaching the changed code. Require an independently funded equivalent fixture or a restored snapshot for each build and each required reproduction.
🧰 Tools
🪛 SkillSpector (2.4.4)
[warning] 174: [RA2] Session Persistence: Skill establishes unauthorized persistence across sessions via cron jobs, startup scripts, or state files. Session persistence allows an attacker to maintain access beyond the current interaction.
Remediation: Remove any persistence mechanisms (cron jobs, startup scripts, state files). Skills should not maintain state across sessions without explicit user consent.
(Rogue Agent (RA2))
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.claude/skills/release-review/SKILL.md around lines 247 - 258, Update the
guidance for mutating A/B scenarios so equivalent preconditions are mandatory:
require each build and reproduction to use an independently funded equivalent
fixture or a restored snapshot, rather than merely recording post-baseline
differences. Preserve the exception for read-only steps, and revise the existing
alternatives in this section to remove accounting for changed state as a valid
option.
| **Blocker rule** (confirm with the user for each run — the default below is a starting point, not a | ||
| universal constant): | ||
| - **Blocking**: new version worse than old from the user's perspective **in the happy flow**, or a | ||
| **data-loss** scenario. | ||
| - **Not blocking**: concurrency/timing glitches; anything reproduced identically on **both** | ||
| builds (pre-existing — note it, don't flag as a regression). | ||
| - Reproduce anything about to be marked blocking at least twice before confirming. |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Do not classify all concurrency and timing glitches as non-blocking.
A new intermittent race can break the happy flow, crash the application, or lose data. That satisfies the blocking rule. Limit the exemption to non-user-visible timing noise or behavior reproduced identically on both builds.
🧰 Tools
🪛 SkillSpector (2.4.4)
[warning] 174: [RA2] Session Persistence: Skill establishes unauthorized persistence across sessions via cron jobs, startup scripts, or state files. Session persistence allows an attacker to maintain access beyond the current interaction.
Remediation: Remove any persistence mechanisms (cron jobs, startup scripts, state files). Skills should not maintain state across sessions without explicit user consent.
(Rogue Agent (RA2))
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.claude/skills/release-review/SKILL.md around lines 260 - 266, Update the
“Not blocking” guidance in the Blocker rule to exempt only non-user-visible
timing noise and behavior reproduced identically on both builds. Ensure new
concurrency or timing races that break the happy flow, crash the application, or
cause data loss are classified as blocking and follow the existing reproduction
requirement.
| These six scenarios are written **version-agnostic**: the same procedure | ||
| runs unmodified against both the baseline release build and the current | ||
| `v1.0-dev` build for an A/B comparison, with the blocker rule (worse than | ||
| baseline in the happy flow, or data loss, is blocking; concurrency/glitches | ||
| and issues present on both builds are not) baked into each file's own | ||
| "Expected outcome" section. |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Make the release-review contract version-agnostic and internally consistent.
The index names v1.0-dev, while the scenarios use campaign-selected binaries and HEAD. Several pass criteria also assume this PR's fix direction. The DPNS scenario says a wrong-identity result is blocking, then exempts timing-sensitive races that reproduce intermittently. These rules can waive a real regression or classify it against the wrong artifact.
docs/gui-testing/README.md#L256-L261: define the development artifact once and limit non-blocking results to cosmetic or identical behavior.docs/gui-testing/scenarios/identity-key-recovery-migration.md#L117-L120: remove “introduced within this diff”; compare feature presence and behavior between selected builds.docs/gui-testing/scenarios/dapi-budget-resilience-after-resync.md#L91-L97: remove the assumption that baseline failure and development success are always expected.docs/gui-testing/scenarios/dpns-registration-flow.md#L97-L109: define precedence for intermittent wrong-identity results.docs/gui-testing/scenarios/error-banners-identity-home-actions.md#L104-L108: remove the reference to this PR's purpose.docs/gui-testing/scenarios/platform-shielded-availability-after-sync.md#L79-L82: remove the assumption that this PR fixes the baseline behavior.
📍 Affects 6 files
docs/gui-testing/README.md#L256-L261(this comment)docs/gui-testing/scenarios/identity-key-recovery-migration.md#L117-L120docs/gui-testing/scenarios/dapi-budget-resilience-after-resync.md#L91-L97docs/gui-testing/scenarios/dpns-registration-flow.md#L97-L109docs/gui-testing/scenarios/error-banners-identity-home-actions.md#L104-L108docs/gui-testing/scenarios/platform-shielded-availability-after-sync.md#L79-L82
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@docs/gui-testing/README.md` around lines 256 - 261, Make the release-review
rules version-agnostic and consistent across all listed scenarios: in
docs/gui-testing/README.md lines 256-261, define the development artifact once
and allow non-blocking outcomes only for cosmetic differences or behavior
identical to baseline; in
docs/gui-testing/scenarios/identity-key-recovery-migration.md lines 117-120,
compare feature presence and behavior between the selected builds without
referring to changes introduced by this diff; in
docs/gui-testing/scenarios/dapi-budget-resilience-after-resync.md lines 91-97,
remove the assumption that baseline failure and development success are
required; in docs/gui-testing/scenarios/dpns-registration-flow.md lines 97-109,
establish that intermittent wrong-identity results remain blocking despite
timing sensitivity; in
docs/gui-testing/scenarios/error-banners-identity-home-actions.md lines 104-108
and docs/gui-testing/scenarios/platform-shielded-availability-after-sync.md
lines 79-82, remove references assuming this PR fixes the behavior.
| 1. **Baseline Platform Info check.** Open Platform Info (Developer/Advanced | ||
| view). Record the exact "Current Epoch Information" text, in particular | ||
| whether the fee-multiplier line reads "unavailable" or states it is "a | ||
| fixed value, not read from the network" (or neither, if built against a | ||
| version predating both #936 and #950). |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Use a build-neutral name for the first Platform Info check.
The procedure runs unchanged against both builds, but this step is named “Baseline Platform Info check.” During the development run, that label assigns the observation to the wrong artifact. Rename it to “Initial Platform Info check.”
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@docs/gui-testing/scenarios/dapi-budget-resilience-after-resync.md` around
lines 44 - 48, Rename the procedure step heading from “Baseline Platform Info
check” to “Initial Platform Info check” while preserving its existing Platform
Info instructions and observations.
| 5. **Check the logs.** Search `det.log`/`det-stderr.log` for | ||
| `DapiAllAddressesExhausted` or similar rate-limit/address-pool exhaustion | ||
| markers appearing near the resync cycles from step 2, independent of | ||
| whether the UI showed an error. |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
Document the actual diagnostic log source.
Both setups redirect process output to $LOG, but these steps only name det.log and det-stderr.log without a path or confirmation that those files exist. Search $LOG as well, or document the guaranteed DET log location. Otherwise the procedure can miss DapiAllAddressesExhausted markers or panic output.
docs/gui-testing/scenarios/dapi-budget-resilience-after-resync.md#L63-L66: specify the path for the DAPI exhaustion logs.docs/gui-testing/scenarios/error-banners-identity-home-actions.md#L65-L68: specify the path for crash and panic logs.
📍 Affects 2 files
docs/gui-testing/scenarios/dapi-budget-resilience-after-resync.md#L63-L66(this comment)docs/gui-testing/scenarios/error-banners-identity-home-actions.md#L65-L68
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@docs/gui-testing/scenarios/dapi-budget-resilience-after-resync.md` around
lines 63 - 66, Update the log-checking instructions in
docs/gui-testing/scenarios/dapi-budget-resilience-after-resync.md lines 63-66 to
explicitly search the guaranteed $LOG path, or document the verified DET log
path, for DapiAllAddressesExhausted markers. Also update
docs/gui-testing/scenarios/error-banners-identity-home-actions.md lines 65-68 to
specify the actual path used to locate crash and panic output, ensuring both
procedures inspect the process-output destination.
| **Run against BOTH builds with this identical procedure** — the baseline and | ||
| development binaries selected for the current campaign (record their exact | ||
| SHAs in that campaign's own artifacts, not here) — do not skip or alter | ||
| steps between runs. Record what actually happens on each build; do not | ||
| presuppose which behaviors are "the fix." |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
rg -n -C 3 'fixture|snapshot|reprovision|reset|isolated data|DATADIR|baseline|development' \
.claude/skills/release-review/SKILL.md \
docs/gui-testing/scenariosRepository: dashpay/dash-evo-tool
Length of output: 50379
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== Scenario file sizes =="
for f in \
docs/gui-testing/scenarios/identity-key-recovery-migration.md \
docs/gui-testing/scenarios/dapi-budget-resilience-after-resync.md \
docs/gui-testing/scenarios/dpns-registration-flow.md \
docs/gui-testing/scenarios/error-banners-identity-home-actions.md \
docs/gui-testing/scenarios/platform-shielded-availability-after-sync.md \
docs/gui-testing/scenarios/wallet-max-send-asset-lock.md
do
wc -l "$f"
done
echo "== Procedures around state-mutating actions =="
python3 - <<'PY'
import pathlib, re
files = [
"docs/gui-testing/scenarios/identity-key-recovery-migration.md",
"docs/gui-testing/scenarios/dapi-budget-resilience-after-resync.md",
"docs/gui-testing/scenarios/dpns-registration-flow.md",
"docs/gui-testing/scenarios/error-banners-identity-home-actions.md",
"docs/gui-testing/scenarios/platform-shielded-availability-after-sync.md",
"docs/gui-testing/scenarios/wallet-max-send-asset-lock.md",
]
for f in files:
lines = pathlib.Path(f).read_text().splitlines()
print(f"\n--- {f} ---")
for i, line in enumerate(lines, 1):
if re.search(r"(restore|key|import|identity|profile|deposit|send|max|top-up|username|legacy|SPV|sync)", line, re.I):
lo=max(1, i-2); hi=min(len(lines), i+3)
print(f"lines {lo}-{hi}:")
for n in range(lo, hi+1):
print(f"{n:4}: {lines[n-1]}")
PY
echo "== Parent skill block on live network state =="
python3 - <<'PY'
from pathlib import Path
skill=Path(".claude/skills/release-review/SKILL.md").read_text()
for marker in [
"Isolated data directories give the two builds separate local state",
"Shared fixtures are not automatically restored",
"fixture",
"baseline",
"development",
]:
idx = skill.find(marker)
print(f"\nIndex for {marker!r}: {idx}")
if idx >= 0:
print(skill[max(0, idx-500):idx+800])
PYRepository: dashpay/dash-evo-tool
Length of output: 50379
Add the same fixture-restoration requirement here as in the parent skill.
A fresh DATADIR isolates app-local output only, not shared live-network state. For both baseline and new builds, require each mutating scenario to launch from equivalent funded identity/wallet state or restore recorded starting balances/deposits/identities before comparing results.
📍 Affects 6 files
docs/gui-testing/scenarios/identity-key-recovery-migration.md#L16-L20(this comment)docs/gui-testing/scenarios/dapi-budget-resilience-after-resync.md#L16-L18docs/gui-testing/scenarios/dpns-registration-flow.md#L18-L20docs/gui-testing/scenarios/error-banners-identity-home-actions.md#L17-L19docs/gui-testing/scenarios/platform-shielded-availability-after-sync.md#L14-L16docs/gui-testing/scenarios/wallet-max-send-asset-lock.md#L18-L21
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@docs/gui-testing/scenarios/identity-key-recovery-migration.md` around lines
16 - 20, In docs/gui-testing/scenarios/identity-key-recovery-migration.md lines
16-20, add the parent skill’s fixture-restoration requirement: every mutating
scenario must start each baseline and development run with equivalent funded
identity/wallet state, or restore recorded starting balances, deposits, and
identities before comparison. Apply the same requirement in
docs/gui-testing/scenarios/dapi-budget-resilience-after-resync.md lines 16-18,
docs/gui-testing/scenarios/dpns-registration-flow.md lines 18-20,
docs/gui-testing/scenarios/error-banners-identity-home-actions.md lines 17-19,
docs/gui-testing/scenarios/platform-shielded-availability-after-sync.md lines
14-16, and docs/gui-testing/scenarios/wallet-max-send-asset-lock.md lines 18-21.
| - A voting key stored directly on the identity's own record (rather than a | ||
| separate voting identity) has a documented residual on HEAD: saving/ | ||
| removing it by hand can affect a same-numbered voting key on a linked | ||
| voting identity. After doing so, re-open the keys list and check every key | ||
| still reads as expected before treating anything odd here as a new | ||
| regression. |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major
Do not waive key-collision behavior because it is a HEAD residual.
Step 8 explicitly tests same-numbered-key collisions, and the scenario makes data loss blocking. This note tells the tester to treat affected behavior on HEAD as expected. That can hide a new regression or key loss. Compare the observed behavior on the selected build SHAs, and store any confirmed exception in campaign artifacts.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@docs/gui-testing/scenarios/identity-key-recovery-migration.md` around lines
129 - 134, Update the residual note in the identity key recovery migration
scenario so same-numbered voting-key collisions remain blocking and are not
waived as expected HEAD behavior. In the guidance around Step 8, instruct
testers to compare observed behavior across the selected build SHAs and record
any confirmed exception in campaign artifacts before treating it as an
established residual.
thepastaclaw
left a comment
thepastaclaw
left a comment
There was a problem hiding this comment.
Preliminary review — Codex only
Six of the seven prior findings are fixed, including canonical-remote selection, exact-SHA worktrees, GUI provisioning, regression attribution, reusable scenario references, and field-specific validation. The remaining shared-state gap and a newly confirmed stale development-ref resolution can still invalidate the advertised A/B comparison; annotated-tag verification and the identity-fixture preflight inventory also need correction.
Source: reviewers gpt-5.6-sol (general); verifier gpt-5.6-sol.
Validated blockers were found in the Codex precheck. Sonnet is deferred until a fresh Codex revalidation clears the blocker gate.
Review provenance
- Codex reviewers:
gpt-5.6-sol— general (completed) - Verifier:
gpt-5.6-sol— verifier - Sonnet: not run (deferred by blocker gate)
🔴 2 blocking | 🟡 2 suggestion(s)
1 carried-forward finding(s) already raised on this PR; not re-posting as new inline comments.
🤖 Prompt for all review comments with AI agents
These findings are from an automated code review. Verify each finding against the current code and only fix it if needed.
In `.claude/skills/release-review/SKILL.md`:
- [BLOCKING] .claude/skills/release-review/SKILL.md:121-128: Fetch the canonical development branch before resolving its SHA
The procedure freezes `<canonical-remote>/<base-branch>` without first refreshing that remote-tracking ref, so it can review and build an obsolete development commit. At this exact head, `git rev-parse upstream/v1.0-dev` returns `2ee9fa64898fcb34d382f11ccea91eecdfe4439f`, while `git ls-remote upstream refs/heads/v1.0-dev` returns the current canonical head `39b399281efbce6fc86a7425a973aca8e094acb0`. The direct-URL alternative from lines 103-104 is also incompatible with the documented `git rev-parse <canonical-remote>/<base-branch>` command because a URL is not a local ref namespace. Fetch the branch into an explicit local ref, or obtain its SHA with `git ls-remote`, fetch that object locally, and then freeze the verified SHA for every later phase.
- [SUGGESTION] .claude/skills/release-review/SKILL.md:112-117: Compare the peeled commit when verifying annotated release tags
The local command reports the tag's peeled commit with `git rev-parse <tag>^{commit}`, but the documented `git ls-remote --tags <canonical-remote> <tag>` command reports the tag object SHA for an annotated tag. For `v1.0.0-weekly.20260731`, those values are `51de95902013f743e788850bb0e60689c5832776` and `8671835d105909a2b22cac513402d141c1c46023` respectively, so a correct annotated tag appears to diverge. Query and compare `refs/tags/<tag>^{}` for the peeled remote commit, or separately compare the local and remote tag-object SHAs and their peeled commit SHAs.
- [SUGGESTION] .claude/skills/release-review/SKILL.md:64-74: Include the identity fixture in the preflight inventory
The section says it lists the fixture variable names used by the GUI scenarios, but it omits `E2E_IDENTITY_ID`, which `docs/gui-testing/scenarios/dapi-budget-resilience-after-resync.md` uses for the identity top-up check. The cheap preflight can therefore pass without confirming that the required identity is available. Add this variable to the inventory and require either a live identity lookup or an explicit decision to create the ad hoc identity during provisioning.
| dedicated long-lived dev branch instead. Resolve it once: | ||
| `git rev-parse <canonical-remote>/<base-branch>`, and treat that output as **the development SHA** | ||
| for the rest of this run — every later phase (the diff range, Phase 2b's second worktree, | ||
| screenshots, the final report) uses this exact SHA. Never substitute a bare `HEAD` for it (that's | ||
| the invoking checkout's HEAD, not necessarily the development branch's — wrong whenever this | ||
| procedure is run from a feature/PR checkout, which is common) and never re-resolve the branch ref | ||
| later in the run, since it can move while you're still working. Compute the range: | ||
| `git log <tag>..<development-sha> --oneline`. |
There was a problem hiding this comment.
🔴 Blocking: Fetch the canonical development branch before resolving its SHA
The procedure freezes <canonical-remote>/<base-branch> without first refreshing that remote-tracking ref, so it can review and build an obsolete development commit. At this exact head, git rev-parse upstream/v1.0-dev returns 2ee9fa64898fcb34d382f11ccea91eecdfe4439f, while git ls-remote upstream refs/heads/v1.0-dev returns the current canonical head 39b399281efbce6fc86a7425a973aca8e094acb0. The direct-URL alternative from lines 103-104 is also incompatible with the documented git rev-parse <canonical-remote>/<base-branch> command because a URL is not a local ref namespace. Fetch the branch into an explicit local ref, or obtain its SHA with git ls-remote, fetch that object locally, and then freeze the verified SHA for every later phase.
source: ['codex']
| 3. **Force-sync the tag from `<canonical-remote>` before trusting it**: | ||
| `git fetch <canonical-remote> "+refs/tags/<tag>:refs/tags/<tag>"`, then | ||
| `git rev-parse <tag>^{commit}`. **Confirmed gotcha**: a local tag ref can silently diverge from the | ||
| remote (stale from an earlier fetch/session) and point at an *ancestor* commit, producing a diff | ||
| range that's wrong (larger than reality) without any error. Always re-verify against | ||
| `git ls-remote --tags <canonical-remote> <tag>` before computing any diff range from a tag. |
There was a problem hiding this comment.
🟡 Suggestion: Compare the peeled commit when verifying annotated release tags
The local command reports the tag's peeled commit with git rev-parse <tag>^{commit}, but the documented git ls-remote --tags <canonical-remote> <tag> command reports the tag object SHA for an annotated tag. For v1.0.0-weekly.20260731, those values are 51de95902013f743e788850bb0e60689c5832776 and 8671835d105909a2b22cac513402d141c1c46023 respectively, so a correct annotated tag appears to diverge. Query and compare refs/tags/<tag>^{} for the peeled remote commit, or separately compare the local and remote tag-object SHAs and their peeled commit SHAs.
source: ['codex']
| separate test binary, not this GUI). As of this writing, the scenarios under | ||
| `docs/gui-testing/scenarios/` use: | ||
|
|
||
| - `E2E_MN_PROTX_HASH`: confirm the value you intend to use is a currently-registered | ||
| masternode/evonode before relying on it in a scenario — a stale hash surfaces as a typed | ||
| `MasternodeNotFound` error the first time something touches it in the GUI. Check via an SDK/CLI | ||
| lookup or a quick platform-explorer query first, since nothing enforces this automatically. | ||
| - `E2E_WALLET_MNEMONIC`: confirm the wallet is actually funded on testnet *right now* (a live balance | ||
| check, not "it was funded last time"). If empty, get a faucet drop (a testnet faucet — web, CLI, | ||
| or whatever this project's e2e docs point at) or a fresh funded fixture BEFORE starting Phase 4 — | ||
| don't discover this after a long SPV sync. |
There was a problem hiding this comment.
🟡 Suggestion: Include the identity fixture in the preflight inventory
The section says it lists the fixture variable names used by the GUI scenarios, but it omits E2E_IDENTITY_ID, which docs/gui-testing/scenarios/dapi-budget-resilience-after-resync.md uses for the identity top-up check. The cheap preflight can therefore pass without confirming that the required identity is available. Add this variable to the inventory and require either a live identity lookup or an explicit decision to create the ad hoc identity during provisioning.
source: ['codex']
3 participants
TL;DR
Adds a reusable QA procedure for reviewing a nightly/weekly build before it ships, plus six ready-to-run test scenarios covering the areas most likely to regress in the current development cycle. No application code changes.
User story
As a maintainer preparing a release, I want a repeatable, written-down procedure for comparing the last published pre-release against the current development branch — what changed, whether a code review turns up anything, and whether the GUI is at least as good as before — so that release QA doesn't have to be reinvented (or skipped) each time.
Scenario
Actual behavior:
docs/gui-testing/scenarios/only contained a template — no real scenarios existed yet, and there was no written procedure tying changelog verification, code review, and GUI regression testing together into one release-readiness pass.Expected behavior: A documented, environment-independent procedure (
.claude/skills/release-review/) any Claude Code session can follow with standard tools, plus six version-agnostic GUI scenarios (the same steps run against both the old and new build) covering identity key recovery, wallet Max-send/asset-lock behavior, DPNS registration, error banners and Identity Home actions, shielded-feature availability after sync, and DAPI request-budget resilience across repeated resyncs.Detailed discussion
.claude/skills/release-review/SKILL.md: procedure for baseline identification, a verified (not just CHANGELOG-trusted) changelog, a scoped code review, building both the old and new binaries for a true A/B comparison, GUI regression testing with an explicit blocking-vs-non-blocking rule, and a self-contained visual changelog (screenshot pairs) as the human-reviewable deliverable. Written to make no assumptions about the host OS, file layout, or which optional tooling/plugins are installed — every accelerator is named as optional, not required.docs/gui-testing/scenarios/*.md: six new scenario files following the existingTEMPLATE.mdstructure, each scoped to a specific set of recent PRs/issues and written so the identical procedure applies to both builds under test.docs/gui-testing/README.md: scenario index table populated (previously a placeholder row) and a short note on the version-agnostic convention these scenarios follow.Testing
Documentation/process-only change — no application code touched. The scenarios themselves have been exercised manually against real builds during the procedure's development; no automated test suite applies here.
🤖 Co-authored by Claudius the Magnificent AI Agent
Summary by CodeRabbit
Documentation
Tests