chore: add new plan docs, update submodule refs

2026-05-27 02:44:12 +07:00
parent 3ca36940fc
commit 9c30fbd506
6 changed files with 2780 additions and 2 deletions
@@ -0,0 +1,399 @@
+# Fix Device Advanced Settings Form & AllowedIPs /32
+
+## TL;DR
+
+> **Quick Summary**: Fix two issues: (1) DeviceDetail advanced settings form stays empty because TypeScript `Device` interface uses PascalCase but Go API returns snake_case — add `mapDevice()` helper; (2) Config download AllowedIPs still shows `/24` for some paths — verify all 4 config generation points already use `/32`.
+>
+> **Deliverables**:
+> - `apps/dashboard-ui/src/api/devices.ts` — add `mapDevice()` mapping all 9 snake_case fields
+> - `apps/dashboard-ui/src/views/DeviceDetail.vue` — fix nullish coalescing defaults
+> - Config `/32` verified on all 4 endpoints
+>
+> **Estimated Effort**: Small
+> **Parallel Execution**: YES — 2 parallel waves
+> **Critical Path**: Task 1 → Task 3 → (verification)
+
+---
+
+## Context
+
+### Original Request
+User reported: (1) Peer advanced settings (AllowedIPs, DNS override inputs) in DeviceDetail.vue are empty and can't be modified; (2) Downloaded config shows `AllowedIPs = 10.172.21.2/24` instead of `/32`.
+
+### Interview Summary
+**Key Discussions**:
+- Already fixed AllowedIPs `/32` in `api/peers.go` (2 places), `api/share.go`, `peer_sync.go` — need to verify all cover the downloadable config endpoint
+- `Device` interface uses PascalCase (`EndpointAllowedIPs`, `DNS`) but Go JSON serializer outputs snake_case (`endpoint_allowed_ips`, `dns`) for fields with explicit `json` tags
+- Same pattern as `mapServer()` fix in `apps/dashboard-ui/src/api/servers.ts`
+- Need to fix nullish coalescing: `||` should be `??` for fields that can be `0`
+
+### Metis Analysis
+- **9 fields need mapping**: `EndpointAllowedIPs`, `DNS`, `MTU`, `PersistentKeepalive`, `Notes`, `IsSuspended`, `WgServerID`, `RxBytes`, `TxBytes`
+- **Functions requiring mapping**: `fetchDevices()` (used by Devices.vue table), `getDevice()` (used by DeviceDetail.vue + LinkedDevices.vue)
+- **Functions NOT requiring mapping**: `createDevice()` (dead code — never imported), `suspendDevice()`/`unsuspendDevice()` (return `{message, is_suspended}` not a Device)
+- **All 4 config `/32` points verified**: `peers.go:134`, `peers.go:193`, `share.go:66`, `peer_sync.go:40` — all already use `/32`
+- **`provisioning.go` has no AllowedIPs config text** — generates encrypted JSON payload, not a WireGuard config file
+- **Edge case**: `advForm.mtu = device.value.MTU || 1420` — if MTU=0 it incorrectly defaults to 1420. Use `??` instead
+
+---
+
+## Work Objectives
+
+### Core Objective
+Fix Device advanced settings form to correctly populate and save, and verify AllowedIPs `/32` in downloadable config.
+
+### Concrete Deliverables
+- `apps/dashboard-ui/src/api/devices.ts` — `mapDevice()` helper + applied in `fetchDevices()` and `getDevice()`
+- `apps/dashboard-ui/src/views/DeviceDetail.vue` — `??` operator for MTU/PersistentKeepalive
+
+### Definition of Done
+- [ ] `curl /api/v1/devices` returns device with all fields resolved by `mapDevice()`
+- [ ] DeviceDetail advanced settings form populates all fields from API response
+- [ ] Editing + saving advanced settings → reload shows persisted values
+- [ ] `curl /api/v1/devices/:id/config` shows `AllowedIPs = 10.x.x.x/32`
+- [ ] `npm run build` passes
+
+### Must Have
+- Device Advanced Settings form populates correctly
+- Config download URL uses /32 for non-internet devices
+- API client functions properly map snake_case → PascalCase
+
+### Must NOT Have (Guardrails)
+- Do NOT change Go serialization tags — frontend-only fix
+- Do NOT refactor Device interface to use snake_case keys
+- Do NOT touch `api/peers.ts` — its functions don't use Device interface
+- Do NOT modify `provisioning.go` — not a config generation point
+- Do NOT delete `createDevice()` — out of scope (dead code, but not part of this fix)
+
+---
+
+## Verification Strategy
+
+> **ZERO HUMAN INTERVENTION** — ALL verification is agent-executed. No exceptions.
+
+### Test Decision
+- **Infrastructure exists**: NO (no frontend test suite)
+- **Automated tests**: None (no test infrastructure)
+- **Primary verification**: `npm run build` + curl assertions
+
+### QA Policy
+Every task MUST include agent-executed QA scenarios.
+- **Backend verification**: `curl` with admin JWT token, pipe to `jq`, assert field names + values
+- **Frontend build**: `npm run build` — must exit 0 with no errors
+- **Evidence**: `.sisyphus/evidence/task-{N}-{scenario-slug}.txt`
+
+---
+
+## Execution Strategy
+
+### Parallel Execution Waves
+
+```
+Wave 1 (Start immediately — can run in parallel):
+├── Task 1: Add mapDevice() helper to api/devices.ts [quick]
+├── Task 2: Fix nullish coalescing in DeviceDetail.vue [quick]
+
+Wave 2 (After Wave 1 — verification):
+├── Task 3: Verify AllowedIPs /32 on all 4 config endpoints [quick]
+
+Wave FINAL:
+├── Task F1: Plan compliance audit (oracle)
+├── Task F2: Code quality + build check (unspecified-high)
+├── Task F3: Real manual QA — execute all QA scenarios (unspecified-high)
+├── Task F4: Scope fidelity check (deep)
+→ Present results → Get explicit user okay
+```
+
+### Dependency Matrix
+- **1**: — — 3
+- **2**: — — 3
+- **3**: 1, 2 — F1-F4
+- **F1-F4**: 3 — (user okay)
+
+### Agent Dispatch Summary
+- **Wave 1**: 2 tasks
+- **Wave 2**: 1 task
+- **FINAL**: 4 tasks
+
+---
+
+## Final Verification Wave
+
+- [ ] F1. **Plan Compliance Audit** — `oracle`
+  Read the plan end-to-end. For each "Must Have": verify implementation exists (read file, run curl). For each "Must NOT Have": search codebase for forbidden patterns — reject with file:line if found. Check evidence files exist in .sisyphus/evidence/. Compare deliverables against plan.
+  Output: `Must Have [N/N] | Must NOT Have [N/N] | Tasks [N/N] | VERDICT: APPROVE/REJECT`
+
+- [ ] F2. **Code Quality Review** — `unspecified-high`
+  Run `npm run build` and `go build ./...`. Review changed files for: `as any`/`@ts-ignore`, unused imports, console.log in prod. Check AI slop: excessive comments, over-abstraction, generic names.
+  Output: `Build [PASS/FAIL] | Lint [N clean/N issues] | VERDICT`
+
+- [ ] F3. **Real Manual QA** — `unspecified-high`
+  Start from clean state.
+  - Verify `curl /api/v1/devices` returns all 9 PascalCase mapped fields
+  - Verify `curl /api/v1/devices/:id/config` has AllowedIPs with /32
+  - Verify DeviceDetail advanced settings form populates correctly after fix
+  Save evidence to `.sisyphus/evidence/final-qa/`.
+  Output: `Scenarios [N/N pass] | Integration [N/N] | VERDICT`
+
+- [ ] F4. **Scope Fidelity Check** — `deep`
+  For each task: read "What to do", read actual diff (git log/diff). Verify 1:1 — everything in spec was built (no missing), nothing beyond spec was built (no creep). Check "Must NOT do" compliance.
+  Output: `Tasks [N/N compliant] | Contamination [CLEAN/N issues] | VERDICT`
+
+---
+
+## Commit Strategy
+
+- **1+2**: `fix(ui): add mapDevice() and fix nullish coalescing in DeviceDetail` — `apps/dashboard-ui/src/api/devices.ts`, `apps/dashboard-ui/src/views/DeviceDetail.vue`
+- **3**: NO commit — read-only verification
+
+---
+
+## Success Criteria
+
+### Verification Commands
+```bash
+# Verify mapped fields
+curl -s http://localhost:8080/api/v1/devices -H "Authorization: Bearer $TOKEN" | jq '.[0] | {EndpointAllowedIPs, DNS, MTU, PersistentKeepalive, Notes, IsSuspended, WgServerID, RxBytes, TxBytes}'
+# Expected: all 9 fields present (not null), even if empty
+
+# Verify /32 in config
+curl -s http://localhost:8080/api/v1/devices/{ID}/config -H "Authorization: Bearer $TOKEN" | jq -r '.config_text'
+# Expected: AllowedIPs = 10.x.x.x/32 (not /24)
+
+# Verify build
+cd apps/dashboard-ui && npm run build
+# Expected: exit 0, no errors
+```
+
+### Final Checklist
+- [x] All "Must Have" present
+- [x] All "Must NOT Have" absent
+- [x] All builds pass
+
+- [x] 1. Add `mapDevice()` helper to `api/devices.ts`
+
+  **What to do**:
+  - Add `mapDevice()` function between the import and interface declaration
+  - Map all 9 snake_case fields: `endpoint_allowed_ips→EndpointAllowedIPs`, `dns→DNS`, `mtu→MTU`, `persistent_keepalive→PersistentKeepalive`, `notes→Notes`, `is_suspended→IsSuspended`, `wg_server_id→WgServerID`, `rx_bytes→RxBytes`, `tx_bytes→TxBytes`
+  - Apply `mapDevice()` in `fetchDevices()`: `.map(mapDevice)` on array response
+  - Apply `mapDevice()` in `getDevice()`: wrap single object return
+  - Verify with `npm run build`
+
+  **Must NOT do**:
+  - Do NOT modify `createDevice()` — dead code, out of scope
+  - Do NOT modify `suspendDevice()`/`unsuspendDevice()` — return `{message, is_suspended}`
+  - Do NOT modify the `Device` interface (keep PascalCase)
+  - Do NOT touch any Go files
+
+  **Recommended Agent Profile**:
+  - **Category**: `quick`
+    - Reason: Single file change, straightforward mapping pattern, already has precedent in `mapServer()`
+  - **Skills**: none needed
+
+  **Parallelization**:
+  - **Can Run In Parallel**: YES
+  - **Parallel Group**: Wave 1 (with Task 2)
+  - **Blocks**: Task 3
+  - **Blocked By**: None
+
+  **References**:
+
+  **Pattern References** (exact pattern to follow):
+  - `apps/dashboard-ui/src/api/servers.ts:28-43` — `mapServer()` helper — exact same pattern, copy the structure and replace field mappings
+
+  **API/Type References** (contracts):
+  - `apps/dashboard-ui/src/api/devices.ts:3-23` — `Device` interface — these PascalCase fields are the target output shape
+  - `apps/server-core/internal/models/models.go` — Go Device model with `json` tags — these snake_case values are the actual API response keys
+
+  **WHY Each Reference Matters**:
+  - `mapServer()` is the canonical pattern — same architecture, same approach. Follow it exactly.
+  - `Device` interface tells you which PascalCase keys to produce
+  - Go model `json` tags tell you which snake_case keys come from the API
+
+  **Acceptance Criteria**:
+
+  **QA Scenarios (MANDATORY):**
+
+  ```
+  Scenario: Verify mapDevice() resolves all snake_case fields via fetchDevices
+    Tool: Bash (curl)
+    Preconditions: Admin JWT token available, at least one device exists
+    Steps:
+      1. `curl -s http://localhost:8080/api/v1/devices -H "Authorization: Bearer $TOKEN" | jq '.[0] | keys'`
+      2. Verify keys include PascalCase names like "EndpointAllowedIPs", "DNS", "MTU", "PersistentKeepalive", "Notes", "IsSuspended", "WgServerID", "RxBytes", "TxBytes"
+      3. `curl -s http://localhost:8080/api/v1/devices -H "Authorization: Bearer $TOKEN" | jq '.[0] .EndpointAllowedIPs'`
+      4. Verify value is non-null (empty string or actual value)
+    Expected Result: All 9 mapped fields present with correct PascalCase keys
+    Failure Indicators: Any field is missing from the response keys, or any field is `null` instead of its default value
+    Evidence: .sisyphus/evidence/task-1-fetch-fields.txt
+
+  Scenario: Verify getDevice also resolves fields
+    Tool: Bash (curl)
+    Preconditions: Admin JWT token available, a device ID known
+    Steps:
+      1. `curl -s http://localhost:8080/api/v1/devices/{FIRST_DEVICE_ID} -H "Authorization: Bearer $TOKEN" | jq '.EndpointAllowedIPs'`
+      2. Verify value is non-null
+    Expected Result: Mapped fields present on single device response
+    Evidence: .sisyphus/evidence/task-1-get-fields.txt
+
+  Scenario: Build passes
+    Tool: Bash
+    Preconditions: Dependencies installed
+    Steps:
+      1. `cd apps/dashboard-ui && npm run build`
+    Expected Result: Exit code 0, no errors
+    Evidence: .sisyphus/evidence/task-1-build.txt
+  ```
+
+  **Evidence to Capture:**
+  - [ ] `.sisyphus/evidence/task-1-fetch-fields.txt` — curl output showing all 9 PascalCase fields
+  - [ ] `.sisyphus/evidence/task-1-get-fields.txt` — curl output for single device
+  - [ ] `.sisyphus/evidence/task-1-build.txt` — npm build output
+
+  **Commit**: YES (with Task 2)
+  - Message: `fix(ui): add mapDevice() helper for Device snake_case fields`
+  - Files: `apps/dashboard-ui/src/api/devices.ts`
+
+- [x] 2. Fix nullish coalescing in `DeviceDetail.vue`
+
+  **What to do**:
+  - In `DeviceDetail.vue` lines 154-158, change `||` to `??` for MTU and PersistentKeepalive:
+    - `advForm.value.mtu = device.value.MTU ?? 1420`
+    - `advForm.value.persistentKeepalive = device.value.PersistentKeepalive ?? 25`
+  - For endpointAllowedIPs and dns, `|| ''` is fine since those are strings (empty string is falsy but also a valid default)
+  - Verify with `npm run build`
+
+  **Must NOT do**:
+  - Do NOT change the form HTML template
+  - Do NOT change the saveAdvanced function
+  - Do NOT touch endpontAllowedIPs or dns coalescing — string default is fine
+
+  **Recommended Agent Profile**:
+  - **Category**: `quick`
+    - Reason: Trivial two-line change, no new logic
+  - **Skills**: none needed
+
+  **Parallelization**:
+  - **Can Run In Parallel**: YES
+  - **Parallel Group**: Wave 1 (with Task 1)
+  - **Blocks**: Task 3
+  - **Blocked By**: None
+
+  **References**:
+
+  **Code References**:
+  - `apps/dashboard-ui/src/views/DeviceDetail.vue:154-158` — lines to fix
+
+  **WHY**:
+  - `||` treats `0` as falsy, so a device with `MTU: 0` would show `1420` (wrong)
+  - `??` only falls through for `null`/`undefined`, preserving `0` as a valid value
+
+  **Acceptance Criteria**:
+
+  **QA Scenarios (MANDATORY):**
+
+  ```
+  Scenario: Verify build passes
+    Tool: Bash
+    Preconditions: Dependencies installed
+    Steps:
+      1. `cd apps/dashboard-ui && npm run build`
+    Expected Result: Exit code 0
+    Evidence: .sisyphus/evidence/task-2-build.txt
+  ```
+
+  **Evidence to Capture:**
+  - [ ] `.sisyphus/evidence/task-2-build.txt`
+
+  **Commit**: YES (squash with Task 1)
+  - Message: `fix(ui): use nullish coalescing for MTU default`
+  - Files: `apps/dashboard-ui/src/views/DeviceDetail.vue`
+
+- [x] 3. Verify AllowedIPs /32 on all config endpoints
+
+  **What to do**:
+  - Verify the 4 known config generation points already use `/32`:
+    1. `api/peers.go:134` — CreatePeer config generation
+    2. `api/peers.go:193` — getDeviceConfig (used by GetConfig, UpdateConfig, GetQR)
+    3. `api/share.go:66` — ShareConfig
+    4. `internal/wgmanager/peer_sync.go:40` — SyncLocalPeers
+  - Confirming: there is NO 5th config generation point. `api/provisioning.go` generates encrypted JSON payload with no AllowedIPs field
+  - Run full test suite (if tests pass despite FK failures)
+
+  **Must NOT do**:
+  - Do NOT modify any Go files
+  - Do NOT re-add /24 — the fix was already applied in previous sessions
+  - Do NOT search for additional config generation points — Metis already verified all
+
+  **Recommended Agent Profile**:
+  - **Category**: `quick`
+    - Reason: Read-only verification, no changes needed
+  - **Skills**: none needed
+
+  **Parallelization**:
+  - **Can Run In Parallel**: NO (depends on Task 1 and 2 for full integration test)
+  - **Parallel Group**: Wave 2
+  - **Blocks**: F1-F4
+  - **Blocked By**: Task 1, Task 2
+
+  **References**:
+
+  **Code References**:
+  - `apps/server-core/api/peers.go:134` — CreatePeer AllowedIPs: `ipStr + "/32"`
+  - `apps/server-core/api/peers.go:193` — getDeviceConfig AllowedIPs: `*device.InternalIP + "/32"`
+  - `apps/server-core/api/share.go:66` — ShareConfig AllowedIPs: `*device.InternalIP + "/32"`
+  - `apps/server-core/api/peer_sync.go:40` — SyncLocalPeers AllowedIPs: `*d.InternalIP + "/32"`
+  - `apps/server-core/api/provisioning.go` — no AllowedIPs field in ConfigPayload
+
+  **WHY**:
+  - Read the actual code at each line to confirm `/32` is present
+  - Once confirmed, issue 2 is fully closed
+
+  **Acceptance Criteria**:
+
+  **QA Scenarios (MANDATORY):**
+
+  ```
+  Scenario: Verify all 4 config generation points use /32
+    Tool: Bash (grep)
+    Preconditions: Codebase pulled at latest commit
+    Steps:
+      1. `grep -n "InternalIP.*+.*32" apps/server-core/api/peers.go` (expect 2 matches at lines 134 and 193)
+      2. `grep -n "InternalIP.*+.*32" apps/server-core/api/share.go` (expect 1 match at line 66)
+      3. `grep -n "InternalIP.*+.*32" apps/server-core/api/peer_sync.go` (expect 1 match at line 40)
+    Expected Result: 4 total matches across the 3 files
+    Evidence: .sisyphus/evidence/task-3-grep-results.txt
+
+  Scenario: Verify no /24 exists in config generation
+    Tool: Bash (grep)
+    Preconditions: Same
+    Steps:
+      1. `grep -n "AllowedIPs.*24" apps/server-core/api/peers.go apps/server-core/api/share.go`
+    Expected Result: Zero matches
+    Evidence: .sisyphus/evidence/task-3-no-24.txt
+
+  Scenario: Confirm provisioning.go has no AllowedIPs config text
+    Tool: Bash (grep)
+    Steps:
+      1. `grep -c "AllowedIPs" apps/server-core/api/provisioning.go`
+    Expected Result: 0
+    Evidence: .sisyphus/evidence/task-3-provisioning.txt
+
+  Scenario: Build still passes
+    Tool: Bash
+    Steps:
+      1. `cd apps/server-core && go build ./...`
+    Expected Result: Exit 0
+    Evidence: .sisyphus/evidence/task-3-build.txt
+  ```
+
+  **Evidence to Capture:**
+  - [ ] `.sisyphus/evidence/task-3-grep-results.txt`
+  - [ ] `.sisyphus/evidence/task-3-no-24.txt`
+  - [ ] `.sisyphus/evidence/task-3-provisioning.txt`
+  - [ ] `.sisyphus/evidence/task-3-build.txt`
+
+  **Commit**: NO (read-only verification, no code changes)
+  - Message: N/A
+
+---