refactor: reduce Go complexity, deduplicate handlers/repos, and enfor…

.agents/rules/go-engineering-standards.md

@@ -0,0 +1,71 @@
+# SelfCloud Go Engineering & Maintainability Standards
+
+All AI agents and contributors writing or modifying Go code in SelfCloud (`cmd/...`, `internal/...`, `test/...`) MUST adhere to these engineering standards. These rules are enforced automatically by `golangci-lint` via `make lint` and `make ci`.
+
+---
+
+## 1. Complexity & Function Size Limits
+
+Every non-test Go function must stay within these hard limits:
+
+| Metric | Linter | Maximum Threshold | Required Action When Approaching Limit |
+| :--- | :---: | :---: | :--- |
+| **Cognitive Complexity** | `gocognit` | **20** | Extract sub-steps, loop bodies, or validation blocks into named helper functions. |
+| **Cyclomatic Complexity** | `gocyclo` | **15** | Decompose `switch`/branching dispatchers into per-case handler functions. |
+| **Function Length** | `funlen` | **80 lines / 50 stmts** | Split setup, I/O execution, transaction persistence, and response rendering. |
+| **Nesting Depth** | `nestif` | **4 levels** | Invert conditions into early-return guard clauses (`if err != nil { return err }`). |
+
+### Required Structural Patterns
+1. **Guard Clauses Over `else` Chains**: Never write `if cond { ... return } else { ... }`. Return or `continue` early so the happy path remains at the top indentation level.
+2. **Three-Phase HTTP / WebDAV Handlers**:
+   - **Phase 1 (Validate)**: Parse request inputs, validate path length (`pathutil.ValidatePathLength`), and check preconditions.
+   - **Phase 2 (Execute)**: Perform filesystem I/O outside the DB transaction where possible, then persist metadata atomically via a focused helper (`persist*`, `save*`, `deleteNodeMetadata`).
+   - **Phase 3 (Respond)**: Record the audit event (`audit.Record` / `audit.RecordFailure`) and write the HTTP response.
+3. **Context Structs for Recursive / Multi-Step Operations**: When recursive directory walks or reconciliation loops require more than 4 arguments, group shared dependencies (`ctx`, `user`, repositories, state maps) into a small unexported struct (e.g., `dirCopyContext`, `orphanPruner`) with small methods (`walk`, `copyChild`).
+4. **No Inline Large Templates or Migrations**: Keep HTML/CSS templates and multi-statement SQL schemas in package-level `const` declarations or dedicated per-version migration helpers (`migrateV1`, `migrateV2`, etc.).
+
+---
+
+## 2. Zero Duplication & Constant Hygiene
+
+1. **No Copy-Paste Blocks (`dupl` threshold: 80 tokens)**:
+   - In `internal/repository`, use shared query/mutation helpers (`getUserWhere`, `getTokenWhere`, `deleteTokensWhere`, `deleteSessionsWhere`) for single-row `goqu` lookups and conditional deletes.
+   - In `internal/api`, use shared helpers in `internal/api/helpers.go` (`writeJSON`, `writeJSONError`, `writeStatusOK`, `resolveBaseURL`).
+   - In `internal/middleware`, share session token validation (`resolveSessionUser`) across cookie and Bearer authentication flows.
+2. **Named Constants for Repeated Literals (`goconst`: max 3 occurrences of strings $\ge 4$ chars)**:
+   - Use package-level column constants (`colUserID`, `colPath`, `colUpdatedAt`, `colToken` in `internal/repository/repository.go`) for repeated SQL column names.
+   - Define protocol and header constants (`schemeHTTP`, `schemeHTTPS`) rather than repeating magic strings.
+
+---
+
+## 3. Go 1.26 Idioms & Clean Code Rules
+
+1. **Error Construction & Inspection (`errorlint`, `revive`, `gocritic`)**:
+   - Use `errors.New("static message")` when no formatting verbs are needed (never `fmt.Errorf("static message")`).
+   - Always wrap errors with `%w` (`fmt.Errorf("context: %w", err)`).
+   - Always inspect errors with `errors.Is(err, target)` or `errors.As(err, &target)` (never `err == sql.ErrNoRows` or direct type assertions).
+   - Keep error strings lowercase without trailing punctuation.
+2. **String Formatting (`perfsprint`, `gocritic`)**:
+   - Use `%q` in `fmt.Sprintf` instead of manual escaped quotes `\"%s\"`.
+3. **Octal File Permissions (`gocritic`)**:
+   - Always use Go 1.26 octal literal syntax `0o755`, `0o644`, `0o600` (never legacy `0755`).
+4. **Unused Parameters, Receivers & Shadowing (`unparam`, `revive`, ` gocritic`)**:
+   - Remove unused parameters from internal functions. If a signature is fixed by an interface or `http.HandlerFunc`, rename the unused parameter to `_`.
+   - If a method does not access struct fields, omit the receiver variable name (`func (*Handler) helper(...)`).
+   - Never shadow imported package names (`storage`, `path`, `url`, `context`, `db`) with local variable names.
+5. **Resource Cleanup & `defer` Safety**:
+   - Never call `os.Exit` inside a function that registers `defer` cleanup statements (such as `defer sqlDB.Close()`). Return an `error` to `main()` and let `main()` exit after all `defer` statements execute.
+
+---
+
+## 4. Mandatory Verification Before Completing Any Task
+
+Whenever you add or modify Go code, you MUST run:
+```bash
+make lint && make test
+```
+Or for full-stack / pre-commit verification:
+```bash
+make ci
+```
+If any maintainability linter (`gocognit`, `gocyclo`, `funlen`, `nestif`, `dupl`, `goconst`, `gocritic`, `revive`, `errorlint`, `unparam`, `perfsprint`) reports an issue, refactor the code immediately—never add `//nolint` directives unless there is an unavoidable external interface constraint.

