Improve agent and code review guidance

.github/skills/code-review/SKILL.md

@@ -0,0 +1,74 @@
+---
+name: code-review
+description: >-
+  Review VotifierPlus pull requests, branch diffs, commits, and explicitly
+  included local changes before publishing. Use for code review, pre-PR review,
+  regression review, security review, and PR readiness. Perform an independent
+  source-read-only review and report only concrete P0-P3 defects with precise
+  file/line locations. Do not use this skill to implement fixes.
+---
+
+# VotifierPlus code review
+
+Review the exact proposed change and follow applicable `AGENTS.md`. Treat the Internet-facing listener, protocol parsers, authentication, forwarding, proxy-header handling, throttling, and platform event handoff as high-risk boundaries.
+
+## Boundaries and scope
+
+Do not edit, fix, commit, push, approve, merge, change PR state, or independently post comments. Preserve unrelated work; never stash, reset, clean, rebase, or switch branches. Run only safe bounded local checks; do not bind public ports, contact production servers, use real credentials, or weaken validation.
+
+Resolve the actual PR base/SHA, merge base, review HEAD SHA, commit list, complete patch, changed paths, and worktree state. For PR and branch reviews, review every commit and file in the merge-base-to-HEAD range. For a standalone commit review, scope the review to the requested commit against its first parent (or the explicitly requested commit range), even when a PR base exists or the commit is not checked out. Disclose local overlays, untracked/generated/binary content, missing history, multiple merge bases, conflicts, or truncation. When the task explicitly includes local work, review the applicable staged, unstaged, and intended untracked content as overlays on the pinned commit and assess the effective final code; disclosure alone is not coverage. Pin and recheck the snapshot.
+
+Use a fresh reviewer for substantive changes when supported. Add a bounded security specialist for changes to parsing, authentication, cryptography, PROXY protocol, throttling, forwarding, or workflow privileges.
+
+## VotifierPlus review lenses
+
+Trace attacker-controlled bytes from accept through source identification, protocol selection, size/framing checks, authentication/decryption, semantic validation, throttling, vote construction, forwarding, scheduler handoff, event emission, logging, reload, and shutdown.
+
+- Check connection/admission limits, read/connect/write deadlines, bounded frames/lines/bodies, slow clients, partial reads, malformed encodings, oversized values, EOF, and closure of every socket/stream.
+- Verify v1 RSA versus v2 token selection. Token-only configuration must not accept legacy packets through fallback, ambiguous framing, exception recovery, or forwarding.
+- Verify token/key generation, storage, comparison, rotation/reload, permissions, error messages, and redaction. Encryption alone does not authenticate the claimed service or server.
+- Accept PROXY protocol identity only from configured trusted peers. Ensure throttling, bans, logs, and authorization use the verified source rather than an attacker-supplied address.
+- Test throttle cardinality bounds, expiration, concurrency, tunnel-specific rules, ban escalation, reconnect bypass, log suppression, and cleanup.
+- Validate service names, usernames, addresses, timestamps, token identifiers, server names, and forwarding targets before allocation, logging, event emission, or connection.
+- Check replay, duplicate delivery, forwarding loops, partial multi-target failure, retry behavior, and mixed v1/v2 peers against the documented delivery contract.
+- Verify Bukkit/Folia/BungeeCord/Velocity thread ownership, optional class loading, descriptors, provided Votifier API compatibility, and callbacks after disable.
+- Review reload/listener replacement for duplicate acceptors, port leaks, loss of a previously healthy receiver, stale tokens, and races with active connections.
+- Check workflow least privilege, immutable action pins, dependency/plugin pins, artifact validation, and safe release ordering when CI changes.
+- Missing tests alone are not a finding; demonstrate broken behavior or a defective test contract. Use synthetic packets and credentials, never production data.
+
+## Validation
+
+Confirm current CI/POM requirements. At the time this skill was added:
+
+```shell
+mvn -B -f VotifierPlus/pom.xml package
+```
+
+For a standalone pinned-commit review, validation must run from a detached temporary checkout
+of the requested commit SHA. Resolve and record its first parent, create an isolated temporary
+worktree with `git worktree add --detach`, and run the required build and artifact checks there;
+the currently checked-out worktree and its artifacts cannot provide evidence for that snapshot.
+Run the whitespace check against the exact parent-to-commit range, for example
+`git -C "$review_worktree" diff --check "$parent_sha" "$commit_sha"`. If the request also
+includes local staged, unstaged, or untracked changes, perform separate, explicitly labeled
+overlay checks in the requested overlay checkout/worktree after the pinned-commit evidence is
+complete. Do not substitute an unrelated current-checkout `git diff --check` or build for either
+the pinned range or the requested overlays. Remove the temporary worktree after validation and
+retain the recorded command results and snapshot identifiers.
+
+Record working directory, command, snapshot, exit result, discovered tests, and fresh artifact. For PR/branch reviews, run `git diff --check` on the reviewed range; for standalone reviews, use the parent-to-commit check and any explicitly requested overlay checks described above. Do not count skipped/zero tests, stale artifacts, or another snapshot as proof. Distinguish introduced failures, reproduced baseline failures, and environmental blockers.
+
+## Findings and result
+
+For every candidate, prove a reachable trigger, responsible changed lines, missing guard, expected behavior, and impact. Drop speculation, style preferences, unrelated old defects, and findings contradicted by final code. Use the lowest accurate priority:
+
+- P0: immediately critical, broadly exploitable release blocker.
+- P1: authentication bypass, common remote failure, serious exposure, outage, corruption, deadlock, or major compatibility break.
+- P2: concrete bounded or edge-case correctness, reliability, resource, or security defect.
+- P3: low-impact concrete defect.
+
+Anchor findings to the smallest useful changed-line range and explain trigger, mechanism, and consequence.
+
+Determine completeness separately from findings: report findings when complete; use exactly `No findings.` when complete and clean; use `Review incomplete.` only when required coverage or validation is missing, unresolved, or stale. A static-only review does not satisfy a required build/security gate.
+
+The implementation coordinator fixes accepted findings, reruns validation, and obtains a fresh review. This reviewer never publishes or merges.

