Frostwire Engineer

FrostWire Engineer

Combined house style of gubatron (project owner, lead developer) and aldenml (co-founder, jlibtorrent architect), distilled from the FrostWire Desktop, Android, and JLibTorrent codebases. Replaces the legacy AGENTS.md at the repo root.

If you only read one section, read §1 Mantras and §10 Maintainer's Checklist.

Read the affected code and callers first; use frostwire-code-reviewer for review and frostwire-performance-reviewer for measured optimization. These are rules, not claims that current production code satisfies them. Historical examples are evidence to recheck, not mandatory obsolete wire formats. Section 15 retains newer review #1048 defenses and shared-memory contracts separately from the restored house style.

1. Quick-Reference Mantras

When in doubt, apply the closest mantra. They are not slogans — each one maps to a real review comment we have shipped.

2. Core Philosophies (Non-Negotiable)

KISS — Keep It Simple, Stupid

• If you can't explain what you're doing in one sentence, you're over-engineering.
• Prefer well-named methods and code reusability over comments. Code should be self-explanatory.
• Net-negative LoC after a refactor is the goal, not a side effect.

DRY — Don't Repeat Yourself

• Reuse your code and ours. Search com.frostwire.util.* for any helper (Hex, Base64, URL, hashing, time) before writing a new one.
• DRY code behaves like an equation — it writes itself after the first occurrence.
• The threshold for extracting: 2 call sites tolerable, 3+ extract. Don't preemptively abstract.
• Watch for coincidental duplication (the same value in 3 places that must stay in sync — trackers, SoundCloud credentials, default ports). These get a single source of truth and a test asserting the others match.

Minimum Scope Principle

• Variables: as local as possible. Method scope > field scope > static.
• Class visibility, in order of preference:
  1. Local variable — first choice
  2. Private member — only if it must outlive the method
  3. Protected — only for inheritance
  4. Public — only when you are certain no consumer can break internal state
• Tight scopes prevent concurrency bugs. The wider the surface, the more locks you need.

Composition Over Inheritance

• If you're extending a non-abstract class, you should probably be composing it instead.
• If you don't own the source of the class you're extending, behavior may surprise you across versions.
• Reach for: final fields of helper types, constructor injection, static factory methods. Avoid deep class hierarchies.

Immutability Where Possible

• Favor immutable objects and immutable state. Pairs naturally with minimum scope.
• Final fields, defensive byte[].clone() on POJO boundaries, no setters on value types.
• Reduces entire classes of concurrency bugs (no shared mutable state = no race).

Protocol Reality Before Architecture

• Before designing a distributed protocol, verify the real primitive in the library API and specs. Do not design from a desired abstraction.
• Example: BEP 44 mutable DHT items are single-writer (SHA1(pubkey + salt)), not arbitrary multi-writer buckets. If you need multi-writer discovery, use BEP 5 peer rendezvous (dhtAnnounce / dhtGetPeers) plus an authenticated protocol on top.
• Write a small local integration test that proves the primitive before writing a large design around it. The test should validate discovery, not just put/get with an in-process key.
• Prefer one simple protocol path over three clever DHT record types. If a proposed design needs IndexAnnouncement, search-hint, relay-record, and identity-record all in the DHT, ask whether two of those are really application messages between peers.

Headless-First Modularity

• Network-native features must run in a headless JVM without Swing, Android, or desktop settings classes.
• Split modules by role, not UI: common/ for records, crypto, protocol messages, and interfaces; desktop/ for Swing and JDBC implementations; android/ for Android storage/UI; a small relayd/ module for cloud relays.
• A relay-only node should not require SQLite, a media library, a GUI, or large disk. It needs jlibtorrent, keys, rate limits, RAM-bounded caches, and fast networking.
• Use composition for roles: RelayRole, IndexRole, SearchRole, UiRole. Avoid a monolithic DistributedSearchManager that owns everything.

Network Topology Is a Contract

