nik/bitchat-android
1
0
Fork 0
mirror of https://github.com/permissionlesstech/bitchat-android.git synced 2026-08-08 06:46:11 +00:00
Code Issues Packages Projects Releases Wiki Activity

Merge pull request #817 from permissionlesstech/codex/android-ui-visual-review-skill

Browse Source
This commit is contained in:
callebtc 2026-07-29 15:21:58 +02:00 committed by GitHub
commit 56f91a96b7
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
10 changed files with 1392 additions and 0 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

255
.agents/skills/android-ui-visual-review/SKILL.md Normal file
View File

@ -0,0 +1,255 @@
---
name: android-ui-visual-review
description: Analyze an Android pull request, branch, commit, or patch for user-visible changes and produce reproducible before/after screenshots from isolated builds. Use this skill whenever a user asks for PR screenshots, branch UI comparisons, visual regression evidence, Compose before/after captures, populated message-state screenshots, responsive/theme/locale comparisons, or GitHub comments containing Android UI evidence—even if they only say “show me what changed.” Also use it to determine and document that a suspected UI PR has no visual delta. Do not use it for implementing a new UI, ordinary code review without visual evidence, or physical mesh validation.
compatibility: Requires git, the Android SDK and emulator, adb, Java/Gradle, Python 3, and gh for pull-request resolution or publishing.
---
# Android UI Visual Review
Turn a PR or branch into trustworthy visual evidence. The comparison is useful
only when the before and after builds use the correct commits, Android runtime,
viewport, app state, and navigation path.
## Collect the two user choices
Before starting, resolve:
1. **Target** — a PR URL/number or a branch/commit. If it is missing, ask for it.
For a branch, also ask for the intended base when it cannot be inferred
safely; otherwise default to the repository's `main`.
2. **Publishing** — for a PR target, ask whether the final screenshots and
findings should stay local or be posted as a PR comment. Do not perform any
GitHub write unless the user explicitly chooses publishing. A prior explicit
request such as “post these to the PR” already answers this question.
Do not block on publishing preference while doing read-only analysis if the user
has not answered yet. Keep the local workflow useful on its own.
## Read the routed guidance
- Read [references/analysis-playbook.md](references/analysis-playbook.md) for
every run. It explains how to map a diff to screens, states, and a capture
matrix.
- Read [references/fixture-recipes.md](references/fixture-recipes.md) whenever
the affected screen needs messages, peers, channels, nicknames, settings,
permissions, locale, theme, onboarding state, or another non-empty fixture.
- Read [references/github-publishing.md](references/github-publishing.md) only
when the user has opted into a PR comment.
## Create an isolated review session
Never switch the user's active checkout between before and after revisions.
Run from the repository root:
```sh
.agents/skills/android-ui-visual-review/scripts/create_review_worktree.sh \
--target "<PR URL, PR number, branch, or commit>"
```
For a branch with a non-default base:
```sh
.agents/skills/android-ui-visual-review/scripts/create_review_worktree.sh \
--target "<branch>" \
--base "<base ref>"
```
The script fetches a PR head when needed, computes the **actual merge-base**,
creates a detached temporary worktree at the before SHA, and prints:
- session directory
- worktree path
- artifact directory
- before and after SHAs
- PR number/base metadata when applicable
Keep artifacts outside the worktree so checkouts cannot remove them. The script
may symlink the ignored `local.properties` into the temporary worktree; never
publish it or quote its contents.
If a review session already exists and its SHAs are verified, reuse it. Do not
create a second worktree for the same run.
## Establish the visual contract
Use the actual diff, not the PR title, to determine what should be visible.
1. Record `git diff --stat`, `--name-status`, and the focused diff between the
before and after SHAs.
2. Trace changed UI symbols to their composable/activity, state source, entry
point, and prerequisites.
3. Separate direct visual changes from indirect ones such as dynamic color,
locale recreation, launcher resources, default data, or backend state shown
by an otherwise unchanged screen.
4. Produce a local capture matrix before building. Each row should define:
screen, navigation path, fixture, logical width, theme, locale, permissions,
and what difference is expected.
5. Include a control state where the UI should remain unchanged when that helps
distinguish intentional degradation from a regression.
When the diff contains no UI/resource/state-to-UI change, say so. If the user
asked for a screenshot for every target, capture the nearest affected surface
before and after and label the expected result **no visual delta**. Do not invent
a UI claim for a behavioral fix.
## Use the newest stable Android runtime
Prefer an emulator for repeatable UI evidence. At run time:
1. Inspect installed SDK/system images and current official Android release
information. Choose the newest **stable** API; do not silently use a preview.
2. Create a dedicated AVD and isolated data directory when possible. If current
command-line tools cannot create a newly versioned image (for example an
extension image), a verified `-sysdir` override with an isolated data
directory is acceptable.
3. After boot, record guest properties rather than trusting the AVD name:
```sh
adb -s "$ANDROID_REVIEW_SERIAL" shell getprop ro.build.version.release
adb -s "$ANDROID_REVIEW_SERIAL" shell getprop ro.build.version.sdk
adb -s "$ANDROID_REVIEW_SERIAL" shell getprop ro.build.version.security_patch
adb -s "$ANDROID_REVIEW_SERIAL" shell wm size
adb -s "$ANDROID_REVIEW_SERIAL" shell wm density
```
Use an explicit emulator serial for every ADB command when any physical device
is also connected. Never put serials, device names, local paths, IP addresses,
or other machine identifiers into reports or GitHub comments.
Use a physical device only when the affected UI depends on hardware that the
emulator cannot reproduce. Ask before changing or clearing a physical device.
This skill does not replace Mesh Lab: if the diff changes mesh, transport,
crypto, service, or physical peer behavior, use the `mesh-lab` skill separately
before claiming the behavior works.
## Build and capture the before state
The worktree starts at the before SHA.
1. Build the debug APK with `./gradlew assembleDebug`.
2. Install the ABI-matching APK with `adb install -r`.
3. Complete stable prerequisites such as onboarding and permissions.
4. Apply the fixture from the capture matrix.
5. Navigate using semantic/UI-automator evidence where possible. Use coordinate
taps only after inspecting the current screen, and keep coordinates local.
6. Capture every matrix row with a descriptive name:
```text
before-<surface>-<state>-<width>-<theme>.png
```
Use `adb exec-out screencap -p` so the PNG is written directly to the artifact
directory. Inspect every screenshot immediately; a successful command is not
proof that the intended screen was visible.
## Build and capture the after state
Before switching commits:
- Preserve the artifact directory outside the worktree.
- Preserve only intentional app state.
- Record any temporary debug fixture patch.
Checkout the recorded after SHA in the detached worktree, reapply the same
debug-only fixture if needed, build, and install with `-r` when state
preservation is part of the comparison.
Replay the same navigation and capture matrix. Name files with the matching
`after-` prefix. If reinstalling cannot preserve the state, replay the fixture
from its recorded inputs rather than comparing different states.
For responsive captures, record logical width in dp. On a fixed-pixel emulator,
changing density is acceptable when the calculation is documented:
```text
density = physical_width_px × 160 / desired_width_dp
```
Restore the original density/theme/locale after the matrix is complete.
## Use deterministic artificial data
Prefer existing debug hooks. When they cannot express the visual state, add the
smallest temporary command to
`app/src/debug/java/com/bitchat/android/testhook/TestHookDriver.kt`.
The fixture must:
- stay under `src/debug`
- use synthetic names/content and deterministic IDs
- use the current mesh peer ID for self-authored messages so alignment logic is
exercised correctly
- use a fixed fixture epoch passed to both builds
- return structured success data through the existing test-hook result file
- be logically identical in before and after builds
After the final capture, remove the temporary fixture with a focused patch and
verify that the review worktree has no tracked modifications. Never commit or
publish the fixture unless the user separately asks to productize it.
## Validate and report
Create `capture-manifest.json` in the artifact directory using
[assets/capture-manifest.example.json](assets/capture-manifest.example.json) as
the shape. Use paths relative to the manifest and omit device selectors and
local absolute paths.
Validate it:
```sh
python3 \
.agents/skills/android-ui-visual-review/scripts/validate_capture_manifest.py \
"<artifact-directory>/capture-manifest.json"
```
Write a Markdown report next to the manifest with:
- target and exact before/after SHAs
- verified Android release/API/security patch and viewport
- concise code analysis
- visual findings, including intentional non-changes
- side-by-side before/after tables
- fixture disclosure
- limitations and any hardware behavior not exercised
End the local run only after:
- every manifest image exists and is a valid PNG
- each before/after pair has matching pixel dimensions
- every screenshot has been visually inspected
- the worktree has no tracked fixture changes
- the user's original checkout remains untouched
Leave the review session and artifacts available for later inspection unless
the user asks for cleanup.
## Optionally publish to the PR
Only after explicit user approval, follow
[references/github-publishing.md](references/github-publishing.md).
The PR comment should contain:
- Android capture environment
- detected visual changes
- clear before/after labels
- all requested screenshots
- limitations such as “no visual delta” or “hardware race not reproduced”
Use `gh` for all GitHub reads and writes. Verify the posted comment by reading it
back and counting the expected image embeds. Do not expose local paths or
machine identifiers, do not override Git author/committer identity, and do not
push screenshot files to a source branch unless the user separately authorizes
that repository change.
## Final handoff
Give the user:
- report and artifact links
- screenshot count
- one-line result per surface
- PR comment URL when published
- explicit statement that the original checkout was not modified
- any remaining coverage limitation

