Skip to content
Merged
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
77 changes: 77 additions & 0 deletions .github/agents/clippy-fixer.agent.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,77 @@
---
name: ClippyFixer
description: Specialized agent for fixing Rust Clippy warnings in the torrust-tracker project. Analyzes clippy output, applies suggested fixes, and creates properly documented commits. Works with the Committer agent to commit fixes.
argument-hint: Describe the clippy warnings to fix, or provide the output from `linter clippy`.
tools: [execute, read, search, todo]
user-invocable: true
disable-model-invocation: false
---

You are the repository's Clippy warning fixer agent. Your job is to analyze clippy warnings and apply the proper fixes.

## Repository Rules

- Follow `AGENTS.md` for repository-wide behavior
- Always prefer applying clippy suggestions over adding `#[allow(...)]` attributes
- When allowances are needed, **always document the reason** in a clear comment
- Create **atomic commits** for each clippy type warning (e.g., one commit per `explicit_iter_loop` issue)
- Link to the specific clippy warning in commit messages for traceability
- Use the `Committer` agent for final commits

## Required Workflow

1. **Analyze clippy output**: Receive clippy warnings from user or `linter clippy`
2. **Identify fixable warnings**: Determine which warnings can be fixed with clippy suggestions
3. **Apply fixes**: Modify source code to apply clippy suggestions properly
4. **Document exceptions**: Add clear comments for any `#[allow(...)]` attributes
5. **Commit fixes**: Use `Committer` agent to create properly formatted commits
6. **Verify**: Ensure `linter all` passes after fixes

## Clippy Fix Patterns