• A control-plane registration is not evidence of data-plane reachability. For every advertised host:rudpPort, identify the process that binds that UDP socket and test delivery to it from the sender's network.
• Remote control mode must not silently remove the local data-plane listener required for inbound traffic. If the remote relay owns delivery instead, define how it preserves source identity, queues inbound payloads, and routes replies.
• A fallback is real only when its wire type is emitted and handled. Do not call a method sendRelay or holePunch unless an integration test observes the expected packet type on the wire and the final payload at the target.
• Model request and response routes separately. NAT, endpoint ownership, and registry state may make one direction work while the reply path fails.
• Before claiming a multi-node feature works, exercise the exact production roles, not just same-JVM or loopback fixtures.

rUDP Session & Multi-Hop RELAY Rules (IceBridge)

These are hard-won from multi-hop mesh E2E + adversarial review (MentisDB frostwire #873–#876), with authentication and bounds corrected by later review #1048.

• HELLO / HELLO_ACK symmetry: both roles must prove key possession using domain-separated, peer-bound transcripts including version, identities, fresh challenges, connection IDs and negotiated session parameters. The historical same-shape pub + timestamp + sig(connectionId || timestamp) exchange is insufficient; an empty HELLO_ACK is also a silent half-open session. Set remotePub only after the current authenticated handshake completes, not from a reusable identity record.
• One session per peer address: connect(remote) should reuse a valid sessionsByAddress entry without overwriting an authenticated responder session. Coordinate simultaneous open and identity replacement; address/CID lookup selects a candidate session, not proof of identity. Require authenticated path validation before migration.
• RELAY hop auth: validate packet integrity before acting on the frame; frame.sourcePub must equal the authenticated hop peer (session.remotePub()) where the field represents the hop. Keep logical origin identity separate. Lookup by packet.connectionId() (same as DATA), then any permitted address lookup, is not authentication.
• RELAY_RESPONSE: requires packet authentication within an authenticated session; attribute hop identity to the verified session.remotePub(), never spoofable header bytes, while preserving the logical origin separately. No unauthenticated write() fire-and-forget.
• Local /poll clients: peers that register with this process's own host:rudpPort drain the control inbound queue — deliver with notifyListener / isLocalRudpEndpoint, not self-UDP RELAY_RESPONSE.
• Amplification bounds (all required for multi-hop flood):
  • For the unfragmented RELAY path, enforce RelayFrame.MAX_APP_PAYLOAD = RudpPacket.MAX_FRAGMENT_PAYLOAD - HEADER_LENGTH at encode and decode, accounting for the current authenticated wire overhead; do not assume DATA fragmentation also supports RELAY.
  • Bound hop TTL and mesh fan-out using the current topology/configuration and test their boundaries; historical defaults of 3 are not universal contracts. Enforce per-peer RELAY/RELAY_RESPONSE limits plus bounded global ingress budgets.
• Security hardening vs E2E: removing an unauthenticated path will break tests that depended on it. After any RELAY/HELLO auth change, re-run MultiRelayMeshSearchTest (or three-forwarder equivalent): warm the current authenticated handshake, assert seeder is not on the searcher's home relay, assert signed result arrives. Never restore an insecure handshake to satisfy an old fixture.

Wire Version Discipline

• Bumping a signed wire type (RemoteSearchRequest, RemoteSearchResponse, IdentityRecord) requires: version field bump, coordinated writers/readers, explicit read-path only for older versions whose authenticated contract remains sound, and a test that old maps still decode safely (or are cleanly rejected). No insecure downgrade or mandatory obsolete canonical-signature format.
• Historical example: IdentityRecord wrote v3 (caps) but read v1/v2 to preserve live self-ping and older peers. Recheck the current version/security contract; compatibility is not a reason to accept an insecure legacy record.

3. Concurrency & Threading Rules

Never Block the UI Thread

• Desktop: All heavy ops (JNI, I/O, network, parsing, DB writes) on a background executor. Use GUIMediator.safeInvokeLater() for UI updates; it posts TO the EDT and does not offload work.
• Android: All heavy ops on a background thread. Never network or disk I/O on the main thread. Use SystemUtils.postToHandler(HandlerThreadName.MISC, runnable) for non-urgent work, HandlerThreadName.HIGH_PRIORITY for short user-tap actions (play button, open file) targeting <100 ms response. Inspect the selected helper: some handlers spawn threads rather than provide a bounded queue; priority is not a latency guarantee.
• JNI calls to jlibtorrent must never happen on the EDT. Always offload to a background executor.
• Strict EDT is enabled on desktop: StrictEdtMode installs a timing EventQueue at startup and reports any dispatch event exceeding 2 seconds. Treat every report as a defect; never wait on a latch, Future, DHT/JNI call, network request, disk operation, or lock with unbounded contention from the EDT.

Desktop Executor Selection

• DesktopParallelExecutor.execute(...) is a four-worker bounded executor for independent, reorder-safe work such as file I/O, network fetches, media resolution, and parallel searches. Its bounded queue may reject bursts; callers that cannot drop work must handle RejectedExecutionException. Catch and log failures inside submitted Runnables because execute exposes no result.
• BackgroundQueuedExecutorService.schedule(...) is a single-worker FIFO queue. Use it only when serialization is required for correctness: shared torrent/UI model state, ordered restore/mutation, or operations that must not overlap. Keep every task short; one slow task delays every queued GUI-background operation.
• Neither executor may mutate Swing. Snapshot EDT state before dispatching, perform blocking work in the executor, then use GUIMediator.safeInvokeLater(...) for the smallest possible UI update. Do not use safeInvokeAndWait from background work unless the call chain is proven not to depend on the waiting executor.

Thread-Safe Singletons

• If you use singletons, make them thread-safe (double-checked locking with a volatile instance field, or static-holder pattern).
• Clear singleton data models before reloading to prevent stale state.
• Singletons that hold native handles (BTEngine, SessionManager) must release those handles deterministically in LifecycleManager.onShutdown().

Defensive Programming in Concurrent Contexts

• Null-check everything that crosses thread boundaries.
• Guard recoverable native load/initialization failures with try/catch and a safe disabled state. Example: if Python ELF loading fails on Android with a Java-visible error, log and fall back to "<unavailable>". Java catches cannot stop SIGSEGV/native process crashes or make use-after-free safe; ownership and lifetime checks are still required.
• Invalidate stale state/generations before any async post. Keep in-memory removals and flag flips synchronous; perform blocking store cleanup on the owning worker with ordering/generation checks. Otherwise the callback fires on stale data. (Real bug: DeleteDialog race that left deleted songs in Recent/Favorites.)

4. Java Code Style (FrostWire Conventions)

File Headers

• Java files use the standard FrostWire Apache/GPL header; follow the neighboring file's actual license and authorship. Example:

  /*
   *     Created by Angel Leon (@gubatron)
   *     Copyright (c) 2011-2026, FrostWire(R). All rights reserved.
   *
   *     Licensed under GPL v3. See LICENSE file.
   */
  

• Classes that are co-maintained by both gubatron and aldenml add @author gubatron and @author aldenml to the class javadoc.
• One file = one top-level public class. Inner classes only for tightly-scoped helpers.

Imports

• No wildcard imports.
• Group: java.*, javax.*, third-party, com.frostwire.*. Standard IDE ordering.
• Remove unused imports in the same commit that introduces the change.

Class & Member Style

• public final class on utilities and value types.
• Private constructors on utility classes (private X() {}).
• Fields private final when the class is immutable; explicit this.x = x.clone() for byte arrays.
• Getters on immutable POJOs return defensive clones of mutable fields (return bytes.clone()).
• Builder pattern for value types with >4 fields or any optional field.

Methods

• Methods named for what they do at the caller's level: upsert, needsRepublish, markPublished. The body explains how.
• Boolean accessors: isOpen(), isReadyToSubmitTask(name).
• Package-private statics only when package collaborators need that contract — otherwise private; do not widen access solely for tests.
• A test in the same package cannot directly access private methods. Test behavior through the existing API or a real package-level collaboration, not an invented visibility exception.

Logging

• Always com.frostwire.util.Logger for application/library diagnostics — never org.apache.commons.logging, java.util.logging, org.slf4j, or System.out/System.err. Intentional headless CLI help/output is an exception, not permission to log secrets.
• Declare as private static final Logger LOG = Logger.getLogger(MyClass.class);
• Pass throwables as a second argument: LOG.warn("op failed for " + id, e). Never concatenate the stack trace into the message.
• Don't @SuppressWarnings on printStackTrace or unused log statements — delete the dead code instead.

Null Safety

• Null-check deserialized objects, especially at API boundaries (intents, bundles, JSON, network responses).
• Use Optional<T> for return values that may legitimately be absent.
• Guard Android lifecycle callbacks against null / destroyed state.
• Check for null listData, srList, and similar deserialized collections before use.

Resource Cleanup

• Always try-with-resources for owned InputStream, OutputStream, Reader, Writer, Connection, PreparedStatement, ResultSet, cursors and HTTP responses; document borrowed-resource ownership.
• Clean up unique temp files after use, including failure and cancellation paths. See OkHttpClientWrapper.save() lesson: 0-byte temp files leak when SocketException happens before the response is written.

Avoid Magic Numbers & Strings

• Named constants for regex patterns, buffer sizes, format signatures, table/column names, DHT key prefixes.
• When you find a string literal duplicated in 2+ files, hoist it.

No @SuppressWarnings("unused") on Private Helpers

• If a private helper is unused, delete it. The annotation is a code smell that hides dead code.
• The compiler is right; remove the code, don't silence it.

Reuse Before You Build

• Before writing a utility, grep the project for com.frostwire.util.* (Hex, Base64, UrlUtils, JsonUtils, HttpClientFactory, Ssl, StringUtils, ThreadPool, HistoHashMap, Ref, TaskThrottle, MimeDetector, …). There is almost certainly one already.
• Example: com.frostwire.util.Hex.encode(byte[]) / Hex.decode(String) — don't write your own toHex/fromHex.

5. Android-Specific Rules

minSdkVersion

• minSdk = 26 (Android 8.0), as documented at this revision. Build.VERSION.SDK_INT < 24 and SDK_INT < 26 guards are dead code for targets whose actual minSdk is 26. Don't add them "for safety". Grep build.gradle for minSdkVersion before writing a version guard.

Threading

• SystemUtils.postToHandler(HandlerThreadName.MISC, () -> { ... }) for background work; verify whether this helper queues or spawns a thread, and bound/coalesce repeated submissions.
• HandlerThreadName.HIGH_PRIORITY for short user-tap actions (play button, open file) — target <100 ms response and avoid queuing them behind slow background tasks; never use priority as a substitute for bounded work.
• ExoPlayer: .setLooper(handlerThread.getLooper()) is mandatory.
• OnPreferenceChangeListener callbacks run on the main thread. Calls to BTEngine, TransferManager, or ConfigurationManager that perform JNI, disk, or other blocking work must be dispatched with postToHandler(MISC, ...); return true immediately where the existing preference contract allows it. Do not do a synchronous read-back to verify the write. Inspect actual method behavior rather than labeling every accessor a StrictMode violation.

EditTextPreference

• EditTextPreference ALWAYS reads/writes its value as String via SharedPreferences.putString()/getString(). If a corresponding key is stored as Integer anywhere (ConfigurationDefaults, etc.), the preference will crash with ClassCastException at inflation time.
• Rule: numeric defaults for EditTextPreference keys are stored as String (e.g. "7656" not 7656). Read with Integer.parseInt(cm.getString(key)) + safe fallback.

View Tags

• View.setTag(int, Object) keys must be application resource ids — a key from View.generateViewId() throws IllegalArgumentException("The key must be an application-specific resource id") (generated ids have top package byte 0x00). For idempotence/installed markers use a named wrapper class + instanceof on the owner field, never a generated-id tag key (NavigationViewSafety, 2026-07).

String Resources

• New base res/values/strings.xml keys must be copied to all values-*/strings.xml in the same commit (English base text is the project placeholder practice; translators replace later). AndroidStringResourceParityTest enforces key parity across 36+ locales — 56 unpropagated IceBridge keys broke CI (2026-07). Preserve placeholders and plurals.

Native Init

• Guard native loading/initialization (Python, ffmpeg, custom .so) for Java-visible failures because corrupted native binaries on user devices are a real-world occurrence. Catch expected failures, log, and disable/fall back safely. A catch block cannot recover from a native process crash; prevent unsafe calls and native lifetime violations.

Configuration

• New code prefers Jetpack DataStore over SharedPreferences. ConfigurationManager migration is the standing tech-debt task.
• One source of truth for any config that lives in multiple places. SoundCloud credentials were the most recent offender (4 places drifted).

6. JNI / JLibTorrent (Native Code)

Memory Management

• SWIG-generated objects hold native memory. Be explicit about ownership.
• Close / dispose owned native resources deterministically using the actual binding API. Do not invoke or rely on finalizers for native cleanup.
• Pair every owned resource/listener retained for dht_put_item with a release path on shutdown; verify ownership in the installed binding rather than assuming a DHT put itself returns a disposable handle.

Prefer Library-Native Persistence

• If the underlying library (libtorrent / jlibtorrent) already persists the required state (e.g. save_state_flags_t.all(), ip_filter, DHT items, session state), do not build a parallel persistence layer. Verify the installed API's flags and coverage; do not assume it persists every example.
• Real example: removed ip_filter.db (149 lines) because jlibtorrent persists ip_filter via session state. Net-negative LoC, fewer bugs.

DHT Primitives (Do Not Invent Semantics)

• BEP 5 dhtAnnounce(infohash, port) + dhtGetPeers(infohash) is the right tool for multi-writer rendezvous: many peers can announce themselves under the same 20-byte infohash target.
• BEP 44 immutable put (dhtPutItem(Entry)) is content-addressed. You cannot choose its key; production discovery needs another channel.
• BEP 44/46 mutable put (dhtPutItem(pubkey, privkey, entry, salt)) is single-writer. The address is derived from the publisher key and salt; it is perfect for "my latest manifest", not for "everyone writes into one registry".
• DHT targets are 20-byte SHA-1 hashes. If a design says SHA-256(... )[:32] for a DHT lookup key, stop and correct it before coding.

Transport Semantics Must Be Observable

• API success means accepted delivery only when the API can prove it. If /send is asynchronous, return an explicit queued status and expose a failure or expiry signal; do not let callers treat HTTP 200 as peer delivery.
• Test opaque transport payloads with real source and target identities. The application layer must receive the logical requester identity, not accidentally attribute every forwarded packet to the relay.
• Bound every untrusted transport queue and reassembly buffer by count, bytes, and lifetime. Test backpressure, rejection and expiry under loss, duplicate packets, and a sender that never completes a fragmented message; never silently evict accepted work whose ACK promised retained ownership.

Lazy Loading

• Defer expensive initialization until the user actually needs it. Example: the IP Filter table is loaded only when the panel opens, not at app startup.

Performance & Concurrency

• For high-throughput Rust-backed code: RwLock vs DashMap tradeoffs, write batching, per-entity locking, WAL patterns. See rust-concurrency-patterns skill.

7. UI & User Experience

Internationalization (I18n)

• All user-facing strings must use I18n.tr() (Desktop) or Android strings.xml resources.
• Never hardcode English in UI code. Search for I18n.tr( to verify coverage of new strings.
• changelog.txt entries are user-facing and need i18n awareness too.

Skin / Theme Consistency

• Use themed components (SkinPopupMenu, SkinButton, etc.) instead of raw Swing/Android defaults.
• Test UI changes across light and dark themes.

Graceful Degradation

• If a feature fails (blocklist download, file parse, optional plugin load), show a helpful error and let the user continue.
• Never let an optional feature crash the entire application.

8. Testing & Quality

Tests Are Non-Negotiable

• Every public method gets at least one test. The bonus is when the test catches the bug.
• Coverage shape:
  • Happy path — the obvious use
  • Edge cases — empty input, malformed data, max values, nulls, very long strings
  • I/O paths — file open/close failure, partial read, encoding edge cases
  • Format detection heuristics — when parsing user-supplied formats
  • Concurrency — when the type crosses thread boundaries
• Real-world fixtures are valuable. Use actual iblocklist.com downloads, real Bitsearch responses, real magnet URIs captured as representative local fixtures; do not require live services for deterministic regression tests. Synthetic data misses the bugs that ship.

JUnit 5

• Project standard where supported: JUnit 5 (Jupiter). Follow the affected module's existing runner; Android tests may require JUnit 4/Robolectric (@RunWith(RobolectricTestRunner.class)). Do not impose a blanket JUnit 4 ban or add an unnecessary vintage engine.
• For Jupiter, @Test methods are void, no exceptions declared — use assertThrows instead of @Test(expected=...). Follow existing Android runner conventions where required.
• @TempDir and Files.createTempDirectory() for JVM filesystem tests; clean up in @AfterEach. Use the module's runner lifecycle and shared-code API policy on Android.
• @ExtendWith(LocalDhtCluster.class) for distributed DHT integration tests.

Bug Fix Pattern

1. Reproduce and understand the root cause. Don't fix symptoms.
2. Write the smallest possible fix. One line is better than ten.
3. Add a regression test that would have caught the original bug.
4. Update changelog.txt for user-facing fixes (Desktop and Android have separate changelogs).
5. Verify the build passes on all targets the change touches.

Compile Before Commit

• Desktop, from desktop/: ./gradlew compileJava (compile only) and ./gradlew test (full unit suite)
• Android, from android/: ./gradlew compilePlus1DebugJavaWithJavac and ./gradlew testPlus1DebugUnitTest; verify current flavor/task names.
• Fix compiler warnings; don't suppress them.
• In a shared tree, only the coordinator runs Gradle, including tests, compile, formatting and dependency tasks. Workers request these gates; passing gates never grants permission to commit or push.

Multi-Node / Mesh Tests

• Prefer real multi-process fixtures (with IceBridgeServer instances on free ports) over same-manager fakes when claiming multi-hop works. Multiple instances in one JVM are not multi-process evidence; record which fixture actually ran.
• After mesh link (RelayMesh.linkFully), warm sessions (direct /send or DATA through the authenticated path) so the current handshake settles before RELAY flood; HELLO_ACK arrival alone is not proof of completed mutual authentication.
• Assert topology, not only the end result: e.g. target peer absent from searcher's home registry so the path must multi-hop.
• DHT discovery: snapshot jlibtorrent peer lists (toArray / copy) before iterating — live lists mutate on alert threads (ConcurrentModificationException).

Tests Green Before Each Granular Push

When the user explicitly authorizes commit/push and requires "tests must pass to commit and push":

1. Implement the smallest coherent logical slice; keep coupled API/wire/caller/test updates together when splitting would break the build or contract.
2. Have the coordinator run the narrowest adequate suite (e.g. ./gradlew test --tests 'com.frostwire.search.relay.*' from desktop/), widened to full module/both targets for shared changes.
3. Commit that slice only if green and authorized.
4. Push that commit only if authorized (granular push), then start the next slice.

Never batch failed tests into a "fix later" commit. Never commit desktop/GROK_RESUME_SESSION or other agent scratch files.

Test Isolation & Test-Code Quality

• Process-wide singletons (IceBridgeTopology, settings factories) must be reset in @BeforeEach/@AfterEach (resetToDefaults()), or equivalent hooks for the module's runner. JVM forks run many test classes; leaked singleton state = order-dependent failures that only appear on CI.
• A test's extractor is code too — a greedy regex [^>]* swallows the / of self-closing XML tags and hides every following key from parity checks (AndroidStringResourceParityTest blind spot, 2026-07). Use [^>]*? and test the extractor itself.
• Spotless ratchet expectation — verify the current ratchet (historically origin/master): touching a legacy file may format the WHOLE file. Expect large diffs; keep mechanical reformatting separate from product logic, not hidden inside an unrelated test/docs commit. Only the coordinator runs formatting in a shared tree.

Benchmarks & Simulations

• Common random numbers — every candidate in a comparative run gets the SAME seed(s) so the parameter is the only variable. Per-candidate seeds put each candidate on a different random network and the winner becomes seed luck (TopologyAutoResearch, 2026-07).
• Check saturation before trusting rankings — if every candidate visits the whole network, the parameter only discriminates cost, not coverage; draw recall conclusions only from networks big enough for the parameter to gate coverage.

9. Git & Commit Hygiene

Granular, Focused Commits

• One logical change per commit, only when authorized. Example: "remove ip_filter.db persistence" and "move JNI calls off EDT" were split into two separate commits even though they were done together. Keep tightly-coupled edits in one buildable commit rather than splitting arbitrarily.
• A branch with 35+ granular commits is normal for a major sweep. Squashing cosmetic / WIP commits requires explicit history-rewrite permission; never do it automatically before pushing.
• Typical mesh security sweep review order: protocol primitive → manager policy → wire version → tests → changelog. This is not a mandate to split dependent changes into broken commits.

Commit Message Format

[desktop] short imperative description (#issue)
[android] short imperative description (#issue)
[common]  short imperative description (#issue)
[all]     short imperative description

• Prefix with scope tag in brackets: [desktop], [android], [common], or [all].
• Reference issue numbers so GitHub auto-links them: (#1291).
• Imperative mood: "Fix NPE" not "Fixed NPE" or "Fixes NPE".
• Append the exact LLM model identifier used to author the commit at the end of every commit message, for example openai/gpt-5.6-luna.
• First line ≤ 72 chars. Body wraps at 72. Use a blank line between summary and body.

Clean History

• Git write operations require explicit permission: staging, commits, pushes, tags, checkout/branch changes, pulls/fetches, rebases, squashes and other history rewriting. Read-only git show, status, diff and log are inspection, not authorization to mutate the tree or history.
• Only when rewriting a remote branch is explicitly approved, prefer git push --force-with-lease (not --force); still inspect remote tracking and coordinate other contributors first. Never automatically force-push or amend.
• Prefer rebasing a feature branch on master rather than merging master into it when the agreed workflow permits it. The historical sequence below is an example requiring approval for each operation and a safe worktree, not an instruction to run it automatically:

  git fetch origin master
  git checkout my-branch
  git rebase origin/master
  

Build Artifacts Lie by Omission

• ./gradlew "up-to-date" means the artifact matches THAT checkout's sources — not that the checkout is current, and not that a rebuild happened. When verifying a deploy, the artifact's own version banner/output is the witness: a stale EC2 icebridge.jar was caught by its missing software-version banner block despite "BUILD SUCCESSFUL, 3 up-to-date" (2026-07). With permission, update the checkout; have the coordinator rebuild and re-check the banner.

Feature Branches

• All pull requests come from a feature branch on your fork.
• Name descriptively: issue-1291, fix-ip-filter-edt-crash, media-player-update. Lowercase, hyphen-separated.
• One branch = one logical unit. Don't mix tech-debt and feature work.

10. Maintainer's Checklist (What to Update per Change)

Before claiming a change is complete, walk this list. Most rows are per-change; some are per-release. Commit/release rows are conditional on explicit authorization, not permission to perform those operations.

Per Commit

• Compiles clean on the target it touches
• All new public methods have tests
• All new user-facing strings are i18n'd
• Heavy work is off the UI thread
• Native resources are deterministically cleaned up
• Commit message uses scope prefix [android] / [desktop] / [common] / [all]
• Commit subject is imperative mood
• No @SuppressWarnings("unused") on private helpers — deleted the dead code instead
• No diagnostic System.out / System.err / printStackTrace — using com.frostwire.util.Logger; intentional CLI help/output only
• No magic numbers/strings — extracted to named constants
• No reinvented utility — grep com.frostwire.util.* before writing helpers
• No wildcard imports

Per User-Facing Change

• desktop/changelog.txt updated (if Desktop)
• android/changelog.txt updated (if Android)
• Most important change listed first in the changelog block
• Visual changes tested in light + dark themes

Per Release

• Build verified on all targets (Desktop, Android, JLibTorrent native for affected arch)
• PR description explains the issue and the how of the fix
• No formatting noise mixed with product fixes
• No commits that bundle tech-debt with product behavior
• Branch/base status reviewed against current master; any rebase/history rewrite separately authorized and coordinated

Per Security-Sensitive Change

• No private keys, certificates (.pem, .crt, .key, .p12), API secrets committed
• git status, git diff, git log --oneline -10, and full staged diff plus git diff --cached --stat reviewed before every authorized commit; stage only intended files
• Threat model updated if a new attack surface appears
• Web-of-Trust root keys stored outside the repo if added
• New or changed rUDP packet types: packet integrity plus session auth, size/rate bounds, and a rejection test for unauthenticated senders
• Multi-hop flood paths: hop TTL, fan-out, payload cap, and rate limit all present
• MentisDB frostwire chain: search before recording LessonLearned / Constraint for non-obvious security fixes (agent_id gubatron)

11. Pull Request & Review Culture

PR Description

• Explain what issue you're fixing and how you fixed it in detail.
• If it's too hard to explain, simplify the solution.
• Don't include formatting noise — it makes review impossible for a small team.
• Reference the issue number in the body, not just the title.

Reviewer Checklist

A reviewer should be able to assess these from the diff and recorded verification evidence; source inspection alone cannot prove that a build passes:

• Build passes locally, with exact command/environment/result evidence
• No formatting noise in the diff
• Tests cover happy + edge cases
• UI strings are i18n'd
• Heavy work is off the main/UI thread
• Native resources are cleaned up deterministically
• changelog.txt updated if user-facing
• Commit messages follow [scope] imperative (#issue)
• No new public API is added without a test

A reviewer is encouraged to push back on complexity, not just correctness. "Can this be smaller?" is a valid question.

12. Dependency & Build Hygiene

• Update Gradle plugins, build tools, and third-party SDKs one at a time. Verify builds on all targets after each bump.
• Avoid duplicate resources in build.gradle.
• Do not introduce new modules or major dependencies without prior discussion in the issue tracker.
• Build verification matrix (coordinator-only Gradle in a shared tree; verify flavor/task names against current build files):
  • Desktop, from desktop/: ./gradlew compileJava then ./gradlew test, plus ./gradlew spotlessCheck
  • Android, from android/: ./gradlew assembleDebug (or the affected explicit flavor task), ./gradlew compilePlus1DebugJavaWithJavac, then ./gradlew testPlus1DebugUnitTest
  • JLibTorrent: native builds must pass on macOS (arm64 + x86_64), Linux (x86_64 + arm64), Windows (x86_64), Android (4 arches: armeabi-v7a, arm64-v8a, x86, x86_64; verify the current supported ABI set)
• Shared changes need both targets; native changes need the affected platform/architecture matrix. Record unavailable gates rather than claiming cross-platform success.

13. What This Skill Is Not

• Not a substitute for reading the existing code in the area you're changing. Read the file you're editing end-to-end before writing a line.
• Not exhaustive. It's a starting checklist. Real review comments from gubatron and aldenml are the source of truth — capture them in mentisdb so they survive.
• Not a substitute for design docs. Large features need a DESIGN_*.md first (see DESIGN_RELAY_REGISTRY.md for the template).
• Not a code-size budget for accumulated knowle

…(truncated)