Merge Go frontend (apiserver + xcecli) into meson build

src/frontend/docs/improvement_suggestions.md

@@ -0,0 +1,163 @@
+# Code Improvement Suggestions
+
+> Date: 2026-04-09  
+> Based on current codebase state after all bug fixes.
+
+---
+
+## 1. Critical Bugs Still Present
+
+### 1.1 `queue_name` filter added N times in `GetJobsInfo` (apiserver)
+
+**File:** `controllers/jobs.go` ~line 226  
+**Problem:** The `queue_name:in:(...)` RBAC filter is appended inside the per-filter loop — once for every non-queue filter in the request. With 2 filters (e.g. `job_status` + `submit_user`), it gets added twice. Seen in server logs: `queue_name:in:(normal,priority,short)` appears twice in every query.
+
+**Fix:** Move the `accessQueues` append **outside** the loop, after it completes.
+
+```go
+// BEFORE (inside loop — adds queue_name once per non-queue filter)
+for i, filter := range filters {
+    ...
+    } else {
+        if len(accessQueues) > 0 {
+            filters = append(filters, ...)  // BUG: runs N times
+        }
+    }
+}
+
+// AFTER (outside loop — adds once)
+for i, filter := range filters { ... }
+if len(accessQueues) > 0 && !queueFilterPresent {
+    filters = append(filters, utils.FilterCondition{
+        Field: "queue_name", Operator: "in",
+        Value: "(" + strings.Join(accessQueues, ",") + ")",
+    })
+}
+```
+
+---
+
+## 2. Design Issues
+
+### 2.1 `controllers/jobs.go` is 3590 lines — needs splitting
+
+The file handles job query, submit, control, restart, array jobs, and verbose output. Each is a distinct concern.
+
+**Suggested split:**
+```
+controllers/
+  jobs_query.go    — GetJobsInfo, GetJobByIDs (~600 lines)
+  jobs_submit.go   — CreateJob, RestartJobByID (~500 lines)
+  jobs_control.go  — ControlJob, ControlJobByID (~400 lines)
+  jobs_array.go    — array job handling (~300 lines)
+  jobs_helpers.go  — shared: parseArrayName, processJobData, etc.
+```
+
+### 2.2 RBAC permission check duplicated across controllers
+
+`GetRequireResourcePermission` + `accessQueues` logic is copy-pasted in `GetJobsInfo`, `ControlJob`, `ControlJobByID`, `RestartJobByID`. Extract to a helper:
+
+```go
+// controllers/rbac_helpers.go
+func getJobAccessQueues(username string) ([]string, error) { ... }
+func checkJobViewPermission(username, queueName string) error { ... }
+```
+
+### 2.3 `xcecli/cmd/jobs/submit.go` is 734 lines
+
+`JobSubmitter` mixes CLI flag parsing, file upload, interactive job handling, and HTTP submission. Split:
+
+```
+cmd/jobs/
+  submit.go          — cobra command + flag binding only
+  submitter.go       — JobSubmitter struct + BuildJobRequest
+  submit_files.go    — file upload logic (already in files.go ✓)
+  submit_interactive.go — interactive/TTY/xjob handling
+```
+
+---
+
+## 3. Code Quality
+
+### 3.1 `ioutil.ReadFile` deprecated (Go 1.16)
+
+**File:** `controllers/rbac_check.go:54`  
+**Fix:** `os.ReadFile(path)` — one-line change, tracked as Q3.
+
+### 3.2 Inconsistent error return convention
+
+Some functions return `(error, T)`, others return `(T, error)`. Go convention is always `(T, error)`.
+
+```go
+// Non-standard (apiserver/controllers/rbac_check.go)
+func GetRequireResourcePermission(...) (error, models.RolePermission)
+
+// Should be
+func GetRequireResourcePermission(...) (models.RolePermission, error)
+```
+
+This affects ~8 call sites in `jobs.go` that do `err, perm := GetRequire...`.
+
+### 3.3 Magic strings for domain types
+
+`"all"`, `"self"`, `"self_ug"`, `"queue"` appear as raw strings in multiple files. They're defined as constants in `models/roles.go` but not always used.
+
+**Fix:** Replace all raw string comparisons with the constants (`models.DomainTypeAll`, etc.).
+
+### 3.4 `xcecli` output printers have no shared time formatting
+
+`page_print_jobs.go` now has `parseJobTime`/`formatJobTime`. The same pattern will be needed for `start_time`, `end_time` in other printers. Move to `common/output/time.go`.
+
+---
+
+## 4. Architecture
+
+### 4.1 In-memory store has no persistence
+
+After restart, all submitted jobs (non-builtin) are lost. The `lib/pq` PostgreSQL driver is already in `go.mod`. When backend integration happens, the notification handlers (`OnJobCreated`, etc.) should write through to PostgreSQL, not just the in-memory store.
+
+### 4.2 `UserRoleSeeder` only implemented for `config_file`
+
+After the startup fix, PAM/LDAP/OAuth2 providers don't implement `AllUsers()` — so after restart with those providers, users still need to re-login to get their roles. Consider storing user-role assignments in a persistent store (DB or file) so they survive restarts regardless of provider.
+
+### 4.3 Rate limiter `limiters` map grows unboundedly
+
+**File:** `middleware/rate_limit.go`  
+The per-endpoint `limiters` map is never evicted. Under normal operation with fixed routes this is fine, but worth noting for future dynamic routing.
+
+---
+
+## 5. Priority Order
+
+```mermaid
+quadrantChart
+    title Impact vs Effort
+    x-axis Low Effort --> High Effort
+    y-axis Low Impact --> High Impact
+    quadrant-1 Do First
+    quadrant-2 Plan
+    quadrant-3 Later
+    quadrant-4 Quick Wins
+    queue_name double-add: [0.10, 0.90]
+    ioutil fix: [0.05, 0.20]
+    error convention: [0.25, 0.60]
+    split jobs.go: [0.60, 0.70]
+    split submit.go: [0.50, 0.50]
+    RBAC helper: [0.35, 0.65]
+    time.go shared: [0.15, 0.35]
+    persistence: [0.80, 0.85]
+    UserRoleSeeder all providers: [0.30, 0.55]
+```
+
+**Do immediately:**
+1. Fix `queue_name` double-add bug in `GetJobsInfo` (10 min, high impact)
+2. `ioutil` → `os.ReadFile` (1 min)
+
+**Next sprint:**
+3. Fix `(error, T)` → `(T, error)` return convention
+4. Extract RBAC helpers to reduce duplication in `jobs.go`
+5. Split `jobs.go` into focused files
+
+**Backend integration phase:**
+6. PostgreSQL persistence via notification handlers
+7. `UserRoleSeeder` for PAM/LDAP providers