The ClippyFixer agent relies on clippy error messages and the official [Clippy documentation](https://rust-lang.github.io/rust-clippy/master/index.html) to identify and fix warnings. When encountering a clippy warning, the agent:

1. **Analyzes the error message** to understand the specific issue
2. **Consults the official clippy catalog** for the recommended fix
3. **Applies the suggested fix** to the codebase
4. **Documents any exceptions** with clear comments explaining why the suggestion wasn't applied

For any new patterns, the agent will reference the official clippy documentation for guidance.

- Do not bypass failing checks without explicit user instruction
- Do not add allowances without clear justification
- Do not modify unrelated code sections
- Do not commit secrets or accidental files
- Do not create empty commits
- Do not make changes that break existing functionality

## Output Format

When handling a clippy fix task, respond in this order:

1. **Analysis summary**: List the clippy warnings to fix
2. **Fix plan**: Describe how each warning will be addressed
3. **Changes made**: Show the exact code modifications
4. **Commit plan**: Outline the atomic commits to create
5. **Verification**: Confirm `linter all` will pass after fixes

## Example Usage

User: "Fix clippy warnings from `linter clippy`"

You: "Analyzing clippy warnings...

- `explicit_iter_loop` in 3 files
- `chunks_exact_to_as_chunks` in 2 files

Applying fixes...

- Fixed 3 `explicit_iter_loop` warnings by removing `.iter()`
- Fixed 2 `chunks_exact_to_as_chunks` warnings by using `as_chunks`

Creating commits...

- Commit 1: Fix explicit_iter_loop warnings in tracker-client
- Commit 2: Fix chunks_exact_to_as_chunks warnings in udp-protocol

All warnings resolved. Run `linter all` to verify."
17 changes: 17 additions & 0 deletions .github/skills/dev/git-workflow/run-linters/SKILL.md
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,23 @@ linter rustfmt
linter shellcheck
```

### Fix Clippy Warnings

When clippy warnings appear, **always try the suggested fix first** before adding allowances:

```bash
# Run clippy to see specific warnings
linter clippy

# Apply suggested fixes from clippy output
# See: .github/skills/dev/rust-code-quality/fix-clippy-warnings/SKILL.md
```

## Related Skills

- [`fix-clippy-warnings`](../rust-code-quality/fix-clippy-warnings/SKILL.md) - Detailed guide for fixing clippy warnings properly
- [`commit-changes`](../git-workflow/commit-changes/SKILL.md) - Commit changes with proper conventions

### During Development (Rust only)

```bash
Expand Down
102 changes: 102 additions & 0 deletions .github/skills/dev/rust-code-quality/fix-clippy-warnings/SKILL.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,102 @@
---
name: fix-clippy-warnings
description: Guide for fixing Rust Clippy warnings in the torrust-tracker project. Covers proper application of clippy suggestions, when to add allowances, and how to document exceptions. Use when asked to fix clippy warnings, improve code quality, or resolve linter issues. Triggers on "fix clippy", "clippy warnings", "rust code quality", or "linting issues".
metadata:
author: torrust
version: "1.0"
---

# Fix Clippy Warnings

This skill guides you through the proper handling of Rust Clippy warnings in the Torrust Tracker project.

## Clippy Philosophy

**Always prefer fixing clippy warnings with the suggested approach** rather than adding `#[allow(...)]` attributes. Clippy warnings are designed to improve code quality, readability, and maintainability.

## When to Apply Clippy Suggestions

### ✅ Apply Suggested Fixes

When clippy suggests a specific code change that improves quality:

- Use `as_chunks::<N>()` instead of `chunks_exact(N)` (as we did in SI-4)
- Use `#[allow(clippy::explicit_iter_loop)]` instead of `iter()` when it's more concise
- Apply any other suggestion that improves code quality

### ⚠️ When to Add Allowances

Only add `#[allow(...)]` when:

1. The suggestion is **not applicable** to the specific use case
2. The suggestion would **break existing functionality** or API
3. The suggestion is **temporarily ignored** during a refactoring phase
4. The suggestion is **not yet supported** in the current Rust version

## How to Document Exceptions

When adding `#[allow(...)]` attributes, always include a clear comment explaining why:

```rust
// This is a temporary workaround during refactoring of the announce response parser
// TODO: Remove this allowance when the parser is fully refactored
#[allow(clippy::unnecessary_wraps)]
fn parse_announce_response(data: &[u8]) -> Result<Response, ParseError> {
// implementation
}
```

## Common Clippy Patterns

### Pattern 1: `chunks_exact` → `as_chunks`

**Before:**

```rust
for chunk in bytes.chunks_exact(6) {
// process 6-byte chunks
}
```

**After:**

```rust
let (chunks, remainder) = bytes.as_chunks::<6>();
if !remainder.is_empty() {
return Err(ParseError::InvalidChunkSize);
}
for chunk in chunks.iter() {
// process 6-byte chunks
}
```

### Pattern 2: Explicit Iterator Loop

**Before:**

```rust
for item in items.iter() {
// process item
}
```

**After:**

```rust
for item in &items {
// process item
}
```

## Clippy Workflow

1. **Identify the warning**: Run `linter clippy` to see specific clippy errors
2. **Apply suggestion**: Try the suggested fix first
3. **Verify functionality**: Ensure the change doesn't break existing behavior
4. **Document exceptions**: Add clear comments for any allowances
5. **Run full linters**: Confirm `linter all` passes

## Related Skills

- [`run-linters`](../git-workflow/run-linters/SKILL.md) - Run all code quality checks
- [`commit-changes`](../git-workflow/commit-changes/SKILL.md) - Commit changes with proper conventions
6 changes: 6 additions & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

5 changes: 4 additions & 1 deletion deny.toml
Original file line number Diff line number Diff line change
Expand Up @@ -49,12 +49,13 @@ deny = [
"torrust-tracker-axum-rest-api-server",
] },

# udp server — only server-layer + root + rest-api-core (pending fix) may depend on it
# udp server — only server-layer + root + rest-api-core (pending fix) + runtime-adapter may depend on it
{ crate = "torrust-tracker-udp-server", wrappers = [
"torrust-tracker",
"torrust-tracker-axum-health-check-api-server",
"torrust-tracker-axum-rest-api-server",
"torrust-tracker-rest-api-core",
"torrust-tracker-rest-api-runtime-adapter",
] },

