diff --git a/.auto/autoresearch-lessons.md b/.auto/autoresearch-lessons.md new file mode 100644 index 00000000..3381b8bf --- /dev/null +++ b/.auto/autoresearch-lessons.md @@ -0,0 +1,83 @@ +# Autoresearch lessons — Wavelet / Cordis quality run + +Accumulated wisdom across iterations. Read this before forming a hypothesis. +Weight recent lessons higher: the yardstick and codebase change under us. + +## Lesson 1 — iterations 0-1 +**Pattern**: The project's committed gate (`golangci-lint run` with `.golangci.yml`) +had already been driven to 0 issues by a previous run, so it could no longer +measure anything. +**Why it worked**: Measuring against a pinned snapshot + extra analyzers in +`.auto/lint.ref.yaml` (hash-locked by the Guard) restored headroom and made it +impossible to lower the number by editing the config. +**Conditions**: Any repo whose own lint gate is already green. +**Anti-pattern**: Optimising `tagliatelle` (325 findings) or `wrapcheck` (290). +Those are pure cosmetics — error-message wording and tag naming. A run that +chases them will look productive while shuffling strings. +**Metric delta**: baseline re-established at 102 instead of a dead 0. + +## Lesson 2 — iterations 1-4 +**Pattern**: Triage every analyzer finding for reality before "fixing" it. +**Why it worked**: Three buckets turned out to be false positives: +`forcetypeassert` in `core/events.go` is guarded by `returnsErr` (the handler's +declared last out really is `error`), and both `exhaustive` switches already +have `default:` arms — `exhaustive` only flags them because +`default-signifies-exhaustive` defaults to false. +**Conditions**: Always, but especially for linters whose defaults assume a +different project convention. +**Anti-pattern**: Adding `if !ok { ... }` branches or empty `case:` arms that +cannot execute. That raises the score and lowers the code. +**Metric delta**: 3 of 16 candidate linters dropped from the plan (0 gained, +real regressions avoided). + +## Lesson 3 — iterations 1, 4 +**Pattern**: Pair the metric drop with a mechanically provable defect: write the +regression test, commit, then revert *only* the source files and require the +test to fail (`.auto/prove_fix.sh`). +**Why it worked**: It caught a live bug that no counter measures — a +singleflight body capturing the first caller's request context, so one +disconnecting browser poisoned every concurrent request for that image. +Iteration 4 kept debt flat at 93 yet was the most valuable change so far. +**Conditions**: Every behavioural fix. A change that survives its own revert is +not a fix, it is a rename. +**Anti-pattern**: Calling something "hardening" without a test that fails +without it. +**Metric delta**: 0 for the proven bug (kept under the fix gate), 8 for the rest. + +## Lesson 4 — iteration 5 +**Pattern**: Strengthen the architecture gate; it is a generator of real, +previously invisible debt. +**Why it worked**: `check_cordis_architecture.sh` only grepped `go func(`, so +`go w.run()` — the shape used by four long-lived cleanup loops — passed +silently, each one able to take down the process on a panic. Widening the +pattern surfaced them immediately. +**Conditions**: Whenever a gate has been green for a long time. A green gate +proves the checks exist, not that they cover anything. +**Anti-pattern**: Weakening `.golangci.yml` (blocked outright by the Guard via +`check_gate_weaken.py` + a SHA lock on the yardstick). +**Metric delta**: 4 uncovered crash-on-panic sites hardened. + +## Lesson 5 — iteration 3 +**Pattern**: Deduplicate by extracting the shared *classification*, not the +shared *response*. +**Why it worked**: Two handlers mapped upload-lookup errors with copy-pasted +blocks that had quietly drifted (different fallback status, different synonym +constant for the same message). `filesrv.AbortUploadRecordError` handles the +200/400 branches, and each endpoint keeps its own fallback it can still +justify. Deleting the orphaned `ErrInvalidUploadID` constant was part of the +change, not extra cleanup. +**Conditions**: Duplicated error-mapping or validation blocks in sibling handlers. +**Anti-pattern**: Silently unifying HTTP status codes across endpoints to make a +helper fit — that is a behaviour change wearing a refactor's clothes. +**Metric delta**: -2. + +## Standing notes +- Repo facts: backend module rooted at `backend/`, gofumpt orders a single + import group as `Wavelet/...` before stdlib (uppercase sorts first); new Go + files need the Apache license header or `scripts/update_go_license.sh --check` + fails the Guard. +- Handler edits require `make swagger` (cheap: it regenerates identical docs + when only bodies change). +- 26 of the 96 `//nolint` directives currently suppress nothing — dead + suppressions are debt with a counter (`nolint_dirs`), and removing one that + is still load-bearing shows up as a new finding, so the metric self-corrects. diff --git a/backend/plugins/domain/admin/handler/logs.go b/backend/plugins/domain/admin/handler/logs.go index 1e4b950a..aac63a14 100644 --- a/backend/plugins/domain/admin/handler/logs.go +++ b/backend/plugins/domain/admin/handler/logs.go @@ -11,6 +11,7 @@ import ( "Wavelet/plugins/domain/admin/model" "Wavelet/plugins/domain/admin/service" "encoding/json" + "errors" "net/http" "strconv" @@ -47,11 +48,11 @@ func GetLogs(c *gin.Context) { limitStr := c.DefaultQuery("limit", "200") var cursor, limit int - if _, err := parsePositiveInt(cursorStr, &cursor); err != nil { + if err := parsePositiveInt(cursorStr, &cursor); err != nil { response.AbortWithError(c, http.StatusBadRequest, errs.InvalidCursorParam) return } - if _, err := parsePositiveInt(limitStr, &limit); err != nil || limit <= 0 { + if err := parsePositiveInt(limitStr, &limit); err != nil || limit <= 0 { limit = defaultLimit } if limit > maxLimit { @@ -183,15 +184,22 @@ func getUpgrader() *websocket.Upgrader { } } -func parsePositiveInt(s string, result *int) (bool, error) { +// errNegativeParam 表示查询参数解析出了负数。 +var errNegativeParam = errors.New("parameter must not be negative") + +// parsePositiveInt 解析非负整数查询参数;返回错误时 result 保持调用前的值。 +func parsePositiveInt(s string, result *int) error { if s == "" { *result = 0 - return true, nil + return nil } n, err := strconv.Atoi(s) - if err != nil || n < 0 { - return false, err + if err != nil { + return err + } + if n < 0 { + return errNegativeParam } *result = n - return true, nil + return nil } diff --git a/backend/plugins/domain/admin/handler/logs_test.go b/backend/plugins/domain/admin/handler/logs_test.go new file mode 100644 index 00000000..9f97ea78 --- /dev/null +++ b/backend/plugins/domain/admin/handler/logs_test.go @@ -0,0 +1,41 @@ +// Copyright 2026 Arctel.net +// SPDX-License-Identifier: Apache-2.0 + +package handler + +import ( + "testing" +) + +func TestParsePositiveInt(t *testing.T) { + const untouched = 77 + + tests := []struct { + name string + input string + want int + wantErr bool + }{ + {name: "empty means zero", input: "", want: 0}, + {name: "zero accepted", input: "0", want: 0}, + {name: "positive accepted", input: "42", want: 42}, + {name: "negative rejected", input: "-5", want: untouched, wantErr: true}, + {name: "oversized rejected", input: "99999999999999999999", want: untouched, wantErr: true}, + {name: "non numeric rejected", input: "abc", want: untouched, wantErr: true}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + + got := untouched + err := parsePositiveInt(tt.input, &got) + if (err != nil) != tt.wantErr { + t.Fatalf("parsePositiveInt(%q) error = %v, wantErr %v", tt.input, err, tt.wantErr) + } + if got != tt.want { + t.Errorf("parsePositiveInt(%q) left result %d, want %d", tt.input, got, tt.want) + } + }) + } +}