32
.agents/skills/android-ui-visual-review/assets/capture-manifest.example.json Normal file
View File

@ -0,0 +1,32 @@
{
"target": "PR #000 or branch-name",
"before_sha": "40-character merge-base SHA",
"after_sha": "40-character target SHA",
"environment": {
"android_release": "stable release number",
"android_sdk": "API number",
"security_patch": "YYYY-MM-DD",
"resolution_px": "1080x1920",
"default_density_dpi": 420
},
"captures": [
{
"id": "surface-state-411dp-light",
"surface": "Human-readable surface",
"state": "Synthetic fixture and navigation state",
"expected_change": "Specific visual hypothesis",
"before": "before-surface-state-411dp-light.png",
"after": "after-surface-state-411dp-light.png"
}
],
"extras": [
{
"id": "after-expanded-menu",
"role": "after-detail",
"file": "after-expanded-menu.png"
}
],
"limitations": [
"Static screenshots do not prove hardware behavior."
]
}

20
.agents/skills/android-ui-visual-review/assets/pr-comment-template.md Normal file
View File

@ -0,0 +1,20 @@
<!-- android-ui-visual-review:TARGET:AFTER_SHA -->
## Android UI verification
Captured from clean before/after debug builds on **Android ANDROID_RELEASE
(API ANDROID_SDK)**. The before build is this target's actual merge-base.
### Visual changes detected
- FINDING_ONE
- FINDING_TWO
- LIMITATION_OR_INTENTIONAL_NON_CHANGE
### SURFACE_OR_STATE
| Before | After |
|---|---|
| ![Before: ACCESSIBLE_DESCRIPTION](BEFORE_IMAGE_URL) | ![After: ACCESSIBLE_DESCRIPTION](AFTER_IMAGE_URL) |
Fixture disclosure: FIXTURE_DESCRIPTION_OR_NONE.

41
.agents/skills/android-ui-visual-review/evals/evals.json Normal file
View File

