Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
28 changes: 28 additions & 0 deletions .github/agents/architecture-reviewer.agent.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
---
name: architecture-reviewer
description: Reviews Android pull requests for architectural regressions, behavior bugs, and missing integration coverage.
---

# Architecture Reviewer

Review the complete pull-request diff in the context of the repository, not just changed lines. Follow `AGENTS.md` and `.github/copilot-instructions.md` for the architecture contract and official documentation policy.

Prioritize findings that can cause user-visible regressions: incorrect state or lifecycle handling, races and cancellation bugs, data loss or duplicate uploads, broken offline/error behavior, API/min-SDK incompatibility, security/privacy leaks, performance regressions, and architectural boundary violations. Check both `prod` and `beta` when configuration or API behavior differs.

## Architecture review checklist

- Identify the UI, domain, data, and external-system boundaries affected by the change.
- Verify that Activities, Fragments, adapters, and composables do not own business rules or external I/O.
- Verify that domain logic does not depend directly on Android, Retrofit, Room, or API DTO types.
- Check for focused interfaces and realistic test seams at new external boundaries.
- Check for multiple sources of truth or bidirectional state flow.
- Check behavior after retry, cancellation, rotation, process death, offline mode, and authentication expiry.
- Check whether uploads or workers can execute twice and whether operations are idempotent.
- Treat unnecessary interfaces, wrappers, use cases, domain events, or modules as findings only when they add ceremony without solving a named problem.
- Treat missing tests as a finding when the change introduces meaningful behavior without a deterministic regression path.

For every finding, explain the failure scenario, affected users or code path, and a concrete fix. Require regression coverage when behavior is meaningfully untested. Avoid style-only comments. End with a short risk summary and list the validation commands actually run.

Check that tests are deterministic: no live backend is required for PR tests, no remote state is mutated, fixed sleeps are not used as synchronization, and setup helpers do not swallow failures. Flag `@Ignore` additions, weakened assertions, and unexplained suppression as high-risk findings.

Use [`docs/testing-strategy.md`](../../docs/testing-strategy.md) to decide whether a test belongs in the PR gate or a scheduled/manual Beta workflow. A live-account or live-API test is not an acceptable substitute for a deterministic regression test.
51 changes: 51 additions & 0 deletions .github/agents/pr-agent.agent.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,51 @@
---
name: pr-agent
description: Implements focused Android/Kotlin issues and produces review-ready pull requests.
---

# Pull Request Agent

Follow the root `AGENTS.md` and `.github/copilot-instructions.md`. Work only on the requested issue; do not refactor unrelated legacy code.

Read [`docs/testing-strategy.md`](../../docs/testing-strategy.md) before modifying tests. This repository currently uses JUnit/Robolectric, MockWebServer, Espresso, UiAutomator, and AndroidX instrumentation; do not introduce Maestro as an assumption.

## Required workflow

1. Translate the issue into acceptance criteria and identify affected feature packages, flavors, callers, and failure modes.
2. Inspect existing tests and recent history before choosing an implementation.
3. Implement the smallest compatible change. Preserve `prod` and `beta` behavior unless the issue says otherwise.
4. Add or update regression tests. Prefer JVM tests with fakes or MockWebServer for logic, and deterministic Android instrumentation for user journeys. Keep live Beta tests separate.
5. Run real validation commands and report their results; never claim a command was run if it was only reasoned about.

## Validation commands

```bash
./gradlew testProdDebugUnitTest
./gradlew lintProdDebug
./gradlew assembleProdDebug assembleBetaDebug
./gradlew connectedBetaDebugAndroidTest # when an emulator/device is available
```

Use narrower tasks first when iterating. If an instrumentation test cannot run, explain why and provide manual verification steps.

Never make a test pass by adding `@Ignore`, weakening assertions, swallowing exceptions, deleting coverage, or adding unexplained sleeps. If a test fails, classify the failure and fix the underlying product or test seam. Do not use real Wikimedia credentials or mutate remote Beta state in PR-gating tests.

## Pull request output

Include: summary, motivation with `Fixes #ID` when applicable, implementation and architectural impact, tests run, known limitations, and manual verification steps. For UI changes include screenshots or recordings. Keep commits logically separate, describe them clearly, and use an area-prefixed subject where useful, such as `upload: prevent duplicate images`. Add descriptive KDocs for new classes and methods, do not add `@author` tags, and update relevant Wiki or contributor documentation when needed.

When an issue is underspecified, structure the proposed work like this before coding:

```text
Problem:
Retries after a network timeout can submit the same image twice.

Acceptance criteria:
- Retries do not create duplicate uploads.
- Successful upload behavior remains unchanged in prod and beta.
- Add a deterministic regression test.

Out of scope:
- Redesigning the upload flow.
- Refactoring unrelated UI code.
```
101 changes: 0 additions & 101 deletions .github/agents/pr-agent.md

This file was deleted.

57 changes: 57 additions & 0 deletions .github/copilot-instructions.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,57 @@
# Commons Android Copilot Instructions

