Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
Walkthroughcht-app-github 개선 문서 세트가 추가되어 현재 아키텍처, webhook 인그레스의 문제점, Redis 기반 ingress/큐/락/aggregate 상태 설계, 이벤트 배칭 전략 및 Phase 1–3 로드맵과 단계별 구현·운영 계획을 문서화합니다. Changes
Sequence Diagram(s)sequenceDiagram
participant GitHub
participant Ingress as "/hook/v1 (ingress)"
participant Redis
participant Worker
participant LockStore as "lock:agg:{key}"
participant ChannelTalk
participant GitHubAPI as "GitHub API"
GitHub->>Ingress: POST webhook (headers + raw payload)
Ingress->>Ingress: verify signature, compute deliveryID
Ingress->>Redis: SETNX dedup:delivery:{deliveryID}
alt dedup reserved
Ingress->>Redis: XADD stream:github-events <envelope>
Ingress-->>GitHub: respond 200/202 (ack)
else duplicate
Ingress-->>GitHub: respond 200 (duplicate)
end
loop worker loop
Worker->>Redis: XREAD/XCLAIM stream:github-events
Worker->>Worker: compute canonical aggregateKey
Worker->>LockStore: SET NX lock:agg:{aggregateKey}
LockStore-->>Worker: lock acquired
Worker->>Redis: HGET state:agg:{aggregateKey}
alt root not present
Worker->>ChannelTalk: create root message
ChannelTalk-->>Worker: root_id
Worker->>GitHubAPI: post anchor comment
GitHubAPI-->>Worker: comment_id
Worker->>Redis: HSET state:agg:{aggregateKey} (root_id, comment_id, status)
else root exists
Worker->>ChannelTalk: write/update message under root
end
Worker->>Redis: HSET state:agg:{aggregateKey} (progress), XACK stream
Worker->>LockStore: DEL lock:agg:{aggregateKey}
end
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@docs/improve-github-app/11-phase-1-implementation-plan.md`:
- Line 3: Replace the actor-specific prefix "**For Claude:** REQUIRED SUB-SKILL:
Use superpowers:executing-plans to implement this plan task-by-task." with a
neutral, team-oriented instruction such as "REQUIRED SUB-SKILL: Use
superpowers:executing-plans to implement this plan task-by-task" or "Team note:
use superpowers:executing-plans to implement this plan task-by-task," removing
any mention of a specific executor; update the sentence in the document heading
that currently starts with "For Claude:" so the phrasing is repository-wide and
inclusive.
In `@docs/improve-github-app/12-phase-2-implementation-plan.md`:
- Line 3: Remove the agent-specific directive "For Claude: REQUIRED SUB-SKILL:
Use superpowers:executing-plans" and replace it with a neutral,
repository-agnostic instruction such as "Execute the plan task-by-task" or
"Perform tasks sequentially"; update the sentence containing "Use
superpowers:executing-plans" so it does not reference a specific agent or
special skill set and instead conveys the intended behavior in generic terms.
In `@docs/improve-github-app/13-phase-3-implementation-plan.md`:
- Line 13: The heading "Task 0: worker processing seam 정리" is using ### which
skips a level after H1; change that heading to use ## (i.e., prefix with two
hashes) or insert an appropriate intermediate H2 section above so the document's
heading levels increase by one step to satisfy MD001 and remove the markdownlint
warning for the "Task 0: worker processing seam 정리" heading.
- Line 3: 문서 본문에 남아있는 에이전트 전용 실행 지시문("For Claude: REQUIRED SUB-SKILL: Use
superpowers:executing-plans to implement this plan task-by-task")는 협업 문서에 적절하지
않으니 본문에서 삭제하고 해당 문구를 별도의 에이전트 전용 실행 지침 섹션으로 이전하거나 저장소 공통 실행 가이드(예:
CONTRIBUTING/AGENT_GUIDELINES)에 일반화하여 옮기고, 문서 본문에는 공개적으로 남겨도 무방한 대체 문구(예: "에이전트가
자동 실행을 필요로 하는 작업이 있습니다" 등)만 남기도록 수정하세요.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: c731a24c-0157-46ee-bb3c-54b7105fd065
📒 Files selected for processing (8)
docs/improve-github-app/01-current-architecture.mddocs/improve-github-app/02-ordering-and-concurrency.mddocs/improve-github-app/03-event-batching.mddocs/improve-github-app/10-phase-roadmap.mddocs/improve-github-app/11-phase-1-implementation-plan.mddocs/improve-github-app/12-phase-2-implementation-plan.mddocs/improve-github-app/13-phase-3-implementation-plan.mddocs/improve-github-app/README.md
quino0627
changed the title
quino0627
force-pushed
the
codex/docs-improve-github-app
branch
from
cfc41d3 to
3cfddbc
Compare
leesea14
left a comment
leesea14
left a comment
There was a problem hiding this comment.
전체적으로 설계가 잘 정리되어 있고 Goals/Non-goals 명시가 특히 좋았습니다.
1. ValKey distributed lock vs Kafka partition-by-subject
현재 Kafka 도입 전환 중인 상황을 감안하면, subject key로 partition을 나누는 방식도 worker 경쟁 문제를 구조적으로 해결할 수 있는 선택지인 것 같습니다. out-of-order 문제는 어느 쪽이든 state machine이 필요하긴 하지만, lock TTL 관련 엣지케이스를 구조적으로 없앨 수 있다는 점에서 고려해보셨는지 궁금합니다. ValKey lock 방식을 선택한 이유가 있으면 문서에 남겨두면 좋지 않을까요?!
2. Lock TTL 만료 시 first root wins forever invariant
lock:subject:{subjectKey} TTL이 30s인데, downstream write가 느릴 경우 lock이 만료된 뒤 다른 worker가 같은 subject를 처리할 수 있습니다. 이때 두 worker가 각각 root가 없다고 판단해 subject:{subjectKey}에 rootMessageId를 write하려 하면, first root wins forever invariant를 ValKey 레벨에서 어떻게 보장하는지 궁금합니다. SET NX 또는 optimistic locking 같은 보호가 필요하지 않을까요?!
3. GitHub App 서비스 규모 대비 설계 복잡도
실제 관찰된 문제를 해결하는 설계라는 건 이해하는데, GitHub App의 서비스 특성상 subject당 동시 webhook 빈도가 얼마나 되는지 궁금합니다. ValKey stream + distributed lock + state machine + Phase 1/2 순차 rollout은 상당한 운영 복잡도를 수반하는데, 현재 관찰된 문제의 규모가 이 복잡도를 감수할 만한 수준인지, 혹은 더 단순한 접근(예: 단일 worker + dedup만)으로 먼저 검증하고 점진적으로 복잡도를 추가하는 방향은 고려해보셨는지 여쭤봐도 될까요?!
2 participants
Summary
docs/improve-github-appwith current architecture, concurrency, and batching design docsTesting
git diff --check -- docs/improve-github-appSummary by CodeRabbit