jcfs
195be5e575
- Add comment handlers for WebSocket API (comment.add, comment.list) - Update e2e-test.sh to focus on comment system testing - Test multi-turn conversation flow (3 turns) - Display comment content and metadata in test output - Reduce test timeouts for faster execution - Fix various orchestrator and lifecycle manager issues - Add CODE_REVIEW_ISSUES.md documenting known issues
2.3 KiB
2.3 KiB
Backend Code Review Issues
Generated: 2026-01-11
High Priority
1. Security TODOs
internal/gateway/websocket/handler.go:20- TODO: In production, validate origininternal/gateway/websocket/handler.go:46- TODO: Implement proper JWT validation
2. Deprecated API
internal/agent/docker/client.go:300-301-inspect.NetworkSettings.IPAddressis deprecated (removed in Docker v29). UseNetworkSettings.Networksinstead.
Medium Priority
3. Unused Functions
internal/agent/lifecycle/manager.go:641-updateInstanceCompleteis unusedinternal/agent/streaming/reader.go:232-marshalMessageis unusedinternal/task/repository/sqlite.go:617-scanTasksis unused
4. Unused Field / Race Condition
internal/gateway/websocket/client.go:36-mu sync.RWMutexfield is unused. Either use it to protectsubscriptionsor remove it.
5. Outdated Comment
internal/agent/agentctl/acp.go:108- Comment referencesneeds_inputwhich was removed. Update to:// The StopReason indicates why the agent stopped (e.g., "end_turn")
Low Priority
6. Unused Test Helpers
internal/agent/lifecycle/manager_test.go:130-testManagertype unusedinternal/agent/lifecycle/manager_test.go:135-newTestManagerfunction unused
7. Code Style (staticcheck S1000)
internal/agentctl/api/acp.go:215- Usefor range chinstead offor { select {} }internal/agentctl/api/acp.go:336- Usefor range chinstead offor { select {} }
8. Hardcoded Sleep
internal/agent/lifecycle/manager.go:253-time.Sleep(500 * time.Millisecond)- Consider proper ready-check mechanism
9. Ignored Error
internal/task/repository/sqlite.go:626-_ = json.Unmarshal(...)ignores error
Questions / Review Needed
10. InputRequestHandler Redundancy
Files: internal/orchestrator/service.go (lines 44-45, 71, 387-389, 507-512), cmd/kandev/main.go (lines 305-363)
The InputRequestHandler and handleInputRequired functionality still exists using input_required message type. Is this still needed given the new comment-based conversation flow?
11. Other TODOs
internal/gateway/websocket/hub.go:127- TODO: Add topic-based routing for task-specific notifications