KYRBOT/OmniOilPersonal
1
0
Fork 0
Code Issues Pull Requests 1 Actions 1 Packages Projects Releases Wiki Activity
Files
6880ead1f059abf1766d38d59bdaee8c1b03fe3b
OmniOilPersonal/openspec/changes/archive/2026-05-07-console-tech-debt-cleanup/archive-report.md
henryor af5535dfcc docs(openspec): archiva el change console-tech-debt-cleanup 
Persiste proposal, specs delta (auth, api-client, access-requests,
platform-shell, testing), design con 8 ADRs, tasks (27 items en 10 fases),
verify-report (28 tests, 0 críticos, 0 warnings tras el cierre) y
archive-report final.

Cierra el ciclo SDD del cleanup de deuda técnica del console.
2026-05-07 21:14:56 -05:00

163 lines
8.5 KiB
Markdown
Raw Blame History

This file contains ambiguous Unicode characters

This file contains Unicode characters that might be confused with other characters. If you think that this is intentional, you can safely ignore this warning. Use the Escape button to reveal them.

# Archive Report: console-tech-debt-cleanup
**Change**: `console-tech-debt-cleanup`
**Project**: `services/frontend-console`
**Archived**: 2026-05-07
**Status**: DONE
---
## Executive Summary
The `console-tech-debt-cleanup` change is COMPLETE and CLOSED. All 28 tests pass green (23/23 PR2 + baseline). All phases (0–9) implemented across two PRs with full strict-TDD validation. Three warnings from PR1 closure resolved. Implementation spans auth correctness (single token field, aligned types), API client robustness (no null-as-T casts, typed 204 path, 401 retry + dedupe), access-requests error UX, platform-shell error boundary, and Vitest testing scaffolding. Zero outstanding tasks; only project-rule skip (pnpm build) noted. Change is ready for deployment.
---
## Artifacts & Traceability
| Phase | Engram Topic Key | Observation ID | Content |
|-------|------------------|----------------|---------|
| Proposal | `sdd/console-tech-debt-cleanup/proposal` | #152 | P0/P1/P2 bundling, correctness debt drivers, risk analysis, success criteria |
| Spec | `sdd/console-tech-debt-cleanup/spec` | #155 | Delta specs: 5 capabilities (auth, api-client, access-requests, platform-shell, testing) |
| Design | `sdd/console-tech-debt-cleanup/design` | #156 | 8 ADRs, no new layers, refactor over existing structure, TDD ordering |
| Tasks | `sdd/console-tech-debt-cleanup/tasks` | #157 | 10 phases, 28 total work items; PR1 phases 0–5, PR2 phases 6–9, phase 10 validation |
| Apply-Progress | `sdd/console-tech-debt-cleanup/apply-progress` | #159 | PR1 + PR2 completion matrix; 28 tests pass; warning closure (W1/W2/W3) |
| Verify-Report | `sdd/console-tech-debt-cleanup/verify-report` | #160 | PR1 PASS + PR2 PASS; 3 WARNINGs (all addressed in warning closure); 0 CRITICAL |
---
## Implementation Summary
### Completed Phases
| Phase | Description | Status | Tests | Notes |
|-------|-------------|--------|-------|-------|
| 0 | Pre-flight audits (R1/A2/A4) | DONE | N/A | Grep audit confirmed safe refactor surface |
| 1 | Vitest scaffolding (config + setup) | DONE | 0 | Config-only; W1 warning: --passWithNoTests removed in closure |
| 2 | Auth store: drop token field | DONE | 2 | GREEN: setSession writes accessToken; logout clears |
| 3 | Auth API: LoginResponse shape | DONE | 3 | GREEN: access_token required, token required, refresh_token removed |
| 4 | API client: split + dedupe + retry | DONE | 5 | GREEN: apiClient/apiClientVoid, SessionCoordinator dedupe, 401 retry |
| 5 | ProtectedRoute: cold-visit heuristic | DONE | 2 | GREEN: hasHydrated guard + user-based refresh (no first-load flash) |
| 6 | AccessRequestsPage: error UI | DONE | 3 | GREEN: isError → error card + Reintentar (W3 closure: refetch verified) |
| 7 | AccessRequestsPage: mutations | DONE | 6 | GREEN: approve/reject/review success + error; toast + invalidation (W2 closure: reject/review tested) |
| 8 | AccessRequestsPage: isActionable consolidation | DONE | 4 | GREEN: no inline includes; isActionable enforced |
| 9 | PlatformLayout: ErrorBoundary | DONE | 3 | GREEN: class component, Outlet wrapped, logout fallback, sidebar safe |
| 10 | Final validation | DONE | Phase 10.1/10.3 only | 28/28 tests PASS, lint clean, tsc --noEmit clean; 10.2 build skipped |
### Files Modified
**PR1 (Phases 0–5)**: 11 files
- `package.json` (test script + 5 devDeps)
- `vite.config.ts` (test block)
- `src/test/setup.ts` (new)
- `src/features/auth/stores/auth-store.ts` (drop token)
- `src/features/auth/stores/auth-store.test.ts` (new)
- `src/features/auth/api/auth-api.ts` (LoginResponse shape)
- `src/features/auth/api/auth-api.test.ts` (new)
- `src/features/auth/pages/LoginPage.tsx` (use accessToken)
- `src/app/router/ProtectedRoute.tsx` (hasHydrated + user heuristic)
- `src/app/router/ProtectedRoute.test.tsx` (new)
- `src/lib/api/client.ts` (executeWithRefresh, apiClientVoid, no null-as-T)
- `src/lib/api/client.test.ts` (new)
**PR2 (Phases 6–9)**: 5 files
- `src/features/access-requests/pages/AccessRequestsPage.tsx` (error UI + isActionable)
- `src/features/access-requests/pages/AccessRequestsPage.test.tsx` (13 tests)
- `src/app/layouts/ErrorBoundary.tsx` (new)
- `src/app/layouts/ErrorBoundary.test.tsx` (new)
- `src/app/layouts/PlatformLayout.tsx` (wrap Outlet)
**Total**: 16 files changed (11 PR1 + 5 PR2)
### Test Coverage
- **Total tests**: 28 (passing)
- **Test files**: 6
- `src/features/auth/api/auth-api.test.ts` (3 tests)
- `src/features/auth/stores/auth-store.test.ts` (2 tests)
- `src/lib/api/client.test.ts` (5 tests)
- `src/app/router/ProtectedRoute.test.tsx` (2 tests)
- `src/features/access-requests/pages/AccessRequestsPage.test.tsx` (13 tests)
- `src/app/layouts/ErrorBoundary.test.tsx` (3 tests)
- **Lint**: exits 0 (clean)
- **TypeScript**: tsc --noEmit exits 0 (clean)
- **Build**: skipped per project rule "Never build after changes"
### Quality Gates
| Gate | Status | Evidence |
|------|--------|----------|
| Test coverage (P0 surface) | PASS | 28/28 tests pass, strict TDD ordering enforced |
| No regressions | PASS | All 12 PR1 tests still pass in PR2 verify |
| Spec compliance | PASS | 5 capabilities (auth, api-client, access-requests, platform-shell, testing) compliant per verify-report matrix |
| Lint clean | PASS | pnpm lint exits 0 |
| Type safety | PASS | tsc --noEmit exits 0, zero null-as-T casts |
| TDD ordering | PASS | Tests RED first, then GREEN after refactor (strict TDD mode) |
---
## Warnings & Closure
### PR1 Warning Closure
| Warning | Raised | Closed | Resolution |
|---------|--------|--------|------------|
| W1 | Phase 1 | Phase 10 (apply final) | Removed `--passWithNoTests` from package.json test script; 28 tests exist, flag unnecessary |
| W2 | Phase 7 | Phase 10 (apply final) | Added 4 tests (7.2a reject success, 7.2b reject error, 7.3a review success, 7.3b review error) to AccessRequestsPage.test.tsx; all pass |
| W3 | Phase 6 | Phase 10 (apply final) | Added test 6.1c: Reintentar click calls `listAccessRequests()` again (refetch verified) |
All warnings closed. No outstanding issues.
---
## Deferred Items (Spec Suggestions)
The verify-report noted 6 suggestions; these remain deferred and tracked separately:
- **S1**: Add `pnpm run typecheck` script to package.json (structural — not blocking; PR can land without it)
- **S2**: ProtectedRoute warm-visit setTimeout proxy (optimization — not required for correctness)
- **S3**: Remove unused `refreshToken` state field from auth-store (cleanup; token field already removed)
- **S4**: Install `@testing-library/jest-dom` for RTL `.toBeDisabled()` matcher (testing polish; current `.disabled` works)
- **S5**: Use named expect matchers instead of `expect.anything()` for TQ v5 context arg (testing polish)
- **S6**: Revisar button consolidation — currently checks `status !== 'pending'` directly, not `isActionable()` (deliberate; edge-case behavior difference)
None of these block closure. They are suitable for future tech-debt or polish PRs.
---
## Risks & Mitigations
| Risk | Mitigation | Status |
|------|------------|--------|
| Silent token-rename regression on sign-in | P0 test scaffolding (phases 0–5) landed first, validates behavior before refactor | MITIGATED |
| Bracket-string `'token'` reads not caught by TypeScript | Pre-flight R1 audit confirmed zero bracket-string reads | MITIGATED |
| LoginResponse type does not match backend runtime | Spec requirement aligned with backend `auth.rs` contract (access_token + token emitted) | MITIGATED |
| 204 endpoints break on refactor | A2 pre-flight confirmed zero 204 callsites; TypeScript will catch any new uses | MITIGATED |
| RTL flakiness on AccessRequestsPage mutations | waitFor + findBy* patterns used; modal helpers robust | MITIGATED |
All risks from design document addressed and closed.
---
## Next Steps
None. Change is complete, all tasks done, all tests pass, all warnings closed. Ready for production deployment.
The change can be merged to main and deployed. No follow-up work items remain.
---
## Archive Metadata
- **Created**: 2026-05-07
- **Change ID**: `console-tech-debt-cleanup`
- **Git commits**: 2 PRs (PR1 phases 0–5, PR2 phases 6–9) + warning closure batch
- **Total lines changed**: ~1200 (implementation + tests)
- **Test files added**: 6
- **Breaking changes**: None
- **New dependencies**: vitest@^3.2.4, jsdom@^26.1.0, @testing-library/* (3 packages)
- **Database migrations**: None
- **API changes**: None (internal refactor only)
- **Config changes**: vite.config.ts (test block), package.json (test script + devDeps)
- **Documentation updated**: None (inline spec in code + ADR trail in design document)
Reference in New Issue View Git Blame Copy Permalink