.agents/skills/go-maintainability-review/SKILL.md

@@ -0,0 +1,82 @@
+---
+name: go-maintainability-review
+description: >-
+  Audits and improves Go code maintainability in SelfCloud by detecting copy-paste
+  duplication, high cyclomatic/cognitive complexity, deeply nested control flow,
+  and Go clean code anti-patterns. Use this skill whenever the user asks to check,
+  review, audit, or refactor Go code for maintainability, clean code, complexity
+  reduction, or deduplication.
+---
+
+# Go Maintainability Review & Refactoring Skill
+
+This skill provides a fast, deterministic workflow optimized for **Gemini 3.8 Flash** (and other models) to find and fix Go maintainability issues in SelfCloud without introducing regressions or over-abstraction.
+
+---
+
+## Mode Selection
+
+Determine the operating mode from the user's prompt before starting:
+
+1. **Report Mode (Default)**: Used when the user asks to *check*, *find*, *review*, or *audit* Go code for maintainability improvements. Do **not** modify source files; produce a prioritized maintainability report with concrete refactoring sketches.
+2. **Refactor Mode**: Used when the user explicitly asks to *fix*, *refactor*, or *apply* maintainability improvements (either across a package or for specific findings). Apply focused refactorings and verify with `make lint` and `make test`.
+
+---
+
+## Step-by-Step Workflow
+
+### Phase 1: Run the Static Maintainability Scan
+
+Always begin by running the helper script to gather complexity, duplication, and clean-code diagnostics in a single pass:
+
+```bash
+./.agents/skills/go-maintainability-review/scripts/scan_maintainability.sh [optional_package_path...]
+```
+
+- By default, the script scans `./internal/...` and `./cmd/...` (excluding `_test.go` to keep signal high) using [golangci-maintainability.yml](./resources/golangci-maintainability.yml).
+- Pass `--include-tests` if the user also wants test files analyzed.
+- Do **not** manually grep or read every Go file in the repository; rely on the scan output to pinpoint exact file and line hotspots.
+
+### Phase 2: Targeted Code Inspection & Checklist Review
+
+1. Read [go-clean-code-checklist.md](./references/go-clean-code-checklist.md) to load the refactoring rules, Go 1.26 idioms, and SelfCloud invariants.
+2. Select the highest-signal hotspots from the scan output across the three core categories:
+   - **Copy-Paste / Duplication (`dupl`, `goconst`)**: Repeated repository query blocks, duplicated handler utilities (e.g., identical base URL resolution or error responses across handlers), or repeated validation blocks.
+   - **Cyclomatic & Cognitive Complexity (`gocyclo > 15`, `gocognit > 20`, `nestif >= 4`, `funlen`)**: Monolithic CLI dispatchers (`cmdUser`, `cmdServe`), deep `if/else` trees in HTTP/WebDAV handlers or auth middleware (`DualAuthMiddleware`, `GoogleCallbackHandler`, `UploadHandler`, `handleCopyDir`, `ReindexUser`).
+   - **Go Clean Code Practices (`gocritic`, `revive`, `errorlint`, `unparam`, `perfsprint`)**: `os.Exit` after `defer` (`exitAfterDefer`), unused internal helper parameters, package import shadowing (`storage`), `if-else` chains replaceable by `switch`, and `fmt.Errorf` without format verbs.
+3. Use `read_file` with targeted `StartLine` and `EndLine` ranges around the flagged functions to inspect their actual control flow and domain context.
+
+### Phase 3: Filter Out False Positives (Anti-Noise Gate)
+
+Before recommending or applying a change, filter out low-value churn:
+- **Do not** merge functions that merely look similar if their domain or SQL semantics will diverge.
+- **Do not** replace readable `goqu.Ex{"user_id": userID}` column names with global string constants just to silence `goconst`. (However, **do** investigate when `goconst` flags repeated URLs/schemes like `https` or `localhost:8080` across multiple handlers, which indicates a copy-pasted helper function).
+- **Do not** alter standard `http.HandlerFunc` signatures (`w http.ResponseWriter, r *http.Request`) when `r` is unused; focus `unparam` fixes on internal helper functions.
+- **Never** break SelfCloud invariants defined in `AGENTS.md` (audit logging order via `audit.Record(ctx, ...)`, `goqu` node update fields, symlink & path length checks).
+
+### Phase 4A: Output Format (Report Mode)
+
+Group findings into three clear sections prioritized by maintainability impact (**High → Medium → Low**). Use clickable `file://` links for all files and symbols:
+
+1. **Copy-Paste & Code Duplication Opportunities**
+   - Location(s) with line ranges (`file:///.../path.go#L10-L40`).
+   - Why it is duplicated and the exact helper signature to extract.
+2. **Cyclomatic & Cognitive Complexity Hotspots**
+   - Function link + current metrics (e.g., `gocyclo: 68`, `gocognit: 125`, `lines: 146`).
+   - Root cause of complexity (e.g., inline subcommand switch, deeply nested `if/else`, mixed parsing and storage logic).
+   - Concrete decomposition plan (which helper functions or guard clauses to extract).
+3. **Go Clean Code & Idiom Improvements**
+   - Grouped summary of actionable clean-code fixes (`exitAfterDefer`, unused helper parameters, import shadowing, early returns, modern Go idioms).
+
+### Phase 4B: Execution & Verification (Refactor Mode)
+
+When applying refactorings:
+1. Refactor **one logical unit at a time** (e.g., deduplicate one repository or decompose one complex handler/CLI function).
+2. Re-run the maintainability scan on the modified package to confirm complexity/duplication dropped:
+   ```bash
+   ./.agents/skills/go-maintainability-review/scripts/scan_maintainability.sh ./internal/<pkg>/...
+   ```
+3. Run the project's standard linter and test suite to verify zero regressions:
+   ```bash
+   make lint && make test
+   ```