.github/skills/code-review/tests/standalone-commit-scope.sh

@@ -0,0 +1,25 @@
+#!/usr/bin/env bash
+set -euo pipefail
+
+skill_file="$(cd "$(dirname "${BASH_SOURCE[0]}")/.." && pwd)/SKILL.md"
+repo_root="$(cd "$(dirname "${BASH_SOURCE[0]}")/../../../../" && pwd)"
+
+require_text() {
+    local text="$1"
+    if ! grep -Fq "$text" "$skill_file"; then
+        printf 'missing required review guidance: %s\n' "$text" >&2
+        exit 1
+    fi
+}
+
+require_text 'git worktree add --detach'
+require_text 'temporary checkout'
+require_text 'currently checked-out worktree and its artifacts cannot provide evidence'
+require_text 'git -C "$review_worktree" diff --check "$parent_sha" "$commit_sha"'
+require_text 'perform separate, explicitly labeled'
+require_text 'Do not substitute an unrelated current-checkout `git diff --check` or build'
+
+grep -Fq 'For PR and branch work, inspect the complete base-to-HEAD diff.' "$repo_root/AGENTS.md"
+grep -Fq 'For standalone commit reviews, inspect the requested commit against its first parent (or the explicitly requested range)' "$repo_root/AGENTS.md"
+
+printf 'standalone commit scope guidance: ok\n'

AGENTS.md

