feat: issue-based auto workflow orchestration (#529)

[dalsoop]

Summary

• POST /api/issue-workflow endpoint for full orchestration: wake member with issue branch → assign task → track execution → extract PR URL → notify dalroot via bridge/webhook/callback
• dalcli-leader issue-workflow <issue-id> <task> command for leader containers
• dalcli-leader wake --issue <N> flag for issue-branch creation
• Client SDK methods: StartIssueWorkflow(), GetIssueWorkflow(), ListIssueWorkflows()
• Duplicate workflow detection (409 Conflict for same issue)
• 25+ unit/integration tests covering store, handlers, auth, bridge notification, full HTTP end-to-end flow

Full Flow

1. dalroot: gh issue create → dalcenter tell <team> --issue N
2. leader: dalcli-leader issue-workflow N "구현해"
3. daemon: POST /api/issue-workflow → wake dev --issue N → assign → track
4. member: executes task → commits → PR
5. daemon: extracts PR URL → bridge notification → webhook → dalroot callback

Test plan

• Store CRUD: Create, Get, List, FindByIssue
• Duplicate detection and eviction
• State transitions: fail(), complete()
• extractPRUrl with various formats
• Handler validation: bad JSON, missing fields
• Duplicate active workflow returns 409
• Auth: write requires token, read does not
• Full flow: wake failure → graceful error + bridge notification
• HTTP end-to-end: start → poll status → list → retry after failure

🤖 Generated with Claude Code

[dalsoop]

코드 리뷰: issue-based auto workflow orchestration (#529)

전체적인 구조와 흐름이 깔끔하게 설계되어 있습니다. 테스트 커버리지도 우수합니다.
아래에 개선이 필요한 사항들을 정리했습니다.

---

1. [중요] Race Condition — IssueWorkflow 필드 보호 없음

issue_workflow.go 전반:

runIssueWorkflow는 goroutine에서 실행되면서 wf.Status, wf.Phase, wf.TaskID 등의 필드를 lock 없이 직접 수정합니다 (라인 203, 217, 243, 248 등). 동시에 handleIssueWorkflowStatus (라인 182-189)는 Get()으로 같은 *IssueWorkflow 포인터를 반환하여 동일 객체를 읽습니다.

Get()은 RLock으로 map 접근만 보호하지, 반환된 포인터의 필드 읽기/쓰기는 보호하지 않습니다.

이는 Go race detector에서 잡히는 data race입니다. 해결 방안:

• IssueWorkflow 자체에 sync.RWMutex를 추가하거나
• Get()에서 deep copy를 반환하거나
• 모든 필드 수정을 store의 lock 안에서 수행

2. [중요] FindByIssue와 Create 사이 TOCTOU 경합

issue_workflow.go:158-168:

if existing := d.issueWorkflows.FindByIssue(req.IssueID); existing != nil {
    // 409 반환
}
wf := d.issueWorkflows.Create(req.IssueID, req.Member, req.Task)

FindByIssue()와 Create() 호출 사이에 다른 요청이 같은 issue ID로 workflow를 생성할 수 있습니다. 중복 방지 로직이 원자적이지 않습니다. Create 내부에서 중복 검사를 함께 수행하거나, 외부에서 하나의 lock으로 묶어야 합니다.

3. [보통] json.Unmarshal 에러 무시

client.go:590, client.go:606:

json.Unmarshal(b, &result)
return &result, nil

json.Unmarshal 에러를 무시하고 있습니다. 서버가 예상치 못한 응답을 반환하면 빈 struct를 정상 결과로 반환하게 됩니다. 에러를 체크해야 합니다.

4. [보통] ListIssueWorkflows — 상태 코드 미확인

client.go:611-620:

func (c *Client) ListIssueWorkflows() ([]IssueWorkflowStatus, error) {
    resp, err := c.http.Get(c.baseURL + "/api/issue-workflows")
    ...
    json.NewDecoder(resp.Body).Decode(&results)
    return results, nil
}

resp.StatusCode >= 400 체크가 없습니다. StartIssueWorkflow와 GetIssueWorkflow에는 있는데 ListIssueWorkflows에만 빠져 있습니다.

5. [보통] captureResponseWriter.code 기본값 0

issue_workflow.go:281:

if rec.code >= 400 {

WriteHeader가 호출되지 않은 경우 code는 0입니다. 성공(200)을 가정하지만, handler가 WriteHeader를 호출하지 않고 Write만 호출하면 code가 0인 상태에서 성공으로 처리됩니다. 이 자체는 현재 handleWake 구현에서는 문제가 안 되겠지만, captureResponseWriter를 범용적으로 쓰려면 기본값을 200으로 초기화하는 것이 http.ResponseWriter 계약과 일치합니다.

6. [경미] extractPRUrl — URL 검증 부족

issue_workflow.go:355-367:

https:// prefix 검증 없이 github.com과 /pull/만 체크합니다. 악의적 출력에서 http://evil.com/github.com/foo/pull/1 같은 문자열도 매칭될 수 있습니다. 실제 위험은 낮지만, strings.HasPrefix(word, "https://github.com/") 검증을 추가하면 더 안전합니다.

7. [경미] wakeForIssue — http.NewRequest 에러 무시

issue_workflow.go:272:

wakeReq, _ := http.NewRequest("POST", path, nil)

이 케이스에서 실제로 실패할 가능성은 극히 낮지만, 에러를 무시하는 것은 Go 관례에 어긋납니다.

8. [경미] daemon 필드 정렬 불일치

daemon.go:170-175:

costs          *costStore
issueWorkflows *issueWorkflowStore
registry       *Registry
startTime      time.Time

기존 필드들은 탭 하나로 정렬되어 있었는데, 새로 추가된 필드들은 공백 여러 개로 정렬되어 있습니다. 기존 스타일과 맞추면 좋겠습니다.

---

요약

• 항목 1, 2는 동시성 관련 버그로, 프로덕션 배포 전에 수정이 필요합니다
• 항목 3, 4는 에러 핸들링 누락으로 디버깅을 어렵게 만들 수 있습니다
• 나머지는 코드 품질 개선 사항입니다

전반적으로 잘 구현되어 있고, 테스트가 충실합니다. 위 동시성 이슈만 해결되면 머지 준비 완료라고 봅니다.

[dalsoop]

[Code Review] REQUEST CHANGES

1. extractPRUrl() 중복: PR #544 notify.go에도 동일 함수 정의 → 컴파일 에러. #544 머지 후 제거 필요.
2. daemon.New() 충돌: #548의 6-param New()와 충돌. #548 머지 후 rebase 필요.
3. issueWorkflowStore 비영속: 메모리 전용 → 데몬 재시작 시 workflow 상태 유실. JSON persistence 권장.
4. wakeForIssue() 내부 HTTP 호출: captureResponseWriter 패턴은 작동하나 URL path 설정 불안정 가능성.

테스트 커버리지 우수 (664줄), duplicate detection, webhook 연동 등은 좋음.