.agents/skills/go-maintainability-review/references/go-clean-code-checklist.md

@@ -0,0 +1,77 @@
+# Go Maintainability & Clean Code Checklist (SelfCloud)
+
+Use this reference during **Phase 2 (Semantic Inspection)** and **Phase 3 (Filtering & Refactoring)** of the `go-maintainability-review` skill.
+
+---
+
+## 1. Reducing Copy-Paste & Duplication (DRY vs. Premature Abstraction)
+
+### High-Value Deduplication Targets
+1. **Repository Query Helpers (`internal/repository/`)**:
+   - **Pattern**: Multiple repository methods (`GetByUsername` vs `GetByID`, `GetTokenByAccessToken` vs `GetTokenByRefreshToken`, `DeleteByUserID` across repos) that build the exact same `Select(...)`, execute `QueryRowContext`, `Scan(...)` the same struct fields, and check `errors.Is(err, sql.ErrNoRows)`.
+   - **Fix**: Extract an unexported receiver helper on the repository struct, e.g.:
+     ```go
+     func (r *userRepo) getUserWhere(ctx context.Context, where goqu.Ex) (*domain.User, error)
+     ```
+2. **Duplicated Handler Utilities (`internal/api/`, `internal/webdav/`)**:
+   - **Pattern**: Repeated request-inspection helpers copy-pasted across handlers (e.g., base URL resolution checking `X-Forwarded-Proto` / `r.TLS` / `localhost:8080` in `graph_handler.go`, `oauth_handler.go`, `shares_handler.go`; or identical JSON/XML error response + audit logging blocks).
+   - **Fix**: Consolidate shared handler helpers into a package-level helper file (e.g., `internal/api/helpers.go` or `internal/httputil/`) when the logic is identical in purpose and behavior.
+3. **Repeated Precondition / Validation Blocks**:
+   - **Pattern**: Copy-pasted path normalization, user extraction from context, or node existence checks within the same handler file.
+   - **Fix**: Extract small unexported helper methods on the handler struct.
+
+### When NOT to Deduplicate (False-Positive Guardrails)
+- **Divergent Domain Semantics**: Do not merge two functions just because they look structurally similar if they represent independent domain concepts that will evolve separately (*"A little copying is better than a little dependency"*).
+- **Trivial SQL Literals**: Do not replace clear `goqu` column literals (`goqu.Ex{"user_id": id, "path": p}`) with global string constants (`colUserID`, `colPath`) unless a complex expression is repeated verbatim.
+
+---
+
+## 2. Reducing Cyclomatic & Cognitive Complexity
+
+Target functions where `gocyclo > 15`, `gocognit > 20`, `nestif >= 4`, or `funlen > 80 lines / 50 statements`.
+
+### Refactoring Techniques
+1. **Flatten Arrow Code with Guard Clauses (Early Returns)**:
+   - Invert `if condition { ... 40 lines ... }` to `if !condition { return ... }` so the happy path stays at indentation level 1.
+   - Replace `if err != nil { ... } else { ... }` with an early return inside the `if err != nil` block (`revive: early-return`, `indent-error-flow`).
+2. **Decompose Monolithic HTTP & WebDAV Handlers**:
+   - Split 100+ line handlers (like `UploadHandler`, `GoogleCallbackHandler`, `handleAuthorizePost`, `handlePut`, `handleCopyDir`) into cohesive phases:
+     1. Parse & validate request input.
+     2. Execute domain/storage/repository operations via focused helper methods.
+     3. Record `[AUDIT]` event and write HTTP response.
+3. **Decompose Large CLI Subcommand Dispatchers (`cmd/selfcloud/main.go`)**:
+   - When a CLI dispatcher like `cmdUser` or `cmdServe` exceeds 50+ cyclomatic complexity due to handling many subcommands (`create`, `list`, `delete`, `update`, `set-password`, `set-email`) inline, extract each subcommand into its own dedicated function (`cmdUserCreate`, `cmdUserList`, etc.) and keep `cmdUser` as a thin router.
+4. **Decompose Auth & Middleware Pipelines (`internal/middleware/`)**:
+   - In complex middleware like `DualAuthMiddleware`, extract authentication strategies into dedicated helpers (e.g., `authenticateBearer(...)`, `authenticateBasic(...)`) rather than deeply nesting both flows in a single closure.
+5. **Replace Long `if-else` Chains with `switch`**:
+   - Use tagged or tagless `switch` statements when branching on the same variable or status category (`gocritic: ifElseChain`).
+
+---
+
+## 3. Go 1.26 & Clean Code Practices
+
+1. **Error Handling**:
+   - Always use `errors.New("...")` instead of `fmt.Errorf("...")` when there are no format verbs (`perfsprint: error-format`).
+   - Always wrap underlying errors with `%w` (`fmt.Errorf("failed to ...: %w", err)`) and inspect errors using `errors.Is` / `errors.As` (`errorlint`).
+   - Never call `os.Exit` / `log.Fatal` inside a function after registering `defer sqlDB.Close()` (`gocritic: exitAfterDefer`); instead, return an `error` to `main()` or a wrapper function so deferred cleanup runs before exiting.
+2. **Unused Parameters & Receivers (`unparam`, `revive`)**:
+   - Remove genuinely unused parameters from internal helper methods (e.g., unused `basePath` or `sourceNode` in internal `handleMove`/`handleCopy`/`handleCopyDir` helpers, or constant-only parameters like `generateSecureToken(32)`).
+   - **Exception**: Keep `w http.ResponseWriter, r *http.Request` on standard `http.HandlerFunc` methods even if `r` is unused; you may rename `r` to `_` only if it improves clarity without hurting readability.
+3. **Avoid Package Import Shadowing (`gocritic: importShadow`)**:
+   - Rename local variables or parameters named `storage` (e.g., to `store` or `blobStore`) when package `selfcloud/internal/storage` is imported in the same file.
+4. **Modern Go Standard Library Idioms**:
+   - Prefer `strconv.Itoa(n)` over `fmt.Sprintf("%d", n)`.
+   - Prefer `%q` in `fmt.Sprintf` over `"\"%s\""` (`gocritic: sprintfQuotedString`).
+   - Prefer `0o755` / `0o644` octal literals over legacy `0755` (`gocritic: octalLiteral`).
+   - Prefer `http.NoBody` over `nil` in `http.NewRequestWithContext` when sending no body (`gocritic: httpNoBody`).
+   - Prefer `strings.Cut`, `strings.CutPrefix`, `strings.CutSuffix`, `slices.Contains`, `min()`, and `max()` over manual index slicing or verbose loops.
+
+---
+
+## 4. SelfCloud Non-Negotiable Invariants (`AGENTS.md`)
+
+When proposing or applying any maintainability refactoring, **never** violate these project rules:
+1. **Audit & Logging Order**: Always preserve `audit.Record(ctx, ...)` calls before HTTP responses/access logs, and keep `slog` context correlation (`request_id`, `int_request_id`).
+2. **Database Node Invariants**: Any refactoring touching node updates or moves must still update all four fields: `name`, `path`, `parent_path`, and `updated_at`, using `goqu`.
+3. **Security Boundaries**: Never weaken or bypass `checkSymlinkEscape`, `pathutil.ValidatePathLength` (max 700 chars), CSRF origin validation, or `TRUST_PROXY` IP resolution.
+4. **WebDAV & OCS Compliance**: Preserve exact HTTP status codes, headers (`ETag`, `DAV`, `Retry-After`, `WWW-Authenticate`), and XML response structures required by ownCloud clients.