@ -0,0 +1,41 @@
{
"skill_name": "android-ui-visual-review",
"evals": [
{
"id": 1,
"prompt": "Review PR #813 and make before/after screenshots of its Android UI changes. Keep everything local; do not comment on GitHub.",
"expected_output": "The agent creates an isolated worktree, compares the PR head with its actual merge-base, discovers the header width thresholds, captures a wide control plus the changed compact widths on the newest stable Android emulator, and produces a validated local report without any GitHub write.",
"files": [],
"expectations": [
"Uses the actual PR merge-base rather than current main or head~1.",
"Creates a capture matrix that includes a wide control and every changed width threshold.",
"Populates joined-channel state so the conditional count is visible before testing degradation.",
"Keeps artifacts local and performs no GitHub write."
]
},
{
"id": 2,
"prompt": "Compare branch feature/message-redesign against main. The empty chat will not show the change, so give me populated light and dark screenshots and also show any fresh-install nickname change.",
"expected_output": "The agent traces message rendering and nickname generation, adds a temporary deterministic debug-only fixture with received/self short/wrapped messages, applies equivalent state to both builds, captures light/dark and fresh nickname states, removes the fixture, and validates paired PNGs.",
"files": [],
"expectations": [
"Uses synthetic deterministic messages with the real local peer ID for self classification.",
"Uses one fixed fixture epoch and identical logical data in both builds.",
"Captures both light and dark populated message states plus the fresh nickname state.",
"Removes temporary debug fixture changes before handoff."
]
},
{
"id": 3,
"prompt": "This hotspot PR looks UI-related, but inspect it rather than assuming. Capture before/after and, after you show the evidence, post the screenshots and findings to the PR.",
"expected_output": "The agent determines whether the diff actually changes UI, captures the nearest affected hotspot surface even if equality is expected, labels behavioral limitations, asks or recognizes explicit publishing authorization, uploads safe screenshots without a source branch, posts through gh, and verifies the comment.",
"files": [],
"expectations": [
"Does not invent a visual delta when the diff is backend-only.",
"Labels the screenshots as an expected no-visual-change comparison.",
"Does not claim that a static screenshot proves the hotspot ownership race.",
"Posts only after explicit authorization and verifies image count and comment URL."
]
}
]
}

152
.agents/skills/android-ui-visual-review/references/analysis-playbook.md Normal file
View File

@ -0,0 +1,152 @@
# Diff-to-screen analysis playbook
Use this reference to turn a code diff into a minimal but complete screenshot
matrix.
## Resolve the correct comparison
For a PR, “before” is the merge-base of the fetched PR head and its configured
base branch. It is not necessarily the PR's current `baseRefOid`, the local
`main`, or the commit immediately preceding the head.
For a branch or commit, confirm the intended base. Compute the merge-base after
fetching both sides.
Record both SHAs before any checkout. Build each SHA rather than attempting to
reverse selected files on a single build.
## Triage the diff in layers
Start broad:
```sh
git diff --stat "$BEFORE_SHA" "$AFTER_SHA"
git diff --name-status "$BEFORE_SHA" "$AFTER_SHA"
git diff "$BEFORE_SHA" "$AFTER_SHA" -- app/src/main app/src/debug
```
Then classify changed files.
| Diff area | Likely visual impact |
|---|---|
| `ui/*.kt`, composables, modifiers, layouts | Direct screen/layout change |
| `ui/theme/*`, colors, shapes, typography | Cross-screen theme change |
| `res/values*`, drawables, mipmaps | Text, locale, icon, launcher, or palette |
| Manifest locale/theme/activity metadata | System or activity presentation |
| `DataManager`, preferences, defaults | Fresh-install/default-state change |
| `AppStateStore`, ViewModel/state flows | UI changes only after specific state |
| Service/transport/backend only | Usually no static UI delta; trace exposed state |
| `src/debug` or Android tests | Fixture/test mechanism, not production UI |
Do not stop at filenames. Search every changed public symbol and resource:
```sh
rg -n "<ChangedSymbol|resource_name>" app/src
```
Trace in both directions:
- Who calls or renders this code?
- Which state branch selects it?
- What user action reaches it?
- What permissions, onboarding, peers, channels, messages, theme, locale, or
width are required?
- Does the change affect an empty screen, only populated state, or both?
## Repository UI surface map
These are orientation points, not a substitute for inspecting the current
revision:
| Surface | Starting points |
|---|---|
| App launch/navigation/permissions | `MainActivity.kt`, `ui/ChatScreen.kt` |
| Top bar, nickname, peer/channel/location controls | `ui/ChatHeader.kt` |
| Message rows, bubbles, timestamps, media | `ui/MessageComponents.kt` |
| App state consumed by Compose | `services/AppStateStore.kt`, ViewModels |
| Default nickname/preferences | `ui/DataManager.kt` |
| About and Settings | `ui/AboutSheet.kt` |
| Location/geohash/channel controls | `ui/LocationChannelsSheet.kt` |
| Hotspot UI | `hotspot/HotspotActivity.kt` |
| Dynamic/fallback colors and shapes | `ui/theme/Theme.kt`, `ThemePreference.kt` |
| Debug ADB hooks | `src/debug/.../testhook/TestHookReceiver.kt`, `TestHookDriver.kt` |
Files and packages can move. Use `rg --files` and symbol search to re-establish
the current map at the target commits.
## Identify indirect UI changes
Some visual changes appear far away from the edited function:
- A nickname generator change is visible only after deleting or bypassing a
saved nickname.
- A Material You change appears only on Android 12+ and depends on the emulator
wallpaper/system palette.
- Locale selection may recreate the Activity and return to a different tab.
- A launcher background is not visible inside the running Activity.
- Message delivery status may appear only for self-authored private messages.
- A width policy may be invisible at the default device width.
- A backend ownership fix may produce no screenshot difference at all.
Write these as explicit hypotheses before capture. Each hypothesis needs either
a matrix row or a documented reason it cannot be shown statically.
## Build the capture matrix
Keep the matrix small enough to review but large enough to hit every changed
branch.
| Field | What to record |
|---|---|
| Surface | Human-readable screen/component |
| Entry path | Actions from launch to the target |
| Fixture | Messages, peer, channel, setting, or empty state |
| Platform | Android API feature needed, such as dynamic color |
| Width | Logical dp breakpoint |
| Theme | System/light/dark |
| Locale | System/default or selected locale |
| Expected before | Specific visual contract |
| Expected after | Specific visual contract |
| Control | State expected not to change, when useful |
Examples:
- Header crowding: 411 dp control, 380 dp middle breakpoint, 320 dp compact
breakpoint, with a joined channel so the count is actually present.
- Message bubbles: identical short/long received and self messages in light and
dark modes.
- Nickname prefix: fresh generated nickname in each build; explain that random
digits differ.
- Language picker: Settings before, Settings after, expanded menu, and one live
selection.
- Backend-only hotspot fix: the nearest hotspot surface before/after, explicitly
expecting equality.
## Distinguish visual proof from behavioral proof
A static screenshot can prove rendering, layout, labels, selected state, and
visible recreation. It cannot prove:
- foreign Wi-Fi group ownership
- BLE/Wi-Fi discovery or delivery
- Noise/identity correctness
- race avoidance
- background lifecycle behavior
- accessibility announcement content without an accessibility inspection
Name those limitations. Use relevant unit/instrumented tests or the repository's
Mesh Lab workflow separately.
## Compare carefully
Treat these as expected noise unless the PR changes them:
- status-bar clock
- battery/network indicator
- random nickname suffix
- dynamic palette when system wallpaper differs
- asynchronous peer counts
- animation frame
Stabilize or disclose the noise. Never describe it as a PR effect.

