docs: update status and completion review
This commit is contained in:
298
docs/code-review/PROJECT_REAL_COMPLETION_REVIEW_2026-04-09.md
Normal file
298
docs/code-review/PROJECT_REAL_COMPLETION_REVIEW_2026-04-09.md
Normal file
@@ -0,0 +1,298 @@
|
||||
# Project Real Completion Review 2026-04-09
|
||||
|
||||
## Scope
|
||||
|
||||
- Review date: 2026-04-09
|
||||
- Workspace: `D:\usersystem`
|
||||
- Branch context: `main` ahead of `origin/main` by 6 commits, with additional local uncommitted changes present during review
|
||||
- Review method: local code inspection plus command execution
|
||||
- Environment note: the current shell exports an invalid `GOROOT` value (`D:\Program Files\Go\go`). Repo-level Go verification in this review was re-run with `GOROOT=D:\Program Files\Go` and repo-local `GOCACHE` / `GOMODCACHE`.
|
||||
|
||||
## Executive Summary
|
||||
|
||||
The repository still contains substantial real implementation, but it still cannot be honestly declared release-closed.
|
||||
|
||||
Compared with the earlier 2026-04-09 draft review, several previously reported blockers are no longer current:
|
||||
|
||||
- `go vet ./...` is now green after environment normalization
|
||||
- `go build ./cmd/server` is now green after environment normalization
|
||||
- `npm.cmd run build` is green again
|
||||
- `govulncheck` is green on the current `go1.26.2` toolchain
|
||||
|
||||
However, the following real blockers remain:
|
||||
|
||||
- admin role resolution is still stubbed end-to-end
|
||||
- avatar upload is still stubbed end-to-end
|
||||
- the supported browser E2E entrypoint is still broken in the current workspace
|
||||
- the full backend test matrix is still red because of the `LL_001` login-log pagination SLA gate
|
||||
- frontend lint is still red, and the current test suite emits native-dialog jsdom noise
|
||||
- status documentation is materially out of sync with the current verified state
|
||||
|
||||
## Commands Executed
|
||||
|
||||
### Raw workspace commands
|
||||
|
||||
```powershell
|
||||
go build ./cmd/server
|
||||
go vet ./...
|
||||
cd frontend/admin
|
||||
npm.cmd run lint
|
||||
npm.cmd run build
|
||||
npm.cmd run test:run
|
||||
npm.cmd run test:coverage
|
||||
npm.cmd run e2e:full:win
|
||||
npm.cmd audit --omit=dev --json --registry=https://registry.npmjs.org/
|
||||
```
|
||||
|
||||
### Environment-normalized Go commands
|
||||
|
||||
```powershell
|
||||
$env:GOROOT='D:\Program Files\Go'
|
||||
$env:GOCACHE='D:\usersystem\.gocache'
|
||||
$env:GOMODCACHE='D:\usersystem\.gomodcache'
|
||||
|
||||
go build ./cmd/server
|
||||
go vet ./...
|
||||
go test ./... -short -count=1
|
||||
go test ./... -count=1
|
||||
go run golang.org/x/vuln/cmd/govulncheck@latest ./...
|
||||
```
|
||||
|
||||
### Targeted frontend verification
|
||||
|
||||
```powershell
|
||||
cd frontend/admin
|
||||
npm.cmd run test:run -- src/components/common/ui-consistency.test.tsx
|
||||
```
|
||||
|
||||
## Verification Results
|
||||
|
||||
### Raw workspace blockers
|
||||
|
||||
- `go build ./cmd/server`
|
||||
- failed before compilation because `GOROOT` points to the non-existent path `D:\Program Files\Go\go`
|
||||
- `go vet ./...`
|
||||
- failed for the same workspace environment reason
|
||||
- `npm.cmd run e2e:full:win`
|
||||
- failed for the same workspace environment reason because the wrapper script inherits the broken `GOROOT`
|
||||
|
||||
### Passed
|
||||
|
||||
- normalized `go build ./cmd/server`
|
||||
- normalized `go vet ./...`
|
||||
- normalized `go test ./... -short -count=1`
|
||||
- `npm.cmd run build`
|
||||
- `npm.cmd run test:run -- src/components/common/ui-consistency.test.tsx`
|
||||
- `30` tests passed in `1` file
|
||||
- the run still emitted jsdom `Not implemented: window.alert` noise after the success summary
|
||||
- normalized `govulncheck`
|
||||
- output: `No vulnerabilities found.`
|
||||
- `npm.cmd audit --omit=dev --json --registry=https://registry.npmjs.org/`
|
||||
- production vulnerability counts: `0 / 0 / 0 / 0 / 0`
|
||||
|
||||
### Failed
|
||||
|
||||
- normalized `go test ./... -count=1`
|
||||
- failed in `internal/service.TestScale_LL_001_180DayLoginLogRetention`
|
||||
- observed `P99=2.0027538s`
|
||||
- threshold `2s`
|
||||
- `npm.cmd run lint`
|
||||
- failed in `frontend/admin/src/components/common/ui-consistency.test.tsx:539`
|
||||
- ESLint `react-hooks/immutability`: reassigned `timeout` after render
|
||||
- normalized `npm.cmd run e2e:full:win`
|
||||
- still failed after fixing `GOROOT`
|
||||
- `frontend/admin/scripts/run-playwright-auth-e2e.ps1` currently builds the server with `go build -o ... .\cmd\server\main.go`
|
||||
- that file-based build path does not resolve module dependencies correctly in the current setup, so the wrapper exits with `server build failed`
|
||||
|
||||
### Not fully re-verified in this round
|
||||
|
||||
- `npm.cmd run test:run`
|
||||
- did not complete within the 240s audit timeout
|
||||
- visible output included jsdom `window.alert` noise from `src/components/common/ui-consistency.test.tsx`
|
||||
- `npm.cmd run test:coverage`
|
||||
- did not complete within the 300s audit timeout
|
||||
- visible output included the same jsdom `window.alert` noise
|
||||
|
||||
## Current Findings
|
||||
|
||||
### 1. Admin role chain is still not implemented end-to-end
|
||||
|
||||
Backend:
|
||||
|
||||
- `internal/api/handler/user_handler.go`
|
||||
- `GetUserRoles` still returns an empty `roles` array
|
||||
- `AssignRoles` still returns `"role assignment not implemented"`
|
||||
|
||||
Frontend:
|
||||
|
||||
- `frontend/admin/src/app/providers/AuthProvider.tsx`
|
||||
- still fetches `/users/:id/roles` to determine session roles
|
||||
- `frontend/admin/src/components/guards/RequireAdmin.tsx`
|
||||
- still gates admin access from `isAdmin`
|
||||
- `frontend/admin/src/pages/admin/UsersPage/AssignRolesModal.tsx`
|
||||
- still exposes the role assignment flow in the UI
|
||||
|
||||
Impact:
|
||||
|
||||
- admin capability determination is still not trustworthy
|
||||
- role assignment remains a false product closure
|
||||
|
||||
### 2. Avatar upload is still a visible but unimplemented flow
|
||||
|
||||
Backend:
|
||||
|
||||
- `internal/api/handler/user_handler.go`
|
||||
- `UploadAvatar` still returns `"avatar upload not implemented"`
|
||||
- `internal/api/handler/avatar_handler.go`
|
||||
- `UploadAvatar` still returns `"avatar upload not implemented"`
|
||||
|
||||
Frontend:
|
||||
|
||||
- `frontend/admin/src/services/profile.ts`
|
||||
- still posts avatar data to `/users/:id/avatar`
|
||||
- `frontend/admin/src/pages/admin/ProfileSecurityPage/ProfileSecurityPage.tsx`
|
||||
- still exposes the upload action in the user-facing profile flow
|
||||
|
||||
Impact:
|
||||
|
||||
- a visible account-management path is still not closed on the backend
|
||||
|
||||
### 3. The supported browser E2E path is still broken
|
||||
|
||||
Observed in two layers:
|
||||
|
||||
- current workspace shell:
|
||||
- inherited broken `GOROOT` causes immediate failure
|
||||
- after correcting `GOROOT`:
|
||||
- `frontend/admin/scripts/run-playwright-auth-e2e.ps1` still fails at line `168`
|
||||
- it builds with `go build -o $serverExePath .\cmd\server\main.go` instead of building the package `./cmd/server`
|
||||
- this causes module resolution failures and aborts before the browser suite starts
|
||||
|
||||
Impact:
|
||||
|
||||
- the repo cannot currently claim that the documented browser acceptance path works from the current workspace
|
||||
|
||||
### 4. The backend full matrix is still not green
|
||||
|
||||
- short-path backend verification is strong:
|
||||
- normalized `go test ./... -short -count=1` passed
|
||||
- release-style full backend verification is still negative:
|
||||
- normalized `go test ./... -count=1` failed on the committed `LL_001` SLA gate
|
||||
|
||||
Interpretation:
|
||||
|
||||
- broad functional coverage exists
|
||||
- release-readiness remains blocked by a real, measured performance threshold
|
||||
|
||||
### 5. Frontend validation is improved, but still not clean
|
||||
|
||||
- `npm.cmd run build` is green again
|
||||
- `npm.cmd run lint` is still red
|
||||
- `frontend/admin/src/components/common/ui-consistency.test.tsx`
|
||||
- directly calls native dialogs such as `alert(...)`
|
||||
- still contains the `timeout` reassignment pattern that violates the current lint rule
|
||||
- the targeted `ui-consistency` test file passes, but still emits jsdom native-dialog noise
|
||||
|
||||
Interpretation:
|
||||
|
||||
- the prior build blocker is fixed
|
||||
- the frontend quality gate is still not clean enough for a release-closed claim
|
||||
|
||||
### 6. Status documentation is materially stale
|
||||
|
||||
Examples now verified against current runs:
|
||||
|
||||
- `docs/status/REAL_PROJECT_STATUS.md`
|
||||
- its latest section claims a green backend verification summary, but full `go test ./... -count=1` is still red
|
||||
- it still describes a `govulncheck` blocker tied to `go1.26.1`, but current normalized `govulncheck` on `go1.26.2` is clean
|
||||
- it still describes browser-level E2E closure, but the currently documented entrypoint still fails in this workspace
|
||||
|
||||
Impact:
|
||||
|
||||
- the current status narrative overstates release readiness
|
||||
|
||||
## Historical Findings Rechecked
|
||||
|
||||
The following older findings should not be repeated as current blockers:
|
||||
|
||||
- `frontend/admin/src/pages/admin/WebhooksPage/WebhooksPage.tsx`
|
||||
- now fetches paginated data via `listWebhooks({ page, page_size })`
|
||||
- `frontend/admin/src/pages/admin/ProfileSecurityPage/ProfileSecurityPage.tsx`
|
||||
- now renders `ContactBindingsSection`
|
||||
- `internal/api/handler/webhook_handler_test.go`
|
||||
- the old `go vet` blocker is no longer present
|
||||
- frontend production build
|
||||
- the prior Vite build failure is no longer reproducible in this round
|
||||
- Go stdlib vulnerability blocker
|
||||
- the prior `govulncheck` finding tied to `go1.26.1` is no longer present on the current local `go1.26.2` run
|
||||
|
||||
## Additional Real Gaps Still Present
|
||||
|
||||
Stub-like or incomplete API behavior still visible in current code:
|
||||
|
||||
- `internal/api/handler/user_handler.go`
|
||||
- `GetUserRoles`
|
||||
- `AssignRoles`
|
||||
- `UploadAvatar`
|
||||
- `CreateAdmin`
|
||||
- `DeleteAdmin`
|
||||
- `internal/api/handler/avatar_handler.go`
|
||||
- `UploadAvatar`
|
||||
|
||||
Also still present:
|
||||
|
||||
- toolchain inconsistency
|
||||
- `go.mod`: `go 1.25.0`
|
||||
- local normalized runtime: `go1.26.2`
|
||||
- `Dockerfile`: `golang:1.23-alpine`
|
||||
|
||||
## Real Completion Assessment
|
||||
|
||||
### Can be honestly claimed
|
||||
|
||||
- the repository contains substantial backend and frontend implementation
|
||||
- normalized `go vet ./...` is green
|
||||
- normalized `go build ./cmd/server` is green
|
||||
- normalized `go test ./... -short -count=1` is green
|
||||
- frontend production `build` is green
|
||||
- production npm dependency audit is clean in the current run
|
||||
- current local `govulncheck` run is clean
|
||||
|
||||
### Cannot be honestly claimed
|
||||
|
||||
- "the current workspace passes the full minimum release verification matrix"
|
||||
- "browser-level E2E is currently closed from the documented entrypoint"
|
||||
- "admin permission flow is fully closed"
|
||||
- "avatar upload is fully closed"
|
||||
- "status documentation already reflects current reality"
|
||||
|
||||
## Recommendations
|
||||
|
||||
### Immediate
|
||||
|
||||
- implement or explicitly disable the stubbed role, avatar, and admin-management APIs
|
||||
- fix `frontend/admin/scripts/run-playwright-auth-e2e.ps1` to build the package `./cmd/server` rather than the file path `.\cmd\server\main.go`
|
||||
- fix the workspace Go environment so raw `go` commands and the E2E wrapper stop inheriting an invalid `GOROOT`
|
||||
- clean up `frontend/admin/src/components/common/ui-consistency.test.tsx`
|
||||
- remove direct native-dialog calls from the test flow
|
||||
- replace the render-lifetime `timeout` reassignment pattern
|
||||
- update status documentation only from the fresh evidence above
|
||||
|
||||
### Near term
|
||||
|
||||
- decide whether the `LL_001` SLA threshold should be optimized, isolated, or moved out of the default full test gate
|
||||
- align Go versions across `go.mod`, local development expectations, and Docker build images
|
||||
- re-run the full frontend unit and coverage suites with a longer audit window once the `ui-consistency` issues are cleaned up
|
||||
|
||||
## Final Conclusion
|
||||
|
||||
Real completion is higher than many old "unfinished project" narratives suggest, but still lower than the current status document implies.
|
||||
|
||||
The accurate current description is:
|
||||
|
||||
- real implementation exists across backend and frontend
|
||||
- several previously reported blockers were genuinely fixed
|
||||
- but important stub endpoints still exist
|
||||
- the documented E2E entrypoint is still broken
|
||||
- the full backend gate is still red
|
||||
- and the public status narrative still needs correction
|
||||
Reference in New Issue
Block a user