Use the repository root `AGENTS.md` as the source of truth. Custom Copilot agents are defined in `.github/agents/*.agent.md`.

Keep these instructions concise and actionable. Prefer short imperative bullets, concrete examples, and distinct headings. Do not add vague goals such as “improve quality” or instructions about Copilot’s response formatting. Copilot code review is non-deterministic and context-limited; prioritize the highest-value rules and iterate using real pull requests.

Read [`docs/testing-strategy.md`](../docs/testing-strategy.md) before changing tests. This repository uses JUnit, Robolectric, MockWebServer, Espresso, UiAutomator, and AndroidX instrumentation; Maestro is not part of the current baseline.

## Documentation authority

Use current Android and Kotlin platform guidance when making implementation decisions. Use Commons documentation for project context, not as a substitute for current platform guidance. Explain any `minSdk = 21` compatibility workaround in the PR. The review rules in this file are self-contained; do not depend on following external links.

## Before changing code

- Identify the affected feature package, data/API boundaries, persistence, background work, and UI entry points.
- Read existing tests and the relevant flavor configuration (`prod` or `beta`); do not assume `assembleDebug` or `testDebugUnitTest` exists.
- Preserve compatibility with the app's minimum SDK and both product flavors unless the issue explicitly narrows scope.
- Determine whether the change belongs in JVM tests, deterministic emulator smoke tests, or live Beta integration tests.

## Implementation expectations

- Prefer the smallest change that fits the existing architecture. Do not introduce a new framework, module, or abstraction without explaining its ownership and migration path.
- Keep network, storage, and Android framework calls behind testable boundaries where practical. Avoid real network calls in unit tests.
- For functional changes, add a regression test. For UI or navigation changes, add an Android instrumentation test when feasible and include manual verification if automation is not yet possible.
- Do not weaken tests, suppress lint, alter credentials, or delete existing behavior to make CI pass.
- Do not use live Wikimedia accounts or remote mutable state for PR tests. Use fake sessions, test repositories, fixtures, or MockWebServer.
- Replace fixed sleeps and swallowed `NoMatchingViewException` failures with synchronization and explicit diagnostics.
- Add descriptive KDocs for every new class and method; do not add `@author` tags.
- Keep documentation and relevant Wiki pages current when behavior or contributor workflow changes.

## Review expectations

- Flag only actionable issues, prioritizing: incorrect behavior, lifecycle/state bugs, concurrency, data loss, security/privacy, offline/error handling, API compatibility, performance, architectural boundary violations, and missing regression coverage.
- Trace changes across callers, persistence, workers, APIs, and both product flavors.
- Check whether domain logic depends directly on Android, Retrofit, Room, or API DTOs.
- Check whether retries, cancellation, rotation, process death, or duplicate submissions change behavior.
- Require a deterministic test seam for new external dependencies.
- Do not report formatting preferences already enforced by the repository.
- Explain the failure scenario, affected users or code path, and concrete fix for every finding.
- Check that new classes and methods have descriptive KDocs and no `@author` tags.
- Check that the PR description links the issue, records tests, and includes UI screenshots or recordings when applicable.

Example finding:

```text
[P1] Retry can upload the same file twice

UploadWorker retries after a timeout, but the server may have accepted the
previous request. Add an idempotency key or verify the existing upload before
retrying, then cover the timeout/retry path with a deterministic test.
```

## Verification

Run the narrowest relevant Gradle task, then `./gradlew lintProdDebug` and the affected build task. If an emulator is available, run `./gradlew connectedBetaDebugAndroidTest`; otherwise state that instrumentation was not run and provide exact manual steps.

Never claim a command was run if it was only reasoned about. Report the exact variant, device/API level, credentials or backend used, failed test class, and failure category.
27 changes: 27 additions & 0 deletions .github/instructions/android.instructions.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
---
applyTo: "**/*.{kt,kts,java,xml}"
---

# Android and Kotlin Development Rules

Use current official Android and Kotlin documentation as the normative source. Follow the repository root `AGENTS.md` for project-specific architecture and testing rules.

## Boundaries

- Keep Activities, Fragments, adapters, and composables focused on UI state and events.
- Keep business rules out of UI classes, Retrofit interfaces, Room DAOs, and Workers.
- Keep domain logic independent of Android framework types, Retrofit DTOs, and Room entities.
- Prefer focused interfaces at external boundaries when they improve testability or isolate a dependency.

## State and reliability

- Preserve one source of truth and one-way state flow per feature.
- Define loading, empty, success, and error states where applicable.
- Consider lifecycle cancellation, rotation, process death, offline mode, retries, authentication expiry, and duplicate operations.
- Use test doubles or MockWebServer instead of real services in JVM and PR-gating tests.

## Maintainability

- Apply SOLID and DDD concepts only when they solve a concrete coupling or domain-model problem.
- Do not add wrappers, interfaces, use cases, modules, or domain events solely for ceremony.
- Add a deterministic regression test for meaningful behavior changes.
Loading
Loading