fix: address Codex round 2 mediums
Some checks failed
CI / test (push) Failing after 3s

- harvest: message says 'ready-for-review' not 'pushed'
- sync: timestamp advanced after pulls, not before
- sync: accepts main/master/reported branch, not just main
- inbox: checks CORE_BRAIN_KEY env before falling back to file
- inbox: parses 'from' not 'from_agent', 'messages' not 'data'
- queue: strips variant suffix for rate limit lookup (claude:opus → claude)
- review_queue: respects ReviewQueueInput.Reviewer instead of hardcoding coderabbit
- tests: updated to match real API response structure

Co-Authored-By: Virgil <virgil@lethean.io>
This commit is contained in:
Snider 2026-03-21 16:05:59 +00:00
parent 98ce071b13
commit 026b31edf7
6 changed files with 68 additions and 38 deletions

View file

@ -74,7 +74,12 @@ func (s *PrepSubsystem) loadAgentsConfig() *AgentsConfig {
// for a given agent type, based on rate config and time of day.
func (s *PrepSubsystem) delayForAgent(agent string) time.Duration {
cfg := s.loadAgentsConfig()
rate, ok := cfg.Rates[agent]
// Strip variant suffix (claude:opus → claude) for config lookup
base := agent
if idx := strings.Index(agent, ":"); idx >= 0 {
base = agent[:idx]
}
rate, ok := cfg.Rates[base]
if !ok || rate.SustainedDelay == 0 {
return 0
}

View file

@ -94,7 +94,11 @@ func (s *PrepSubsystem) reviewQueue(ctx context.Context, _ *mcp.CallToolRequest,
}
repoDir := filepath.Join(basePath, repo)
result := s.reviewRepo(ctx, repoDir, repo, input.DryRun, input.LocalOnly)
reviewer := input.Reviewer
if reviewer == "" {
reviewer = "coderabbit"
}
result := s.reviewRepo(ctx, repoDir, repo, reviewer, input.DryRun, input.LocalOnly)
// Parse rate limit from result
if result.Verdict == "rate_limited" {
@ -150,7 +154,7 @@ func (s *PrepSubsystem) findReviewCandidates(basePath string) []string {
}
// reviewRepo runs CodeRabbit on a single repo and takes action.
func (s *PrepSubsystem) reviewRepo(ctx context.Context, repoDir, repo string, dryRun, localOnly bool) ReviewResult {
func (s *PrepSubsystem) reviewRepo(ctx context.Context, repoDir, repo, reviewer string, dryRun, localOnly bool) ReviewResult {
result := ReviewResult{Repo: repo}
// Check saved rate limit
@ -160,8 +164,10 @@ func (s *PrepSubsystem) reviewRepo(ctx context.Context, repoDir, repo string, dr
return result
}
// Run reviewer CLI locally
reviewer := "coderabbit" // default, can be overridden by caller
// Run reviewer CLI locally — use the reviewer passed from reviewQueue
if reviewer == "" {
reviewer = "coderabbit"
}
cmd := s.buildReviewCommand(ctx, repoDir, reviewer)
out, err := cmd.CombinedOutput()
output := string(out)

View file

@ -65,7 +65,7 @@ func (m *Subsystem) harvestCompleted() string {
})
}
} else {
parts = append(parts, fmt.Sprintf("%s: pushed %s (%d files)", h.repo, h.branch, h.files))
parts = append(parts, fmt.Sprintf("%s: ready-for-review %s (%d files)", h.repo, h.branch, h.files))
if m.notifier != nil {
m.notifier.ChannelSend(context.Background(), "harvest.complete", map[string]any{
"repo": h.repo,

View file

@ -263,11 +263,15 @@ func (m *Subsystem) checkCompletions() string {
// checkInbox checks for unread messages.
func (m *Subsystem) checkInbox() string {
home, _ := os.UserHomeDir()
keyFile := filepath.Join(home, ".claude", "brain.key")
apiKeyStr, err := coreio.Local.Read(keyFile)
if err != nil {
return ""
apiKeyStr := os.Getenv("CORE_BRAIN_KEY")
if apiKeyStr == "" {
home, _ := os.UserHomeDir()
keyFile := filepath.Join(home, ".claude", "brain.key")
data, err := coreio.Local.Read(keyFile)
if err != nil {
return ""
}
apiKeyStr = data
}
// Call the API to check inbox
@ -293,11 +297,11 @@ func (m *Subsystem) checkInbox() string {
}
var resp struct {
Data []struct {
Messages []struct {
Read bool `json:"read"`
From string `json:"from_agent"`
From string `json:"from"`
Subject string `json:"subject"`
} `json:"data"`
} `json:"messages"`
}
if json.NewDecoder(httpResp.Body).Decode(&resp) != nil {
return ""
@ -306,7 +310,7 @@ func (m *Subsystem) checkInbox() string {
unread := 0
senders := make(map[string]int)
latestSubject := ""
for _, msg := range resp.Data {
for _, msg := range resp.Messages {
if !msg.Read {
unread++
if msg.From != "" {

View file

@ -228,10 +228,10 @@ func TestCheckInbox_Good_UnreadMessages(t *testing.T) {
assert.NotEmpty(t, r.URL.Query().Get("agent"))
resp := map[string]any{
"data": []map[string]any{
{"read": false, "from_agent": "clotho", "subject": "task done"},
{"read": false, "from_agent": "gemini", "subject": "review ready"},
{"read": true, "from_agent": "clotho", "subject": "old msg"},
"messages": []map[string]any{
{"read": false, "from": "clotho", "subject": "task done"},
{"read": false, "from": "gemini", "subject": "review ready"},
{"read": true, "from": "clotho", "subject": "old msg"},
},
}
w.Header().Set("Content-Type", "application/json")
@ -262,8 +262,8 @@ func TestCheckInbox_Good_UnreadMessages(t *testing.T) {
func TestCheckInbox_Good_NoUnread(t *testing.T) {
srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
resp := map[string]any{
"data": []map[string]any{
{"read": true, "from_agent": "clotho", "subject": "old"},
"messages": []map[string]any{
{"read": true, "from": "clotho", "subject": "old"},
},
}
w.Header().Set("Content-Type", "application/json")
@ -281,8 +281,8 @@ func TestCheckInbox_Good_NoUnread(t *testing.T) {
func TestCheckInbox_Good_SameCountNoRepeat(t *testing.T) {
srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
resp := map[string]any{
"data": []map[string]any{
{"read": false, "from_agent": "clotho", "subject": "msg"},
"messages": []map[string]any{
{"read": false, "from": "clotho", "subject": "msg"},
},
}
w.Header().Set("Content-Type", "application/json")
@ -338,10 +338,10 @@ func TestCheckInbox_Bad_InvalidJSON(t *testing.T) {
func TestCheckInbox_Good_MultipleSameSender(t *testing.T) {
srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
resp := map[string]any{
"data": []map[string]any{
{"read": false, "from_agent": "clotho", "subject": "msg1"},
{"read": false, "from_agent": "clotho", "subject": "msg2"},
{"read": false, "from_agent": "gemini", "subject": "msg3"},
"messages": []map[string]any{
{"read": false, "from": "clotho", "subject": "msg1"},
{"read": false, "from": "clotho", "subject": "msg2"},
{"read": false, "from": "gemini", "subject": "msg3"},
},
}
w.Header().Set("Content-Type", "application/json")

View file

@ -75,12 +75,11 @@ func (m *Subsystem) syncRepos() string {
return ""
}
// Update timestamp for next checkin
m.mu.Lock()
m.lastSyncTimestamp = checkin.Timestamp
m.mu.Unlock()
if len(checkin.Changed) == 0 {
// No changes — safe to advance timestamp
m.mu.Lock()
m.lastSyncTimestamp = checkin.Timestamp
m.mu.Unlock()
return ""
}
@ -98,12 +97,22 @@ func (m *Subsystem) syncRepos() string {
continue
}
// Check if we're already on main and clean
// Check if on the default branch and clean
branchCmd := exec.Command("git", "rev-parse", "--abbrev-ref", "HEAD")
branchCmd.Dir = repoDir
branch, err := branchCmd.Output()
if err != nil || strings.TrimSpace(string(branch)) != "main" {
continue // Don't pull if not on main
currentBranch, err := branchCmd.Output()
if err != nil {
continue
}
current := strings.TrimSpace(string(currentBranch))
// Accept main or master (or whatever the repo reports)
expectedBranch := repo.Branch
if expectedBranch == "" {
expectedBranch = "main"
}
if current != expectedBranch && current != "main" && current != "master" {
continue // Don't pull if on a feature branch
}
statusCmd := exec.Command("git", "status", "--porcelain")
@ -113,14 +122,20 @@ func (m *Subsystem) syncRepos() string {
continue // Don't pull if dirty
}
// Fast-forward pull
pullCmd := exec.Command("git", "pull", "--ff-only", "origin", "main")
// Fast-forward pull on whatever branch we're on
pullCmd := exec.Command("git", "pull", "--ff-only", "origin", current)
pullCmd.Dir = repoDir
if pullCmd.Run() == nil {
pulled = append(pulled, repo.Repo)
}
}
// Only advance timestamp after attempting pulls — missed repos
// will be retried on the next cycle
m.mu.Lock()
m.lastSyncTimestamp = checkin.Timestamp
m.mu.Unlock()
if len(pulled) == 0 {
return ""
}