209
.agents/skills/android-ui-visual-review/references/fixture-recipes.md Normal file
View File

@ -0,0 +1,209 @@
# Deterministic UI fixture recipes
Read current code before applying any recipe. These patterns intentionally use
the debug-only test-hook path and should be adapted to the APIs present at both
comparison SHAs.
## Fixture principles
1. Exercise the UI's real state classification. A self message must carry the
peer identity that production code uses to decide `isSelf`; changing only the
displayed sender nickname can produce a false layout.
2. Use the same logical records in both builds: stable IDs, sender IDs, content,
ordering, and one fixed epoch.
3. Keep content synthetic and review-safe. Do not use real messages, contacts,
peer IDs, channel memberships, device information, or locations.
4. Return structured success data and verify it before capturing.
5. Keep additions under `app/src/debug`; remove them after capture.
## Prefer the existing test hook
Inspect:
- `app/src/debug/java/com/bitchat/android/testhook/TestHookReceiver.kt`
- `app/src/debug/java/com/bitchat/android/testhook/TestHookDriver.kt`
- `app/src/debug/AndroidManifest.xml`
The receiver accepts:
```sh
adb -s "$ANDROID_REVIEW_SERIAL" shell am broadcast \
-n com.bitchat.droid/com.bitchat.android.testhook.TestHookReceiver \
-a com.bitchat.droid.TEST_HOOK \
--es cmd "<command>" \
--es id "<unique-result-id>"
```
Read the result rather than trusting broadcast delivery:
```sh
adb -s "$ANDROID_REVIEW_SERIAL" shell run-as com.bitchat.droid \
cat "cache/testhook/results/<unique-result-id>.json"
```
Existing commands such as `set_nickname`, `broadcast_msg`, and state inspection
may already be sufficient.
## Public message fixture
When existing commands cannot create both received and self messages without a
second device, add a temporary `ui_fixture` command to `TestHookDriver`.
Adapt imports and constructor fields to the checked-out revision. The core shape
is:
```kotlin
private fun uiFixture(context: Context, intent: Intent): JSONObject {
val mesh = mesh(context)
val nickname = AppStateStore.nickname.value
.ifBlank { DataManager(context).loadNickname() }
val epochMs = intent.getLongExtra("fixture_epoch_ms", 1_800_000_000_000L)
listOf(
BitchatMessage(
id = "ui-fixture-received-short",
sender = "mara",
content = "Are you seeing this?",
timestamp = Date(epochMs),
senderPeerID = "ui-fixture-peer",
),
BitchatMessage(
id = "ui-fixture-received-wrap",
sender = "mara",
content = "The mesh stays readable even when a message wraps onto a second line.",
timestamp = Date(epochMs + 60_000),
senderPeerID = "ui-fixture-peer",
),
BitchatMessage(
id = "ui-fixture-self-short",
sender = nickname,
content = "Yep — testing the message layout.",
timestamp = Date(epochMs + 120_000),
senderPeerID = mesh.myPeerID,
),
BitchatMessage(
id = "ui-fixture-self-wrap",
sender = nickname,
content = "Short and long bubbles should align consistently.",
timestamp = Date(epochMs + 180_000),
senderPeerID = mesh.myPeerID,
),
).forEach(AppStateStore::addPublicMessage)
return ok("ui_fixture")
.put("messages", 4)
.put("fixture_epoch_ms", epochMs)
}
```
Why these details matter:
- received/self exercises both alignment branches
- short/wrapped content exercises intrinsic and capped width
- fixed IDs defeat accidental duplication
- a fixed epoch makes before/after timestamps comparable
- `mesh.myPeerID` exercises the real self-classification path
If `AppStateStore` moved packages or the message constructor changed, adapt only
the debug fixture. Do not modify production state logic to accommodate it.
App state is process-local. Launch the Activity before injection, inject after
the process is ready, and capture without force-stopping it.
## Private-message and delivery-status fixture
To inspect private-message bubbles or status placement:
- set the selected private peer in `AppStateStore`
- add messages using `addPrivateMessage`
- use the current local peer ID for self messages
- populate the delivery status explicitly when the changed component reads it
- mark read state consistently
Use a synthetic conversation ID such as `ui-fixture-private-peer`. Avoid writing
to persistent conversation storage unless persistence itself is being reviewed.
If persistence cannot be bypassed safely, use a unique deterministic fixture
conversation and disclose it in the report.
## Fresh default nickname
A saved preference hides generator changes. Add a temporary command that removes
only the nickname preference, invokes the revision's real generator, and updates
the in-memory store:
```kotlin
private fun resetNicknameFixture(context: Context): JSONObject {
context.getSharedPreferences("bitchat_prefs", Context.MODE_PRIVATE)
.edit()
.remove("nickname")
.commit()
val nickname = DataManager(context).loadNickname()
AppStateStore.setNickname(nickname)
return ok("reset_nickname_fixture").put("nickname", nickname)
}
```
First inspect `DataManager` to confirm the preference file/key. Do not use this
recipe blindly after storage migrations.
Random suffixes are expected to differ. Compare the changed prefix/pattern, not
the digits. Do not seed or replace the production generator merely to make the
screenshot deterministic.
## Channel/header state
Responsive header screenshots need every conditional item present.
- Join a synthetic channel such as `#review` through the normal UI or existing
debug state API.
- Confirm the joined count is visible at the control width before changing
density.
- Keep the same nickname, channel, peer count, and location state in both builds.
- Capture a wide control and every threshold changed by the diff.
Do not use a real geohash/location. Synthetic channel state is enough for header
crowding unless the actual geohash label is the feature under review.
## Theme and dynamic color
- Force light and dark through `adb shell cmd uimode night no|yes`.
- Record the system wallpaper/palette only as anonymous environment context.
- Keep the same emulator data directory between builds so Material You input is
identical.
- Capture both themes when theme code, containers, surfaces, or contrast changes.
- Restore the original mode after capture.
## Locale
Use the new in-app picker when that is the feature. Capture:
1. Settings before
2. Settings with the new row
3. expanded picker
4. one live language selection
Locale application may recreate the Activity and reset the selected tab. This is
expected; navigate again rather than assuming the selection failed. Restore
System default after capture.
## Onboarding and permissions
Complete onboarding/permissions once in the before build, then use `adb install
-r` for after when signatures and data schemas are compatible.
If state cannot be preserved, record and replay each action. Do not clear all app
data merely to change one preference. Never run `pm clear` on a physical device
without explicit authorization.
## Remove the fixture
Use a focused patch to remove only additions made for capture. Then check:
```sh
git diff -- app/src/debug
git status --short
```
The finished worktree may contain untracked artifacts only when artifacts were
intentionally stored there. It must not contain tracked fixture changes.