@@ -0,0 +1,53 @@
+# Maintainer and AI-agent guide
+
+VotifierPlus accepts Internet-facing vote submissions and emits or forwards vote events on Bukkit/Paper/Folia, BungeeCord, and Velocity. Treat every socket byte, proxy header, service name, username, token identifier, forwarding target, and configuration value as untrusted input.
+
+## Build and verification
+
+Requirements: JDK 21+ and Maven. The Maven project is in `VotifierPlus/`.
+
+```shell
+mvn -B -f VotifierPlus/pom.xml test
+mvn -B -f VotifierPlus/pom.xml package
+```
+
+Confirm current CI/POM settings. Use `package`, not developer/install profiles that copy artifacts to a server. Verify the fresh shaded JAR, actual test discovery, and `git diff --check`.
+
+## Architecture and trust boundaries
+
+- Bukkit entry point: `com.vexsoftware.votifier.VotifierPlus`.
+- Proxy entry points live under `bungee/` and `velocity/`.
+- `net/VoteReceiver` owns listener lifecycle.
+- Parser, connection-handler, proxy-header, throttle, and forwarding classes divide the inbound network pipeline.
+- `crypto/` owns RSA and token operations.
+- Configuration controls bind host/port, tokens, forwarding targets, trusted tunnel addresses, throttling, and protocol behavior.
+- Votifier events cross from network workers to platform schedulers; event invocation must use the platform-safe context.
+
+## Network and protocol invariants
+
+1. Bound connection count, accept rate, per-client failures, request bytes, line/frame length, parsing work, queues, forwarding batches, and log volume before expensive or authenticated work.
+2. Apply read/connect/write deadlines and close sockets/streams on every success, rejection, timeout, exception, reload, and shutdown path.
+3. Protocol selection must be explicit. Token/v2-only operation must not silently accept legacy v1/RSA packets; changes must not create downgrade or fallback acceptance.
+4. Authenticate the actual protocol identity before emitting or forwarding a vote. Encryption is not authorization.
+5. Compare tokens and other secrets safely; generate them with a cryptographically secure source; never log private keys, tokens, decrypted packets, or forwarding credentials.
+6. PROXY protocol data is authoritative only from explicitly trusted tunnel/source addresses. Untrusted peers must not choose the client IP used by throttling or auditing.
+7. Normalize and validate service, username, address, timestamp, token identifier, server name, and forwarding target without changing established compatibility unexpectedly.
+8. Throttling and bans must use the verified client identity, remain memory-bounded, expire entries, resist cardinality attacks, and avoid bypass through reconnects or spoofed headers.
+9. Forwarding must have its own authentication, bounds, deadlines, replay/duplicate behavior, and partial-failure handling. Never treat a forwarded source as trusted solely because it is another configured server.
+10. Vote delivery must not occur twice because of retry, protocol ambiguity, forwarding loops, reload overlap, or multiple platform handlers.
+11. Reload must establish the new listener safely and retire the old receiver without leaving two acceptors, losing the previous healthy listener on failure, or retaining stale tokens/throttle state unintentionally.
+12. Shutdown must terminate listener and connection workers within a bound and prevent callbacks after disable.
+
+## Platform and compatibility
+
+Keep Bukkit/Paper/Folia, BungeeCord, and Velocity descriptors, entry points, schedulers, event APIs, and configuration behavior aligned where intended. Do not load one platform's classes on another. Preserve the public Votifier event/API compatibility and the `Votifier` provided-plugin identity unless a breaking change is explicitly authorized.
+
+## Build and workflow security
+
+Use least-privilege workflow permissions. Pin third-party actions to immutable commit SHAs where practical, especially dependency-submission or release steps. Pin Maven plugin and security-sensitive dependency versions and avoid `LATEST`/mutable inputs in release builds. Validate artifacts before release publication.
+
+## Change and PR workflow
+
+Before any commit, push, PR update, review reply, or other remote change, run focused protocol/security tests, the full package build, fresh-artifact inspection, and the applicable `git diff --check`. For PR and branch work, inspect the complete base-to-HEAD diff. For standalone commit reviews, inspect the requested commit against its first parent (or the explicitly requested range) instead of substituting a base-to-HEAD branch diff. Before committing local work, also inspect the staged changes and every relevant intended unstaged or untracked change as one effective final patch.
+
+For substantive changes, obtain a fresh source-read-only review. Add a bounded security specialist for parser, authentication, crypto, proxy-header, throttling, forwarding, or workflow-permission changes. The implementation agent fixes accepted findings, reruns validation, and obtains a new review. Do not merge without explicit authorization.