.agents/skills/go-maintainability-review/resources/golangci-maintainability.yml

@@ -0,0 +1,86 @@
+version: "2"
+
+run:
+  timeout: 5m
+  tests: false
+
+linters:
+  default: none
+  enable:
+    - dupl
+    - errorlint
+    - funlen
+    - gocognit
+    - goconst
+    - gocritic
+    - gocyclo
+    - ineffassign
+    - nestif
+    - perfsprint
+    - revive
+    - unparam
+    - unused
+
+  settings:
+    gocyclo:
+      min-complexity: 15
+    gocognit:
+      min-complexity: 20
+    dupl:
+      threshold: 80
+    nestif:
+      min-complexity: 4
+    funlen:
+      lines: 80
+      statements: 50
+      ignore-comments: true
+    goconst:
+      min-len: 4
+      min-occurrences: 4
+    errorlint:
+      errorf: true
+      asserts: true
+      comparison: true
+    perfsprint:
+      strconcat: false
+    gocritic:
+      enabled-tags:
+        - diagnostic
+        - style
+        - performance
+      disabled-checks:
+        - hugeParam
+        - unnamedResult
+        - whyNoLint
+    revive:
+      rules:
+        - name: bool-literal-in-expr
+        - name: constant-logical-expr
+        - name: context-as-argument
+        - name: context-keys-type
+        - name: early-return
+        - name: empty-block
+        - name: empty-lines
+        - name: error-naming
+        - name: error-return
+        - name: error-strings
+        - name: errorf
+        - name: identical-branches
+        - name: if-return
+        - name: increment-decrement
+        - name: indent-error-flow
+        - name: range
+        - name: receiver-naming
+        - name: superfluous-else
+        - name: time-equal
+        - name: unexported-return
+        - name: unnecessary-stmt
+        - name: unreachable-code
+        - name: unused-parameter
+        - name: unused-receiver
+        - name: useless-break
+
+issues:
+  max-issues-per-linter: 0
+  max-same-issues: 0
+  uniq-by-line: false