Skip to content

Commit 7edcce8

Browse files
chore: track PR smart-mcp-proxy#255 review suggestions as beans
Created follow-up tasks from code review: - mcpproxy-go-4ydu: Extract shared RefreshState type - mcpproxy-go-75ss: Extract containsIgnoreCase to utility - mcpproxy-go-3st0: Replace custom toLower with strings.ToLower - mcpproxy-go-xguu: Document 24-hour token expiry threshold Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
1 parent 0a244b9 commit 7edcce8

6 files changed

Lines changed: 98 additions & 0 deletions

.beans.yml

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
beans:
2+
path: .beans
3+
prefix: mcpproxy-go-
4+
id_length: 4
5+
default_status: todo
6+
default_type: task
Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,15 @@
1+
---
2+
# mcpproxy-go-3st0
3+
title: Replace custom toLower with strings.ToLower
4+
status: todo
5+
type: task
6+
priority: low
7+
created_at: 2026-01-19T14:25:30Z
8+
updated_at: 2026-01-19T14:25:30Z
9+
---
10+
11+
From PR #255 code review.
12+
13+
File: `internal/oauth/refresh_manager.go:792-802`
14+
15+
Uses a manual ASCII-only lowercase implementation. Consider using `strings.ToLower()` from stdlib unless there's a specific performance reason for the custom implementation.
Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,21 @@
1+
---
2+
# mcpproxy-go-4ydu
3+
title: Extract shared RefreshState type to common package
4+
status: todo
5+
type: task
6+
priority: low
7+
created_at: 2026-01-19T14:25:29Z
8+
updated_at: 2026-01-19T14:25:29Z
9+
---
10+
11+
From PR #255 code review.
12+
13+
The `RefreshState` type is duplicated in:
14+
- `internal/health/calculator.go:9-22`
15+
- `internal/oauth/refresh_manager.go:37-70`
16+
17+
The health package mirrors the oauth package's `RefreshState` "for decoupling". Options:
18+
1. Create a shared types package
19+
2. Add a test to ensure the duplicates stay in sync
20+
21+
Consider which approach better serves the architecture.
Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,17 @@
1+
---
2+
# mcpproxy-go-75ss
3+
title: Extract containsIgnoreCase to shared utility package
4+
status: todo
5+
type: task
6+
priority: low
7+
created_at: 2026-01-19T14:25:29Z
8+
updated_at: 2026-01-19T14:25:29Z
9+
---
10+
11+
From PR #255 code review.
12+
13+
The `containsIgnoreCase` function is duplicated in:
14+
- `internal/health/calculator.go:305`
15+
- `internal/oauth/refresh_manager.go:780`
16+
17+
Both packages have identical implementations. Should be extracted to a shared utility package (e.g., `internal/util/strings.go`).
Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,18 @@
1+
---
2+
# mcpproxy-go-sskj
3+
title: Add --all flag to auth login command
4+
status: in-progress
5+
type: feature
6+
created_at: 2026-01-19T13:53:58Z
7+
updated_at: 2026-01-19T13:53:58Z
8+
---
9+
10+
Add support for re-authenticating all servers at once with mcpproxy auth login --all. Include confirmation prompt (yes/no) before opening multiple browser tabs, with optional --force flag to bypass confirmation.
11+
12+
## Checklist
13+
- [ ] Explore existing auth command implementation
14+
- [ ] Add --all flag to auth login command
15+
- [ ] Implement confirmation prompt for multiple servers
16+
- [ ] Add --force flag to bypass confirmation
17+
- [ ] Test with multiple servers requiring auth
18+
- [ ] Update any relevant documentation
Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,21 @@
1+
---
2+
# mcpproxy-go-xguu
3+
title: Document 24-hour token expiry threshold constant
4+
status: todo
5+
type: task
6+
priority: low
7+
created_at: 2026-01-19T14:25:30Z
8+
updated_at: 2026-01-19T14:25:30Z
9+
---
10+
11+
From PR #255 code review.
12+
13+
File: `internal/oauth/refresh_manager.go:665`
14+
15+
```go
16+
if timeSinceExpiry > 24*time.Hour {
17+
```
18+
19+
The 24-hour threshold for giving up on token refresh is undocumented. Should:
20+
1. Extract to a named constant (e.g., `maxTokenExpiryRecoveryWindow`)
21+
2. Add documentation explaining why 24 hours was chosen

0 commit comments

Comments
 (0)