Skip to content

Commit 0ae4afd

Browse files
technicalpicklestechnicalpickles
committed
docs(spec): add task list for OAuth token refresh implementation (smart-mcp-proxy#23)
Related smart-mcp-proxy#23 Generates 39 implementation tasks organized by user story: - Phase 1: Setup (5 tasks) - types, constants, metrics infrastructure - Phase 2: Foundational (6 tasks) - backoff algorithm, logging fixes - Phase 3: US1 Survive Sleep/Wake (6 tasks) - startup refresh - Phase 4: US2 Proactive Refresh (7 tasks) - exponential backoff - Phase 5: US3 Clear Feedback (9 tasks) - health status integration - Phase 6: Polish (6 tasks) - validation and cleanup Key implementation files: - internal/oauth/refresh_manager.go (primary) - internal/health/calculator.go (health integration) - internal/observability/metrics.go (Prometheus metrics) - internal/upstream/core/connection.go (logging fix)
1 parent cfe9a5c commit 0ae4afd

1 file changed

Lines changed: 208 additions & 0 deletions

File tree

  • specs/023-oauth-state-persistence

‎specs/023-oauth-state-persistence/tasks.md‎

Lines changed: 208 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,208 @@
1+
# Tasks: OAuth Token Refresh Reliability
2+
3+
**Input**: Design documents from `/specs/023-oauth-state-persistence/`
4+
**Prerequisites**: plan.md, spec.md, research.md, data-model.md, contracts/
5+
6+
**Tests**: Not explicitly requested in spec. Tests included for critical functionality only.
7+
8+
**Organization**: Tasks grouped by user story to enable independent implementation and testing.
9+
10+
## Format: `[ID] [P?] [Story] Description`
11+
12+
- **[P]**: Can run in parallel (different files, no dependencies)
13+
- **[Story]**: Which user story this task belongs to (US1, US2, US3)
14+
- Include exact file paths in descriptions
15+
16+
## Path Conventions
17+
18+
- **Go module**: `internal/` for packages, root for `go.mod`
19+
- **Tests**: `*_test.go` alongside implementation files
20+
21+
---
22+
23+
## Phase 1: Setup (Shared Infrastructure)
24+
25+
**Purpose**: Constants, types, and metrics infrastructure needed by all user stories
26+
27+
- [ ] T001 Add RefreshState type and constants in internal/oauth/refresh_manager.go
28+
- [ ] T002 [P] Add RetryBackoffBase (10s) and MaxRetryBackoff (5min) constants in internal/oauth/refresh_manager.go
29+
- [ ] T003 [P] Extend RefreshSchedule struct with RetryBackoff, MaxBackoff, LastAttempt, RefreshState fields in internal/oauth/refresh_manager.go
30+
- [ ] T004 [P] Add OAuth refresh metrics (mcpproxy_oauth_refresh_total counter, mcpproxy_oauth_refresh_duration_seconds histogram) in internal/observability/metrics.go
31+
- [ ] T005 [P] Add RecordOAuthRefresh() and RecordOAuthRefreshDuration() methods to MetricsManager in internal/observability/metrics.go
32+
33+
---
34+
35+
## Phase 2: Foundational (Blocking Prerequisites)
36+
37+
**Purpose**: Core backoff algorithm and rate limiting that ALL user stories depend on
38+
39+
**CRITICAL**: No user story work can begin until this phase is complete
40+
41+
- [ ] T006 Implement calculateBackoff(retryCount int) method with exponential backoff and 5min cap in internal/oauth/refresh_manager.go
42+
- [ ] T007 Add rate limiting check (min 10s between attempts per server) in internal/oauth/refresh_manager.go
43+
- [ ] T008 [P] Fix misleading LogTokenRefreshSuccess() function - rename to LogClientConnectionSuccess() in internal/oauth/logging.go
44+
- [ ] T009 [P] Fix misleading success log at line ~1239 in internal/upstream/core/connection.go (HTTP connection)
45+
- [ ] T010 [P] Fix misleading success log at line ~1696 in internal/upstream/core/connection.go (SSE connection)
46+
- [ ] T011 Add accurate LogTokenRefreshAttempt() and LogTokenRefreshResult() functions in internal/oauth/logging.go
47+
48+
**Checkpoint**: Backoff algorithm and accurate logging ready - user story implementation can begin
49+
50+
---
51+
52+
## Phase 3: User Story 1 - Survive Laptop Sleep/Wake (Priority: P1)
53+
54+
**Goal**: OAuth servers automatically reconnect after laptop wakes from sleep or mcpproxy restarts, using stored refresh tokens
55+
56+
**Independent Test**: Authenticate OAuth server, restart mcpproxy, verify server reconnects within 60s without browser auth
57+
58+
### Implementation for User Story 1
59+
60+
- [ ] T012 [US1] Modify RefreshManager.Start() to detect expired tokens at startup in internal/oauth/refresh_manager.go
61+
- [ ] T013 [US1] Add executeImmediateRefresh() method for expired tokens with valid refresh tokens in internal/oauth/refresh_manager.go
62+
- [ ] T014 [US1] Emit metrics on refresh attempt (success/failure with result label) in internal/oauth/refresh_manager.go
63+
- [ ] T015 [US1] Add structured logging for startup refresh attempts with server name, token age, result in internal/oauth/refresh_manager.go
64+
- [ ] T016 [US1] Update RefreshSchedule state to RefreshStateRetrying on failure in internal/oauth/refresh_manager.go
65+
- [ ] T017 [US1] Add unit test for startup refresh with expired access token in internal/oauth/refresh_manager_test.go
66+
67+
**Checkpoint**: OAuth servers survive restart - verify with manual test per quickstart.md
68+
69+
---
70+
71+
## Phase 4: User Story 2 - Proactive Token Refresh (Priority: P2)
72+
73+
**Goal**: Tokens are refreshed automatically at 80% lifetime before expiration, with exponential backoff retry on failure
74+
75+
**Independent Test**: Authenticate OAuth server, monitor token expiration, verify new token obtained before original expires
76+
77+
### Implementation for User Story 2
78+
79+
- [ ] T018 [US2] Modify executeRefresh() to use new exponential backoff on failure in internal/oauth/refresh_manager.go
80+
- [ ] T019 [US2] Implement rescheduleWithBackoff() using calculateBackoff() in internal/oauth/refresh_manager.go
81+
- [ ] T020 [US2] Continue retries until token expiration (not limited retry count) in internal/oauth/refresh_manager.go
82+
- [ ] T021 [US2] Update RefreshSchedule.RefreshState transitions (Scheduled -> Retrying -> Failed) in internal/oauth/refresh_manager.go
83+
- [ ] T022 [US2] Emit refresh duration metric on each attempt in internal/oauth/refresh_manager.go
84+
- [ ] T023 [US2] Add unit test for exponential backoff sequence (10s, 20s, 40s, 80s, 160s, 300s cap) in internal/oauth/refresh_manager_test.go
85+
- [ ] T024 [US2] Add unit test for unlimited retries until token expiration in internal/oauth/refresh_manager_test.go
86+
87+
**Checkpoint**: Proactive refresh works with backoff - verify token refresh before expiration
88+
89+
---
90+
91+
## Phase 5: User Story 3 - Clear Refresh Failure Feedback (Priority: P3)
92+
93+
**Goal**: Users see specific failure reasons in health status (network error vs expired refresh token vs provider error)
94+
95+
**Independent Test**: Simulate refresh failures, verify distinct error messages in `mcpproxy upstream list` and web UI
96+
97+
### Implementation for User Story 3
98+
99+
- [ ] T025 [US3] Add RefreshState, RefreshRetryCount, RefreshLastError, RefreshNextAttempt fields to HealthCalculatorInput in internal/health/calculator.go
100+
- [ ] T026 [US3] Implement health calculation for RefreshStateRetrying -> degraded level in internal/health/calculator.go
101+
- [ ] T027 [US3] Implement health calculation for RefreshStateFailed -> unhealthy level in internal/health/calculator.go
102+
- [ ] T028 [US3] Set appropriate health detail messages per refresh state in internal/health/calculator.go
103+
- [ ] T029 [US3] Set appropriate health action per state (view_logs for retrying, login for failed) in internal/health/calculator.go
104+
- [ ] T030 [US3] Add error classification for invalid_grant (permanent) vs network errors (retryable) in internal/oauth/refresh_manager.go
105+
- [ ] T031 [US3] Expose RefreshSchedule state via RefreshManager.GetRefreshState(serverName) method in internal/oauth/refresh_manager.go
106+
- [ ] T032 [US3] Wire RefreshManager state into health calculation flow in internal/health/calculator.go
107+
- [ ] T033 [US3] Add unit test for health status output per refresh state in internal/health/calculator_test.go
108+
109+
**Checkpoint**: Health status shows specific refresh failure reasons
110+
111+
---
112+
113+
## Phase 6: Polish & Cross-Cutting Concerns
114+
115+
**Purpose**: Final validation, cleanup, and documentation
116+
117+
- [ ] T034 [P] Verify all logging follows naming convention (OAuth token refresh *) in internal/oauth/logging.go
118+
- [ ] T035 [P] Run existing tests to ensure no regressions: go test ./internal/...
119+
- [ ] T036 [P] Run E2E tests: ./scripts/test-api-e2e.sh
120+
- [ ] T037 Verify metrics endpoint shows new OAuth metrics: curl http://localhost:8080/metrics | grep oauth_refresh
121+
- [ ] T038 Manual verification per quickstart.md success criteria checklist
122+
- [ ] T039 Update CLAUDE.md if any architecture patterns changed
123+
124+
---
125+
126+
## Dependencies & Execution Order
127+
128+
### Phase Dependencies
129+
130+
- **Setup (Phase 1)**: No dependencies - can start immediately
131+
- **Foundational (Phase 2)**: Depends on Setup completion - BLOCKS all user stories
132+
- **User Stories (Phase 3-5)**: All depend on Foundational phase completion
133+
- User stories can proceed in priority order (P1 -> P2 -> P3)
134+
- US2 depends on backoff from Foundational; US3 depends on state tracking from US1/US2
135+
- **Polish (Phase 6)**: Depends on all user stories being complete
136+
137+
### User Story Dependencies
138+
139+
- **User Story 1 (P1)**: Can start after Foundational (Phase 2) - startup refresh
140+
- **User Story 2 (P2)**: Builds on US1 - adds exponential backoff to existing refresh
141+
- **User Story 3 (P3)**: Builds on US1/US2 - surfaces refresh state in health status
142+
143+
### Within Each User Story
144+
145+
- Implementation tasks in order (no parallel within story due to file dependencies)
146+
- Unit tests after implementation of the feature they test
147+
- Story complete before moving to next priority
148+
149+
### Parallel Opportunities
150+
151+
- Setup tasks T002, T003, T004, T005 can run in parallel (different files/functions)
152+
- Foundational tasks T008, T009, T010 can run in parallel (different files)
153+
- Polish tasks T034, T035, T036 can run in parallel
154+
155+
---
156+
157+
## Parallel Example: Setup Phase
158+
159+
```bash
160+
# Launch all setup tasks in parallel (different files):
161+
Task: "Add RetryBackoffBase and MaxRetryBackoff constants" [T002]
162+
Task: "Extend RefreshSchedule struct" [T003]
163+
Task: "Add OAuth refresh metrics" [T004]
164+
Task: "Add MetricsManager methods" [T005]
165+
```
166+
167+
---
168+
169+
## Implementation Strategy
170+
171+
### MVP First (User Story 1 Only)
172+
173+
1. Complete Phase 1: Setup (T001-T005)
174+
2. Complete Phase 2: Foundational (T006-T011)
175+
3. Complete Phase 3: User Story 1 (T012-T017)
176+
4. **STOP and VALIDATE**: Restart mcpproxy with OAuth server, verify auto-reconnect
177+
5. This alone delivers SC-001: "OAuth servers automatically reconnect within 60s"
178+
179+
### Incremental Delivery
180+
181+
1. Complete Setup + Foundational -> Backoff and metrics ready
182+
2. Add User Story 1 -> Startup recovery works -> Deploy/Demo (MVP!)
183+
3. Add User Story 2 -> Proactive refresh with backoff -> Deploy/Demo
184+
4. Add User Story 3 -> Clear failure feedback -> Deploy/Demo
185+
5. Each story adds value without breaking previous stories
186+
187+
### Key Files Modified
188+
189+
| File | Tasks | User Stories |
190+
|------|-------|--------------|
191+
| `internal/oauth/refresh_manager.go` | T001-T003, T006-T007, T012-T016, T018-T022, T030-T031 | Setup, US1, US2, US3 |
192+
| `internal/oauth/logging.go` | T008, T011, T034 | Foundational, Polish |
193+
| `internal/upstream/core/connection.go` | T009, T010 | Foundational |
194+
| `internal/observability/metrics.go` | T004, T005 | Setup |
195+
| `internal/health/calculator.go` | T025-T029, T032 | US3 |
196+
| `internal/oauth/refresh_manager_test.go` | T017, T023, T024 | US1, US2 |
197+
| `internal/health/calculator_test.go` | T033 | US3 |
198+
199+
---
200+
201+
## Notes
202+
203+
- [P] tasks = different files, no dependencies
204+
- [Story] label maps task to specific user story for traceability
205+
- Each user story delivers incremental value
206+
- Verify with quickstart.md manual testing steps after each story
207+
- Commit after each task or logical group
208+
- Stop at any checkpoint to validate story independently

0 commit comments

Comments
 (0)