Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 5 additions & 4 deletions cli/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,10 +9,11 @@ import (
)

type Config struct {
URL string `mapstructure:"url"`
APIKey string `mapstructure:"api_key"`
Realm string `mapstructure:"realm"`
Warnings []string `mapstructure:"-"`
URL string `mapstructure:"url"`
APIKey string `mapstructure:"api_key"`
Realm string `mapstructure:"realm"`
Orchestrate OrchestrateConfig `mapstructure:"orchestrate"`
Warnings []string `mapstructure:"-"`
}

func LoadConfig(workDir, homeDir string) (*Config, error) {
Expand Down
325 changes: 325 additions & 0 deletions cli/orchestrate.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,325 @@
package cli

import (
"context"
"encoding/json"
"fmt"
"os"
"os/signal"
"os/user"
"sync"
"syscall"
"time"

"github.com/spf13/cobra"
)

// OrchestrateConfig holds the orchestrate section of .bifrost.yaml.
type OrchestrateConfig struct {
Dispatcher string `mapstructure:"dispatcher"`
PollInterval time.Duration `mapstructure:"poll_interval"`
Concurrency int `mapstructure:"concurrency"`
Claimant string `mapstructure:"claimant"`
}

// OrchestrateCmd is the bf orchestrate command.
type OrchestrateCmd struct {
Command *cobra.Command
}

func NewOrchestrateCmd(clientFn func() *Client, cfgFn func() *Config) *OrchestrateCmd {
c := &OrchestrateCmd{}

cmd := &cobra.Command{
Use: "orchestrate",
Short: "Poll for ready runes and dispatch them to configured agents",
RunE: func(cmd *cobra.Command, args []string) error {
cfg := cfgFn()

// Resolve effective config: flags override yaml config.
oCfg := cfg.Orchestrate

if v, _ := cmd.Flags().GetString("dispatcher"); v != "" {
oCfg.Dispatcher = v
}
if v, _ := cmd.Flags().GetDuration("poll-interval"); v != 0 {
oCfg.PollInterval = v
}
if v, _ := cmd.Flags().GetInt("concurrency"); v != 0 {
oCfg.Concurrency = v
}
if v, _ := cmd.Flags().GetString("claimant"); v != "" {
oCfg.Claimant = v
}

// Apply defaults.
if oCfg.PollInterval == 0 {
oCfg.PollInterval = 10 * time.Second
}
if oCfg.Concurrency == 0 {
oCfg.Concurrency = 1
}
if oCfg.Claimant == "" {
if u, err := user.Current(); err == nil {
oCfg.Claimant = u.Username
}
}

// Validate configuration values.
if oCfg.Concurrency <= 0 {
return fmt.Errorf("concurrency must be positive, got %d", oCfg.Concurrency)
}
if oCfg.PollInterval <= 0 {
return fmt.Errorf("poll-interval must be positive, got %s", oCfg.PollInterval)
}

if oCfg.Dispatcher == "" {
return fmt.Errorf("dispatcher is required: set orchestrate.dispatcher in .bifrost.yaml or use --dispatcher")
}

// Validate dispatcher is accessible.
if _, err := os.Stat(oCfg.Dispatcher); err != nil {
return fmt.Errorf("dispatcher script not found: %s", oCfg.Dispatcher)
}

saga, _ := cmd.Flags().GetString("saga")
dryRun, _ := cmd.Flags().GetBool("dry-run")
once, _ := cmd.Flags().GetBool("once")
unclaimOnFailure, _ := cmd.Flags().GetBool("unclaim-on-failure")
Comment thread
coderabbitai[bot] marked this conversation as resolved.

dispatcher := &ScriptDispatcher{ScriptPath: oCfg.Dispatcher}

ctx, cancel := signal.NotifyContext(context.Background(), syscall.SIGINT, syscall.SIGTERM)
defer cancel()

return runOrchestrator(ctx, clientFn(), oCfg, dispatcher, saga, dryRun, once, unclaimOnFailure)
},
}

cmd.Flags().String("dispatcher", "", "path to dispatcher script (overrides config)")
cmd.Flags().Duration("poll-interval", 0, "polling interval (default 10s)")
cmd.Flags().Int("concurrency", 0, "number of parallel workers (default 1)")
cmd.Flags().String("claimant", "", "claimant name (default: system username)")
cmd.Flags().Bool("unclaim-on-failure", false, "unclaim rune when dispatched command exits non-zero")
cmd.Flags().String("saga", "", "only orchestrate runes in this saga")
cmd.Flags().Bool("dry-run", false, "resolve dispatch but do not execute or fulfill")
cmd.Flags().Bool("once", false, "process one batch then exit")

c.Command = cmd
return c
}

func runOrchestrator(
ctx context.Context,
client *Client,
cfg OrchestrateConfig,
dispatcher Dispatcher,
saga string,
dryRun, once, unclaimOnFailure bool,
) error {
queue := make(chan map[string]any, cfg.Concurrency*2)
var inFlight sync.Map

var wg sync.WaitGroup
for i := 0; i < cfg.Concurrency; i++ {
wg.Add(1)
go func() {
defer wg.Done()
for {
select {
case <-ctx.Done():
return
case rune, ok := <-queue:
if !ok {
return
}
processRune(ctx, client, cfg, dispatcher, rune, dryRun, unclaimOnFailure, &inFlight)
}
}
}()
}

poll := func() {
runes, err := fetchReadyRunes(client, saga)
if err != nil {
fmt.Fprintf(os.Stderr, "orchestrate: poll error: %v\n", err)
return
}

for _, r := range runes {
id, _ := r["id"].(string)
if id == "" {
continue
}
// Skip if already claimed by someone else.
if claimant, _ := r["claimant"].(string); claimant != "" {
continue
}
// Skip if already in-flight.
if _, loaded := inFlight.LoadOrStore(id, struct{}{}); loaded {
continue
}
// Blocking send in --once mode to guarantee all items are enqueued.
// Non-blocking send otherwise — if queue is full, release from in-flight and skip.
if once {
queue <- r
} else {
Comment on lines +162 to +166

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Make --once enqueue cancellation-aware.

The blocking queue <- r fixes batch loss, but it also means the main goroutine cannot observe ctx.Done() while the queue is full. If a worker is stuck in processRune, SIGINT/SIGTERM can leave runOrchestrator hung inside poll() instead of shutting down.

Suggested fix
 			// Blocking send in --once mode to guarantee all items are enqueued.
 			// Non-blocking send otherwise — if queue is full, release from in-flight and skip.
 			if once {
-				queue <- r
+				select {
+				case queue <- r:
+				case <-ctx.Done():
+					inFlight.Delete(id)
+					return
+				}
 			} else {
 				select {
 				case queue <- r:
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
// Blocking send in --once mode to guarantee all items are enqueued.
// Non-blocking send otherwise — if queue is full, release from in-flight and skip.
if once {
queue <- r
} else {
// Blocking send in --once mode to guarantee all items are enqueued.
// Non-blocking send otherwise — if queue is full, release from in-flight and skip.
if once {
select {
case queue <- r:
case <-ctx.Done():
inFlight.Delete(id)
return
}
} else {
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@cli/orchestrate.go` around lines 162 - 166, The blocking enqueue in
runOrchestrator (the branch using `once` with `queue <- r`) should be made
cancellation-aware so the main goroutine can observe ctx.Done() when the queue
is full; replace the direct send with a select that waits on either `queue <- r`
or `ctx.Done()` and, on ctx cancellation, stop enqueuing (return or break out to
allow shutdown) instead of blocking indefinitely inside poll() while a worker
(e.g., processRune) is stuck. Ensure the change references the same variables
(`once`, `queue`, `r`, `ctx.Done()`) and that any in-flight bookkeeping remains
consistent when you bail out.

select {
case queue <- r:
default:
inFlight.Delete(id)
}
}
}
}

// Run first poll immediately.
poll()

if once {
// Wait for queue to drain, then shut down workers.
// We close the queue after the first poll drains.
// We signal workers by closing the channel once all items are enqueued.
// But since queue is buffered and workers are async, we need to wait
// for in-flight items to complete. We do this by closing queue and
// waiting for wg.
close(queue)
wg.Wait()
return nil
}

ticker := time.NewTicker(cfg.PollInterval)
defer ticker.Stop()

for {
select {
case <-ctx.Done():
close(queue)
wg.Wait()
return nil
case <-ticker.C:
poll()
}
}
}

func fetchReadyRunes(client *Client, saga string) ([]map[string]any, error) {
params := map[string]string{
"status": "open",
"blocked": "false",
"is_saga": "false",
}
if saga != "" {
params["saga"] = saga
}

body, err := client.DoGetWithParams("/runes", params)
if err != nil {
return nil, err
Comment on lines +206 to +218

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash

# Find and examine cli/orchestrate.go
echo "=== Checking cli/orchestrate.go structure ==="
wc -l cli/orchestrate.go

echo -e "\n=== Lines 206-218 (fetchReadyRunes) ==="
sed -n '206,218p' cli/orchestrate.go

echo -e "\n=== Lines 299-323 section ==="
sed -n '299,323p' cli/orchestrate.go

echo -e "\n=== Looking for runOrchestrator function ==="
grep -n "func runOrchestrator" cli/orchestrate.go

echo -e "\n=== Checking all API call functions mentioned ==="
grep -n "func fetchReadyRunes\|func fetchRuneDetail\|func claimRune\|func unclaimRune\|func fulfillRune" cli/orchestrate.go

Repository: devzeebo/bifrost

Length of output: 1820


🏁 Script executed:

#!/bin/bash

# Examine cli/client.go for DoGetWithParams and DoPost signatures
echo "=== Checking DoGetWithParams signature ==="
grep -A 5 "func.*DoGetWithParams" cli/client.go

echo -e "\n=== Checking DoPost signature ==="
grep -A 5 "func.*DoPost" cli/client.go

echo -e "\n=== Checking Client struct ==="
grep -A 15 "type Client struct" cli/client.go

echo -e "\n=== Looking for context in Client methods ==="
rg "ctx.*context\.|context\.Context" cli/client.go -A 3

Repository: devzeebo/bifrost

Length of output: 882


🏁 Script executed:

#!/bin/bash

echo "=== runOrchestrator function (starting at line 112) ==="
sed -n '112,170p' cli/orchestrate.go

echo -e "\n=== Full runOrchestrator signature ==="
grep -A 20 "func runOrchestrator" cli/orchestrate.go | head -25

Repository: devzeebo/bifrost

Length of output: 1958


🏁 Script executed:

#!/bin/bash

echo "=== Checking for context.Context in runOrchestrator ==="
sed -n '112,200p' cli/orchestrate.go | grep -i "context\|ctx"

echo -e "\n=== Searching for cmd.CommandContext usage or context setup ==="
rg "CommandContext|context\.|ctx" cli/orchestrate.go -B 2 -A 2

Repository: devzeebo/bifrost

Length of output: 1544


🏁 Script executed:

#!/bin/bash

echo "=== processRune function ==="
sed -n '224,298p' cli/orchestrate.go

echo -e "\n=== Where is poll() called ==="
sed -n '150,200p' cli/orchestrate.go | grep -A 5 -B 5 "poll()"

Repository: devzeebo/bifrost

Length of output: 2193


🏁 Script executed:

#!/bin/bash

echo "=== Full poll() and main loop context ==="
sed -n '138,203p' cli/orchestrate.go

Repository: devzeebo/bifrost

Length of output: 1528


Propagate ctx into the Bifrost API calls to enable graceful shutdown.

runOrchestrator properly passes context to the dispatcher and subprocess calls (dispatcher.Dispatch(ctx, ...) and RunDispatched(ctx, ...)), but the API operations—fetchReadyRunes, fetchRuneDetail, claimRune, unclaimRune, and fulfillRune—all invoke Client.DoGetWithParams and Client.DoPost without context support. A slow or hung API request in any of these functions will block the worker goroutines indefinitely, preventing wg.Wait() from completing even after SIGINT/SIGTERM triggers ctx.Done(). Update the Client methods and their callers to accept and respect context cancellation.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@cli/orchestrate.go` around lines 206 - 218, The API calls block on
non-cancellable HTTP calls; update Client.DoGetWithParams and Client.DoPost to
accept a context.Context and use it when creating/performing requests, then
propagate that context through all callers: change fetchReadyRunes(ctx, client,
saga), fetchRuneDetail(ctx, ...), claimRune(ctx, ...), unclaimRune(ctx, ...),
and fulfillRune(ctx, ...) to accept ctx and call the new context-aware Client
methods, and ensure runOrchestrator continues to pass its ctx into
dispatcher.Dispatch(ctx, ...) and RunDispatched(ctx, ...) as well as into these
API wrappers so any ctx.Done() cancels in-flight requests promptly.

}

var runes []map[string]any
if err := json.Unmarshal(body, &runes); err != nil {
return nil, fmt.Errorf("parsing runes response: %w", err)
}
return runes, nil
}

func processRune(
ctx context.Context,
client *Client,
cfg OrchestrateConfig,
dispatcher Dispatcher,
summary map[string]any,
dryRun, unclaimOnFailure bool,
inFlight *sync.Map,
) {
id, _ := summary["id"].(string)
defer inFlight.Delete(id)

// Fetch full rune detail.
detail, err := fetchRuneDetail(client, id)
if err != nil {
fmt.Fprintf(os.Stderr, "orchestrate: [%s] fetch detail error: %v\n", id, err)
return
}

// Claim the rune.
if err := claimRune(client, id, cfg.Claimant); err != nil {
fmt.Fprintf(os.Stderr, "orchestrate: [%s] claim error: %v\n", id, err)
return
}

// Resolve dispatch.
input := dispatchInputFromRune(detail)
result, err := dispatcher.Dispatch(ctx, input)
if err != nil {
fmt.Fprintf(os.Stderr, "orchestrate: [%s] dispatcher error: %v\n", id, err)
unclaimRune(client, id)
return
}

// Empty command means skip.
if result.Command == "" {
fmt.Fprintf(os.Stderr, "orchestrate: [%s] no handler, unclaiming\n", id)
unclaimRune(client, id)
return
}

if dryRun {
fmt.Fprintf(os.Stderr, "orchestrate: [%s] dry-run: would invoke: %s %v\n", id, result.Command, result.Args)
unclaimRune(client, id)
return
}

fmt.Fprintf(os.Stderr, "orchestrate: [%s] invoking: %s %v\n", id, result.Command, result.Args)

exitCode, err := RunDispatched(ctx, result, os.Stdout, os.Stderr)
if err != nil {
fmt.Fprintf(os.Stderr, "orchestrate: [%s] exec error: %v\n", id, err)
if unclaimOnFailure {
unclaimRune(client, id)
}
return
}

if exitCode != 0 {
fmt.Fprintf(os.Stderr, "orchestrate: [%s] agent exited with code %d\n", id, exitCode)
if unclaimOnFailure {
unclaimRune(client, id)
}
return
}

if err := fulfillRune(client, id); err != nil {
fmt.Fprintf(os.Stderr, "orchestrate: [%s] fulfill error: %v\n", id, err)
}
}

func fetchRuneDetail(client *Client, id string) (map[string]any, error) {
body, err := client.DoGetWithParams("/rune", map[string]string{"id": id})
if err != nil {
return nil, err
}
var detail map[string]any
if err := json.Unmarshal(body, &detail); err != nil {
return nil, fmt.Errorf("parsing rune detail: %w", err)
}
return detail, nil
}

func claimRune(client *Client, id, claimant string) error {
_, err := client.DoPost("/claim-rune", map[string]string{"id": id, "claimant": claimant})
return err
}

func unclaimRune(client *Client, id string) {
if _, err := client.DoPost("/unclaim-rune", map[string]string{"id": id}); err != nil {
fmt.Fprintf(os.Stderr, "orchestrate: [%s] unclaim error: %v\n", id, err)
}
}

func fulfillRune(client *Client, id string) error {
_, err := client.DoPost("/fulfill-rune", map[string]string{"id": id})
return err
}
Loading
Loading