97
.agents/skills/android-ui-visual-review/references/github-publishing.md Normal file
View File

@ -0,0 +1,97 @@
# Publishing screenshots to a pull request
Follow this only after the user explicitly opts into a GitHub comment.
## Prepare a safe comment
The comment should contain:
- a unique hidden marker for idempotent verification
- verified Android release/API
- statement that before is the target's actual merge-base
- concise visual findings
- paired before/after image tables
- fixture disclosure when artificial state was used
- limitations and intentional non-changes
Do not include:
- local paths or usernames
- ADB serials, device names, peer IDs, addresses, or IPs
- real messages, contacts, locations, or account information
- build logs
- claims about physical behavior that screenshots do not prove
Scan the body locally for machine paths and selectors before posting.
Use [../assets/pr-comment-template.md](../assets/pr-comment-template.md) as the
starting structure, replacing every uppercase placeholder and adding one table
per comparison state.
## Upload image bytes without changing a source branch
Native `gh pr comment` accepts Markdown but not binary files. This skill bundles
an uploader that uses `gh api` to place images on a PR-scoped custom Git ref:
```sh
python3 \
.agents/skills/android-ui-visual-review/scripts/upload_pr_images.py \
--repo OWNER/REPO \
--pr NUMBER \
<ordered PNG files>
```
The helper:
- uploads only image bytes and basenames
- writes to `refs/uploads/issues/<NUMBER>`, outside `refs/heads/*`
- does not pass an author or committer override
- prints JSON containing stable GitHub blob URLs
- does not post a comment
This is a GitHub repository write even though it does not create a visible
branch. The user's approval to publish the screenshots authorizes this
PR-scoped storage. If repository policy rejects custom refs, use an authenticated
GitHub web attachment composer or ask the user for an approved image host. Do
not fall back to a source branch without separate authorization.
Use `--dry-run` first when validating new inputs:
```sh
python3 \
.agents/skills/android-ui-visual-review/scripts/upload_pr_images.py \
--dry-run \
--repo OWNER/REPO \
--pr NUMBER \
<ordered PNG files>
```
## Post through gh
Build the final Markdown body with the returned URLs. Keep before and after in
the same table row. Put extra after-state details, such as an expanded menu, in
a separate labeled table.
Post once:
```sh
gh pr comment NUMBER \
--repo OWNER/REPO \
--body-file "<validated-comment.md>"
```
Record the returned comment URL.
## Verify the side effect
Read the comment back using `gh api` or `gh pr view`. Verify:
- the unique marker is present
- image embed count equals the requested screenshot count
- no local paths or identifiers were included
- the returned URL belongs to the intended PR
If the posting command's outcome is ambiguous, query for the marker before
retrying. Do not create duplicate comments.
Report the comment URL to the user. Leave the PR-scoped image ref in place while
the comment depends on it; deleting the ref can eventually break the images.

201
.agents/skills/android-ui-visual-review/scripts/create_review_worktree.sh Executable file
View File