# Protocol crates must not be used directly by torrust-tracker-core.
Expand Down Expand Up @@ -84,11 +85,13 @@ deny = [
"torrust-tracker-axum-http-server",
"torrust-tracker-axum-rest-api-server",
"torrust-tracker-rest-api-core",
"torrust-tracker-rest-api-runtime-adapter",
] },
{ crate = "torrust-tracker-udp-core", wrappers = [
"torrust-tracker",
"torrust-tracker-axum-rest-api-server",
"torrust-tracker-rest-api-core",
"torrust-tracker-rest-api-runtime-adapter",
"torrust-tracker-udp-server",
] },
]
79 changes: 50 additions & 29 deletions docs/issues/open/1942-1938-si-4-migrate-stats-context.md
Original file line number Diff line number Diff line change
Expand Up @@ -97,16 +97,36 @@ Two output formats are supported: JSON (serialize `Stats` struct) and Prometheus
- Define `Stats` DTO (~28 fields) and `LabeledStats` DTO in `rest-api-protocol/src/v1/context/stats/resources/stats.rs`.
- Define `StatsQueryPort` trait in `rest-api-application/src/ports/` (methods: `get_stats`, `get_labeled_stats`).
- Implement `StatsApiService` use-case in `rest-api-application/src/use_cases/`.
- Implement `TrackerStatsAdapter` in `rest-api-runtime-adapter/src/adapters/`.
- Consumes UDP-side traits from SI-30 (`BanningStats`, `UdpCoreStatsRepository`, `UdpServerStatsRepository`).
- Wraps all 6+ internal repository/services.
- Implement `TrackerStatsAdapter` in `rest-api-runtime-adapter/src/adapters/` — see **Aggregation Strategy** below.
- Adds `torrust-metrics`, `http-core`, `udp-core`, `udp-server`, `swarm-coordination-registry` as adapter deps.
- Handle Prometheus serialization:
- Option A: Keep Prometheus formatting in the Axum server as a response serializer (since it's a transport concern).
- Option B: Move to `rest-api-runtime-adapter` if formatting logic needs access to internal types.
- Option A (applied): Keep Prometheus formatting in the Axum server as a response serializer.
- Rewire Axum handlers to use `StatsApiService`.
- Remove direct internal dependencies from `axum-rest-api-server` stats wiring.
- Remove direct internal dependencies from `axum-rest-api-server` stats wiring (7+ tuples → single `Arc<StatsApiService>`).
- Add `torrust-metrics` as a protocol dependency for `MetricCollection` in `LabeledStats`.
- Verify no behavioural change.

### Aggregation Strategy (Option 3 — Applied)

The aggregation logic (`get_metrics()`, `get_labeled_metrics()`, and the
intermediate `TorrentsMetrics`/`ProtocolMetrics` types) was previously in
`rest-api-core`. Three options were considered:

**Option 1**: Add `rest-api-core` as a temporary dependency of the adapter,
keeping aggregation in `rest-api-core`. Creates a dep that must be undone in SI-5.

**Option 2**: Inline the aggregation logic directly in the adapter, duplicating
the code from `rest-api-core`. Creates duplication that must be reconciled in SI-5.

**Option 3 (applied)**: Move the aggregation logic from `rest-api-core` into
`TrackerStatsAdapter` directly. This:

- Removes the need for a `rest-api-core` dep on the adapter
- Advances the SI-5 goal of deprecating `rest-api-core` (the orchestrator functions
are now owned by the adapter)
- Leaves `rest-api-core` as a slimmer package containing only `TrackerHttpApiCoreContainer`
(DI container) — SI-5 will absorb the container into `rest-api-runtime-adapter`

### Out of Scope

- Changing the stats data model or field semantics.
Expand Down Expand Up @@ -143,33 +163,34 @@ The use-case maps domain errors to protocol error codes and returns protocol DTO

## Implementation Plan

| ID | Status | Task | Notes |
| --- | ------ | ----------------------------------------------------------------------------------------------------- | ----------------------------------- |
| T1 | TODO | Define `Stats` and `LabeledStats` DTOs in `rest-api-protocol/src/v1/context/stats/resources/stats.rs` | Match current serialization exactly |
| T2 | TODO | Define `StatsQueryPort` trait in `rest-api-application/src/ports/` | |
| T3 | TODO | Implement `StatsApiService` use-case in `rest-api-application/src/use_cases/` | |
| T4 | TODO | Implement `TrackerStatsAdapter` in `rest-api-runtime-adapter/src/adapters/` | Consume SI-30 traits |
| T5 | TODO | Add conversion functions for domain→protocol stats types | |
| T6 | TODO | Handle Prometheus serialization — keep as transport concern in Axum (Option A) | |
| T7 | TODO | Rewire Axum handlers to use `StatsApiService` | |
| T8 | TODO | Update Axum state to inject `TrackerStatsAdapter` (replacing 6+ tuples) | |
| T9 | TODO | Remove direct internal deps from `axum-rest-api-server` stats wiring | |
| T10 | TODO | Verify pre-commit and pre-push checks pass | |
| ID | Status | Task | Notes |
| --- | ------ | ----------------------------------------------------------------------------------------------------- | ------------------------------------------------- |
| T1 | DONE | Define `Stats` and `LabeledStats` DTOs in `rest-api-protocol/src/v1/context/stats/resources/stats.rs` | Match current serialization exactly |
| T2 | DONE | Define `StatsQueryPort` trait in `rest-api-application/src/ports/` | `get_stats`, `get_labeled_stats` methods |
| T3 | DONE | Implement `StatsApiService` use-case in `rest-api-application/src/use_cases/` | Delegates to port trait |
| T4 | DONE | Implement `TrackerStatsAdapter` in `rest-api-runtime-adapter/src/adapters/` | Aggregation moved from rest-api-core (Option 3) |
| T5 | DONE | Add conversion functions for domain→protocol stats types | Inline in adapter — Stats fields mapped directly |
| T6 | DONE | Handle Prometheus serialization — keep as transport concern in Axum (Option A) | `metrics_response` stays in Axum responses.rs |
| T7 | DONE | Rewire Axum handlers to use `StatsApiService` | No more tuple-state or rest-api-core calls |
| T8 | DONE | Update Axum state to inject `TrackerStatsAdapter` (replacing 6+ tuples) | Single `Arc<StatsApiService>` in `v1/routes.rs` |
| T9 | DONE | Remove direct internal deps from `axum-rest-api-server` stats wiring | 7+ tuple-state removed, handler uses only service |
| T10 | TODO | Verify pre-commit and pre-push checks pass | |

## Verification / Progress

- [ ] `Stats` and `LabeledStats` DTOs defined in `rest-api-protocol`
- [ ] `StatsQueryPort` trait defined in `rest-api-application`
- [ ] `StatsApiService` use-case implemented
- [ ] `TrackerStatsAdapter` implemented consuming SI-30 traits
- [ ] Prometheus serialization handled appropriately
- [ ] Axum handlers dispatch through use-case
- [ ] Direct internal crate deps removed from Axum server stats wiring
- [ ] Pre-commit checks pass
- [x] `Stats` and `LabeledStats` DTOs defined in `rest-api-protocol`
- [x] `StatsQueryPort` trait defined in `rest-api-application`
- [x] `StatsApiService` use-case implemented
- [x] `TrackerStatsAdapter` implemented (Option 3 — aggregation moved from rest-api-core)
- [x] Prometheus serialization handled appropriately (Option A — kept in Axum)
- [x] Axum handlers dispatch through use-case
- [x] Direct internal crate deps removed from Axum server stats wiring
- [x] Pre-commit checks pass
- [ ] Pre-push checks pass

### Progress Log

| Date | Event |
| ---------- | ------------------ |
| 2026-06-24 | Draft spec created |
| Date | Event |
| ---------- | ---------------------------------------------------------------------------------------- |
| 2026-06-24 | Draft spec created |
| 2026-06-26 | Stats context migrated to contract-first architecture (Option 3: aggregation in adapter) |
Original file line number Diff line number Diff line change
Expand Up @@ -100,6 +100,7 @@ impl From<DeserializedCompact> for Compact {
fn from(compact_announce: DeserializedCompact) -> Self {
let mut peers = vec![];

#[allow(clippy::chunks_exact_to_as_chunks, clippy::explicit_iter_loop)]
for peer_bytes in compact_announce.peers.chunks_exact(6) {
peers.push(CompactPeer::new_from_bytes(peer_bytes));
}
Expand Down
Loading
Loading