@ -0,0 +1,201 @@
#!/usr/bin/env bash
set -euo pipefail
usage() {
cat <<'EOF'
Create an isolated Android UI review worktree.
Usage:
create_review_worktree.sh --target <PR-URL|PR-NUMBER|BRANCH|COMMIT> [options]
Options:
--base <REF> Base for a branch/commit target (default: main)
--repo-root <DIR> Repository root (default: current repository)
-h, --help Show this help
The script never removes an existing worktree. It prints and writes session.env
with the before/after SHAs and artifact paths.
EOF
}
target=""
base_ref=""
repo_root="."
while [[ $# -gt 0 ]]; do
case "$1" in
--target)
target="${2:-}"
shift 2
;;
--base)
base_ref="${2:-}"
shift 2
;;
--repo-root)
repo_root="${2:-}"
shift 2
;;
-h|--help)
usage
exit 0
;;
*)
echo "error: unknown argument: $1" >&2
usage >&2
exit 2
;;
esac
done
if [[ -z "$target" ]]; then
echo "error: --target is required" >&2
exit 2
fi
if [[ "$target" == -* || "$base_ref" == -* ]]; then
echo "error: target and base refs cannot begin with '-'" >&2
exit 2
fi
repo_root="$(git -C "$repo_root" rev-parse --show-toplevel)"
cd "$repo_root"
target_kind="ref"
pr_number=""
base_name=""
head_ref=""
if [[ "$target" =~ ^[0-9]+$ ]]; then
target_kind="pr"
pr_number="${BASH_REMATCH[0]}"
elif [[ "$target" =~ ^https://github\.com/([^/]+)/([^/]+)/pull/([0-9]+)/?$ ]]; then
target_kind="pr"
pr_number="${BASH_REMATCH[3]}"
url_repo="${BASH_REMATCH[1]}/${BASH_REMATCH[2]}"
current_repo="$(gh repo view --json nameWithOwner --jq .nameWithOwner)"
if [[ "${url_repo,,}" != "${current_repo,,}" ]]; then
echo "error: PR URL targets $url_repo but this checkout is $current_repo" >&2
exit 2
fi
fi
resolve_target_ref() {
local ref="$1"
local resolved=""
if git show-ref --verify --quiet "refs/heads/${ref}"; then
git rev-parse --verify "refs/heads/${ref}^{commit}"
return
fi
if [[ "$ref" == refs/* || "$ref" == *"~"* || "$ref" == *"^"* ]] &&
resolved="$(git rev-parse --verify "${ref}^{commit}" 2>/dev/null)"; then
printf '%s\n' "$resolved"
return
fi
if git check-ref-format --branch "$ref" >/dev/null 2>&1; then
if git fetch origin "$ref" >/dev/null 2>&1; then
git rev-parse --verify 'FETCH_HEAD^{commit}'
return
fi
fi
if resolved="$(git rev-parse --verify "${ref}^{commit}" 2>/dev/null)"; then
printf '%s\n' "$resolved"
return
fi
echo "error: cannot resolve ref: $ref" >&2
return 1
}
resolve_base_ref() {
local ref="$1"
local resolved=""
if git check-ref-format --branch "$ref" >/dev/null 2>&1; then
if git fetch origin "$ref" >/dev/null 2>&1; then
git rev-parse --verify 'FETCH_HEAD^{commit}'
return
fi
fi
if resolved="$(git rev-parse --verify "${ref}^{commit}" 2>/dev/null)"; then
printf '%s\n' "$resolved"
return
fi
echo "error: cannot resolve base ref: $ref" >&2
return 1
}
if [[ "$target_kind" == "pr" ]]; then
repo_slug="$(gh repo view --json nameWithOwner --jq .nameWithOwner)"
pr_data="$(gh pr view "$pr_number" --repo "$repo_slug" \
--json baseRefName,headRefOid \
--jq '"\(.baseRefName) \(.headRefOid)"')"
read -r base_name reported_head_sha <<<"$pr_data"
head_ref="refs/pr-visual-review/pull/${pr_number}/head"
git fetch origin "+pull/${pr_number}/head:${head_ref}"
git fetch origin "+refs/heads/${base_name}:refs/remotes/origin/${base_name}"
after_sha="$(git rev-parse --verify "${head_ref}^{commit}")"
if [[ "$after_sha" != "$reported_head_sha" ]]; then
latest_reported_head="$(gh pr view "$pr_number" --repo "$repo_slug" \
--json headRefOid --jq .headRefOid)"
if [[ "$after_sha" != "$latest_reported_head" ]]; then
echo "error: PR head changed while resolving; rerun for a consistent target" >&2
exit 1
fi
fi
base_sha="$(git rev-parse --verify "refs/remotes/origin/${base_name}^{commit}")"
else
after_sha="$(resolve_target_ref "$target")"
base_name="${base_ref:-main}"
base_sha="$(resolve_base_ref "$base_name")"
fi
before_sha="$(git merge-base "$base_sha" "$after_sha")"
if [[ -z "$before_sha" ]]; then
echo "error: no merge-base found between $base_name and $target" >&2
exit 1
fi
session_dir="$(mktemp -d /tmp/bitchat-ui-review.XXXXXX)"
worktree_path="${session_dir}/worktree"
artifact_dir="${session_dir}/artifacts"
mkdir -p "$artifact_dir"
git worktree add --detach "$worktree_path" "$before_sha"
if [[ -f "${repo_root}/local.properties" && ! -e "${worktree_path}/local.properties" ]]; then
ln -s "${repo_root}/local.properties" "${worktree_path}/local.properties"
fi
session_env="${session_dir}/session.env"
{
printf 'SESSION_DIR=%q\n' "$session_dir"
printf 'WORKTREE_PATH=%q\n' "$worktree_path"
printf 'ARTIFACT_DIR=%q\n' "$artifact_dir"
printf 'SOURCE_REPO_ROOT=%q\n' "$repo_root"
printf 'TARGET_KIND=%q\n' "$target_kind"
printf 'TARGET_INPUT=%q\n' "$target"
printf 'BASE_NAME=%q\n' "$base_name"
printf 'BEFORE_SHA=%q\n' "$before_sha"
printf 'AFTER_SHA=%q\n' "$after_sha"
printf 'PR_NUMBER=%q\n' "$pr_number"
} > "$session_env"
printf 'SESSION_DIR=%s\n' "$session_dir"
printf 'WORKTREE_PATH=%s\n' "$worktree_path"
printf 'ARTIFACT_DIR=%s\n' "$artifact_dir"
printf 'BEFORE_SHA=%s\n' "$before_sha"
printf 'AFTER_SHA=%s\n' "$after_sha"
printf 'TARGET_KIND=%s\n' "$target_kind"
if [[ -n "$pr_number" ]]; then
printf 'PR_NUMBER=%s\n' "$pr_number"
fi
printf 'SESSION_ENV=%s\n' "$session_env"

217
.agents/skills/android-ui-visual-review/scripts/upload_pr_images.py Executable file
View File

@ -0,0 +1,217 @@
#!/usr/bin/env python3
"""Upload PR screenshots through gh api without modifying a source branch."""
from __future__ import annotations
import argparse
import base64
import json
import re
import subprocess
import sys
from pathlib import Path
from urllib.parse import quote
REPO_RE = re.compile(r"^[A-Za-z0-9_.-]+/[A-Za-z0-9_.-]+$")
def gh_api(
endpoint: str,
*,
method: str = "GET",
payload: dict[str, object] | None = None,
allow_not_found: bool = False,
) -> dict[str, object] | None:
command = ["gh", "api"]
if method != "GET":
command.extend(["-X", method])
command.append(endpoint)
input_text = None
if payload is not None:
command.extend(["--input", "-"])
input_text = json.dumps(payload)
result = subprocess.run(
command,
input=input_text,
text=True,
capture_output=True,
check=False,
)
if result.returncode != 0:
if allow_not_found and "HTTP 404" in result.stderr:
return None
raise RuntimeError(result.stderr.strip() or f"gh api failed for {endpoint}")
if not result.stdout.strip():
return {}
parsed = json.loads(result.stdout)
if not isinstance(parsed, dict):
raise RuntimeError(f"unexpected gh api response for {endpoint}")
return parsed
def validate_files(paths: list[Path]) -> list[tuple[Path, str, int]]:
if not paths:
raise ValueError("at least one image file is required")
files: list[tuple[Path, str, int]] = []
seen: set[str] = set()
for path in paths:
resolved = path.resolve()
if not resolved.is_file():
raise ValueError(f"file not found: {path}")
name = resolved.name
if name in seen:
raise ValueError(f"duplicate basename would collide in upload: {name}")
seen.add(name)
if resolved.suffix.lower() not in {".png", ".jpg", ".jpeg", ".webp", ".gif"}:
raise ValueError(f"unsupported image type: {name}")
files.append((resolved, name, resolved.stat().st_size))
return files
def image_markdown(files: list[dict[str, str]]) -> str:
sections: list[str] = []
for index in range(0, len(files), 2):
pair = files[index : index + 2]
labels = " | ".join(item["name"] for item in pair)
separators = " | ".join("---" for _ in pair)
images = " | ".join(
f'![{item["name"]}]({item["url"]})' for item in pair
)
sections.append(f"| {labels} |\n| {separators} |\n| {images} |")
return "\n\n".join(sections)
def main() -> int:
parser = argparse.ArgumentParser()
parser.add_argument("--repo", required=True, help="OWNER/REPO")
parser.add_argument("--pr", required=True, type=int)
parser.add_argument("--dry-run", action="store_true")
parser.add_argument("files", nargs="+", type=Path)
args = parser.parse_args()
if not REPO_RE.fullmatch(args.repo):
raise ValueError("--repo must use OWNER/REPO")
if args.pr <= 0:
raise ValueError("--pr must be a positive integer")
files = validate_files(args.files)
if args.dry_run:
print(
json.dumps(
{
"status": "dry-run",
"repo": args.repo,
"pr": args.pr,
"ref": f"refs/uploads/issues/{args.pr}",
"files": [
{"name": name, "size": size} for _, name, size in files
],
},
indent=2,
)
)
return 0
api_prefix = f"repos/{args.repo}"
ref_path = f"uploads/issues/{args.pr}"
ref_response = gh_api(
f"{api_prefix}/git/ref/{ref_path}",
allow_not_found=True,
)
parent_sha = ""
base_tree_sha = ""
if ref_response is not None:
ref_object = ref_response.get("object")
if not isinstance(ref_object, dict) or not isinstance(ref_object.get("sha"), str):
raise RuntimeError("existing upload ref response did not contain a commit SHA")
parent_sha = str(ref_object["sha"])
parent_commit = gh_api(f"{api_prefix}/git/commits/{parent_sha}")
tree = parent_commit.get("tree") if parent_commit else None
if not isinstance(tree, dict) or not isinstance(tree.get("sha"), str):
raise RuntimeError("existing upload commit did not contain a tree SHA")
base_tree_sha = str(tree["sha"])
entries: list[dict[str, str]] = []
for path, name, _ in files:
content = base64.b64encode(path.read_bytes()).decode("ascii")
blob = gh_api(
f"{api_prefix}/git/blobs",
method="POST",
payload={"content": content, "encoding": "base64"},
)
blob_sha = blob.get("sha") if blob else None
if not isinstance(blob_sha, str):
raise RuntimeError(f"blob upload did not return a SHA for {name}")
entries.append(
{"path": name, "mode": "100644", "type": "blob", "sha": blob_sha}
)
tree_payload: dict[str, object] = {"tree": entries}
if base_tree_sha:
tree_payload["base_tree"] = base_tree_sha
tree_response = gh_api(
f"{api_prefix}/git/trees",
method="POST",
payload=tree_payload,
)
tree_sha = tree_response.get("sha") if tree_response else None
if not isinstance(tree_sha, str):
raise RuntimeError("tree creation did not return a SHA")
commit_response = gh_api(
f"{api_prefix}/git/commits",
method="POST",
payload={
"message": f"Add visual comparison screenshots for PR #{args.pr}",
"tree": tree_sha,
"parents": [parent_sha] if parent_sha else [],
},
)
commit_sha = commit_response.get("sha") if commit_response else None
if not isinstance(commit_sha, str):
raise RuntimeError("commit creation did not return a SHA")
if parent_sha:
gh_api(
f"{api_prefix}/git/refs/{ref_path}",
method="PATCH",
payload={"sha": commit_sha, "force": False},
)
else:
gh_api(
f"{api_prefix}/git/refs",
method="POST",
payload={"ref": f"refs/{ref_path}", "sha": commit_sha},
)
uploaded: list[dict[str, str]] = []
for _, name, _ in files:
url = f"https://github.com/{args.repo}/blob/{commit_sha}/{quote(name)}?raw=true"
uploaded.append({"name": name, "url": url})
print(
json.dumps(
{
"status": "ok",
"repo": args.repo,
"pr": args.pr,
"ref": f"refs/{ref_path}",
"sha": commit_sha,
"files": uploaded,
"markdown": image_markdown(uploaded),
},
indent=2,
)
)
return 0
if __name__ == "__main__":
try:
raise SystemExit(main())
except (OSError, ValueError, RuntimeError, json.JSONDecodeError) as exc:
print(f"error: {exc}", file=sys.stderr)
raise SystemExit(1)

168
.agents/skills/android-ui-visual-review/scripts/validate_capture_manifest.py Executable file
View File

@ -0,0 +1,168 @@
#!/usr/bin/env python3
"""Validate a visual-review capture manifest and its PNG evidence."""
from __future__ import annotations
import argparse
import json
import re
import struct
import sys
from pathlib import Path
PNG_SIGNATURE = b"\x89PNG\r\n\x1a\n"
SHA_RE = re.compile(r"^[0-9a-f]{40}$")
def fail(message: str) -> None:
raise ValueError(message)
def relative_file(root: Path, value: object, field: str) -> Path:
if not isinstance(value, str) or not value:
fail(f"{field} must be a non-empty relative path")
candidate = Path(value)
if candidate.is_absolute() or ".." in candidate.parts:
fail(f"{field} must stay relative to the artifact directory: {value!r}")
resolved = (root / candidate).resolve()
try:
resolved.relative_to(root.resolve())
except ValueError as exc:
raise ValueError(f"{field} escapes the artifact directory: {value!r}") from exc
if not resolved.is_file():
fail(f"{field} does not exist: {value}")
return resolved
def png_dimensions(path: Path) -> tuple[int, int]:
with path.open("rb") as handle:
header = handle.read(24)
if len(header) < 24 or header[:8] != PNG_SIGNATURE or header[12:16] != b"IHDR":
fail(f"not a valid PNG with an IHDR header: {path.name}")
width, height = struct.unpack(">II", header[16:24])
if width <= 0 or height <= 0:
fail(f"invalid PNG dimensions in {path.name}: {width}x{height}")
return width, height
def require_string(data: dict[str, object], key: str) -> str:
value = data.get(key)
if not isinstance(value, str) or not value.strip():
fail(f"{key} must be a non-empty string")
return value
def main() -> int:
parser = argparse.ArgumentParser()
parser.add_argument("manifest", type=Path)
args = parser.parse_args()
manifest_path = args.manifest.resolve()
if not manifest_path.is_file():
fail(f"manifest not found: {manifest_path}")
root = manifest_path.parent
with manifest_path.open(encoding="utf-8") as handle:
data = json.load(handle)
if not isinstance(data, dict):
fail("manifest root must be an object")
require_string(data, "target")
before_sha = require_string(data, "before_sha")
after_sha = require_string(data, "after_sha")
if not SHA_RE.fullmatch(before_sha) or not SHA_RE.fullmatch(after_sha):
fail("before_sha and after_sha must be lowercase 40-character Git SHAs")
if before_sha == after_sha:
fail("before_sha and after_sha must differ")
environment = data.get("environment")
if not isinstance(environment, dict):
fail("environment must be an object")
for key in ("android_release", "android_sdk", "security_patch", "resolution_px"):
require_string(environment, key)
density = environment.get("default_density_dpi")
if not isinstance(density, int) or density <= 0:
fail("environment.default_density_dpi must be a positive integer")
captures = data.get("captures")
if not isinstance(captures, list) or not captures:
fail("captures must contain at least one before/after pair")
used_paths: set[Path] = set()
pair_summaries: list[dict[str, object]] = []
seen_ids: set[str] = set()
for index, item in enumerate(captures):
if not isinstance(item, dict):
fail(f"captures[{index}] must be an object")
capture_id = require_string(item, "id")
if capture_id in seen_ids:
fail(f"duplicate capture id: {capture_id}")
seen_ids.add(capture_id)
require_string(item, "surface")
require_string(item, "state")
require_string(item, "expected_change")
before_path = relative_file(root, item.get("before"), f"captures[{index}].before")
after_path = relative_file(root, item.get("after"), f"captures[{index}].after")
for path in (before_path, after_path):
if path in used_paths:
fail(f"screenshot reused by multiple manifest entries: {path.name}")
used_paths.add(path)
before_size = png_dimensions(before_path)
after_size = png_dimensions(after_path)
if before_size != after_size:
fail(
f"{capture_id} dimensions differ: before={before_size[0]}x{before_size[1]}, "
f"after={after_size[0]}x{after_size[1]}"
)
pair_summaries.append(
{
"id": capture_id,
"width": before_size[0],
"height": before_size[1],
}
)
extras = data.get("extras", [])
if not isinstance(extras, list):
fail("extras must be an array")
for index, item in enumerate(extras):
if not isinstance(item, dict):
fail(f"extras[{index}] must be an object")
extra_id = require_string(item, "id")
if extra_id in seen_ids:
fail(f"duplicate capture/extra id: {extra_id}")
seen_ids.add(extra_id)
require_string(item, "role")
path = relative_file(root, item.get("file"), f"extras[{index}].file")
if path in used_paths:
fail(f"screenshot reused by multiple manifest entries: {path.name}")
used_paths.add(path)
png_dimensions(path)
limitations = data.get("limitations", [])
if not isinstance(limitations, list) or not all(
isinstance(item, str) and item.strip() for item in limitations
):
fail("limitations must be an array of non-empty strings")
result = {
"status": "ok",
"pairs": len(captures),
"extras": len(extras),
"png_files": len(used_paths),
"dimensions": pair_summaries,
}
print(json.dumps(result, indent=2))
return 0
if __name__ == "__main__":
try:
raise SystemExit(main())
except (OSError, json.JSONDecodeError, ValueError) as exc:
print(f"error: {exc}", file=sys.stderr)
raise SystemExit(1)