fix(dog): address code review issues in dispatch command
Fixes from code review: - Remove duplicate generateDogNameForDispatch, reuse generateDogName - Fix race condition: assign work BEFORE sending mail - Add rollback if mail send fails (clear work assignment) - Fix misleading help text (was "hooks mail", actually sends mail) - Add --json flag for scripted output - Add --dry-run flag to preview without executing The order change (assign work first, then send mail) ensures that if AssignWork fails, no mail has been sent. If mail fails after work is assigned, we rollback by clearing the work assignment. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
This commit is contained in:
@@ -32,6 +32,8 @@ var (
|
|||||||
dogDispatchRig string
|
dogDispatchRig string
|
||||||
dogDispatchCreate bool
|
dogDispatchCreate bool
|
||||||
dogDispatchDog string
|
dogDispatchDog string
|
||||||
|
dogDispatchJSON bool
|
||||||
|
dogDispatchDryRun bool
|
||||||
)
|
)
|
||||||
|
|
||||||
var dogCmd = &cobra.Command{
|
var dogCmd = &cobra.Command{
|
||||||
@@ -155,21 +157,20 @@ uses this during patrol cycles to dispatch plugins with open gates.
|
|||||||
|
|
||||||
The command:
|
The command:
|
||||||
1. Finds the plugin definition (plugin.md)
|
1. Finds the plugin definition (plugin.md)
|
||||||
2. Creates a mail work unit with plugin instructions
|
2. Assigns work to an idle dog (marks as working)
|
||||||
3. Hooks the mail to an idle dog
|
3. Sends mail with plugin instructions to the dog
|
||||||
4. Returns immediately (non-blocking)
|
4. Returns immediately (non-blocking)
|
||||||
|
|
||||||
|
The dog discovers the work via its mail inbox and executes the plugin
|
||||||
|
instructions. On completion, the dog sends DOG_DONE mail to deacon/.
|
||||||
|
|
||||||
Examples:
|
Examples:
|
||||||
gt dog dispatch --plugin rebuild-gt
|
gt dog dispatch --plugin rebuild-gt
|
||||||
gt dog dispatch --plugin rebuild-gt --rig gastown
|
gt dog dispatch --plugin rebuild-gt --rig gastown
|
||||||
gt dog dispatch --plugin rebuild-gt --dog alpha
|
gt dog dispatch --plugin rebuild-gt --dog alpha
|
||||||
gt dog dispatch --plugin rebuild-gt --create
|
gt dog dispatch --plugin rebuild-gt --create
|
||||||
|
gt dog dispatch --plugin rebuild-gt --dry-run
|
||||||
Flags:
|
gt dog dispatch --plugin rebuild-gt --json`,
|
||||||
--plugin Plugin name (required)
|
|
||||||
--rig Limit search to specific rig's plugins
|
|
||||||
--dog Dispatch to specific dog (otherwise picks idle)
|
|
||||||
--create Create a dog if none idle`,
|
|
||||||
RunE: runDogDispatch,
|
RunE: runDogDispatch,
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -192,6 +193,8 @@ func init() {
|
|||||||
dogDispatchCmd.Flags().StringVar(&dogDispatchRig, "rig", "", "Limit plugin search to specific rig")
|
dogDispatchCmd.Flags().StringVar(&dogDispatchRig, "rig", "", "Limit plugin search to specific rig")
|
||||||
dogDispatchCmd.Flags().StringVar(&dogDispatchDog, "dog", "", "Dispatch to specific dog (default: any idle)")
|
dogDispatchCmd.Flags().StringVar(&dogDispatchDog, "dog", "", "Dispatch to specific dog (default: any idle)")
|
||||||
dogDispatchCmd.Flags().BoolVar(&dogDispatchCreate, "create", false, "Create a dog if none idle")
|
dogDispatchCmd.Flags().BoolVar(&dogDispatchCreate, "create", false, "Create a dog if none idle")
|
||||||
|
dogDispatchCmd.Flags().BoolVar(&dogDispatchJSON, "json", false, "Output as JSON")
|
||||||
|
dogDispatchCmd.Flags().BoolVarP(&dogDispatchDryRun, "dry-run", "n", false, "Show what would be done without doing it")
|
||||||
_ = dogDispatchCmd.MarkFlagRequired("plugin")
|
_ = dogDispatchCmd.MarkFlagRequired("plugin")
|
||||||
|
|
||||||
// Add subcommands
|
// Add subcommands
|
||||||
@@ -666,18 +669,12 @@ func runDogDispatch(cmd *cobra.Command, args []string) error {
|
|||||||
return fmt.Errorf("finding plugin: %w", err)
|
return fmt.Errorf("finding plugin: %w", err)
|
||||||
}
|
}
|
||||||
|
|
||||||
fmt.Printf("%s Found plugin: %s\n", style.Bold.Render("✓"), p.Name)
|
|
||||||
if p.RigName != "" {
|
|
||||||
fmt.Printf(" Location: %s/plugins/%s\n", p.RigName, p.Name)
|
|
||||||
} else {
|
|
||||||
fmt.Printf(" Location: plugins/%s (town-level)\n", p.Name)
|
|
||||||
}
|
|
||||||
|
|
||||||
// Get dog manager (reuse rigsConfig from above)
|
// Get dog manager (reuse rigsConfig from above)
|
||||||
mgr := dog.NewManager(townRoot, rigsConfig)
|
mgr := dog.NewManager(townRoot, rigsConfig)
|
||||||
|
|
||||||
// Find target dog
|
// Find target dog
|
||||||
var targetDog *dog.Dog
|
var targetDog *dog.Dog
|
||||||
|
var dogCreated bool
|
||||||
if dogDispatchDog != "" {
|
if dogDispatchDog != "" {
|
||||||
// Specific dog requested
|
// Specific dog requested
|
||||||
targetDog, err = mgr.Get(dogDispatchDog)
|
targetDog, err = mgr.Get(dogDispatchDog)
|
||||||
@@ -696,19 +693,27 @@ func runDogDispatch(cmd *cobra.Command, args []string) error {
|
|||||||
|
|
||||||
if targetDog == nil {
|
if targetDog == nil {
|
||||||
if dogDispatchCreate {
|
if dogDispatchCreate {
|
||||||
// Create a new dog
|
// Create a new dog (reuse generateDogName from sling_dog.go)
|
||||||
newName := generateDogNameForDispatch(mgr)
|
newName := generateDogName(mgr)
|
||||||
targetDog, err = mgr.Add(newName)
|
if dogDispatchDryRun {
|
||||||
if err != nil {
|
targetDog = &dog.Dog{Name: newName, State: dog.StateIdle}
|
||||||
return fmt.Errorf("creating dog %s: %w", newName, err)
|
dogCreated = true
|
||||||
}
|
} else {
|
||||||
fmt.Printf("%s Created dog %s (pool was empty)\n", style.Bold.Render("✓"), newName)
|
targetDog, err = mgr.Add(newName)
|
||||||
|
if err != nil {
|
||||||
|
return fmt.Errorf("creating dog %s: %w", newName, err)
|
||||||
|
}
|
||||||
|
dogCreated = true
|
||||||
|
|
||||||
// Create agent bead for the dog
|
// Create agent bead for the dog
|
||||||
b := beads.New(townRoot)
|
b := beads.New(townRoot)
|
||||||
location := filepath.Join("deacon", "dogs", newName)
|
location := filepath.Join("deacon", "dogs", newName)
|
||||||
if _, err := b.CreateDogAgentBead(newName, location); err != nil {
|
if _, beadErr := b.CreateDogAgentBead(newName, location); beadErr != nil {
|
||||||
fmt.Printf(" Warning: could not create agent bead: %v\n", err)
|
// Non-fatal warning
|
||||||
|
if !dogDispatchJSON {
|
||||||
|
fmt.Printf(" Warning: could not create agent bead: %v\n", beadErr)
|
||||||
|
}
|
||||||
|
}
|
||||||
}
|
}
|
||||||
} else {
|
} else {
|
||||||
return fmt.Errorf("no idle dogs available (use --create to add one)")
|
return fmt.Errorf("no idle dogs available (use --create to add one)")
|
||||||
@@ -716,14 +721,48 @@ func runDogDispatch(cmd *cobra.Command, args []string) error {
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
fmt.Printf("%s Dispatching to dog: %s\n", style.Bold.Render("🐕"), targetDog.Name)
|
// Prepare dispatch result for JSON output
|
||||||
|
workDesc := fmt.Sprintf("plugin:%s", p.Name)
|
||||||
|
result := dogDispatchResult{
|
||||||
|
Plugin: p.Name,
|
||||||
|
PluginPath: p.Path,
|
||||||
|
Dog: targetDog.Name,
|
||||||
|
DogCreated: dogCreated,
|
||||||
|
Work: workDesc,
|
||||||
|
DryRun: dogDispatchDryRun,
|
||||||
|
}
|
||||||
|
if p.RigName != "" {
|
||||||
|
result.PluginRig = p.RigName
|
||||||
|
}
|
||||||
|
|
||||||
// Create mail message with plugin instructions
|
// Dry-run mode: show what would happen and exit
|
||||||
|
if dogDispatchDryRun {
|
||||||
|
if dogDispatchJSON {
|
||||||
|
return json.NewEncoder(os.Stdout).Encode(result)
|
||||||
|
}
|
||||||
|
fmt.Printf("Dry run - would dispatch:\n")
|
||||||
|
fmt.Printf(" Plugin: %s\n", p.Name)
|
||||||
|
if p.RigName != "" {
|
||||||
|
fmt.Printf(" Location: %s/plugins/%s\n", p.RigName, p.Name)
|
||||||
|
} else {
|
||||||
|
fmt.Printf(" Location: plugins/%s (town-level)\n", p.Name)
|
||||||
|
}
|
||||||
|
fmt.Printf(" Dog: %s%s\n", targetDog.Name, ifStr(dogCreated, " (would create)", ""))
|
||||||
|
fmt.Printf(" Work: %s\n", workDesc)
|
||||||
|
return nil
|
||||||
|
}
|
||||||
|
|
||||||
|
// Assign work FIRST (before sending mail) to prevent race condition
|
||||||
|
// If this fails, we haven't sent any mail yet
|
||||||
|
if err := mgr.AssignWork(targetDog.Name, workDesc); err != nil {
|
||||||
|
return fmt.Errorf("assigning work to dog: %w", err)
|
||||||
|
}
|
||||||
|
|
||||||
|
// Create and send mail message with plugin instructions
|
||||||
dogAddress := fmt.Sprintf("deacon/dogs/%s", targetDog.Name)
|
dogAddress := fmt.Sprintf("deacon/dogs/%s", targetDog.Name)
|
||||||
subject := fmt.Sprintf("Plugin: %s", p.Name)
|
subject := fmt.Sprintf("Plugin: %s", p.Name)
|
||||||
body := formatPluginMailBody(p)
|
body := formatPluginMailBody(p)
|
||||||
|
|
||||||
// Send mail to dog via router
|
|
||||||
router := mail.NewRouterWithTownRoot(townRoot, townRoot)
|
router := mail.NewRouterWithTownRoot(townRoot, townRoot)
|
||||||
msg := &mail.Message{
|
msg := &mail.Message{
|
||||||
From: "deacon/",
|
From: "deacon/",
|
||||||
@@ -734,15 +773,31 @@ func runDogDispatch(cmd *cobra.Command, args []string) error {
|
|||||||
}
|
}
|
||||||
|
|
||||||
if err := router.Send(msg); err != nil {
|
if err := router.Send(msg); err != nil {
|
||||||
|
// Rollback: clear work assignment since mail failed
|
||||||
|
if clearErr := mgr.ClearWork(targetDog.Name); clearErr != nil {
|
||||||
|
// Log rollback failure but return original error
|
||||||
|
if !dogDispatchJSON {
|
||||||
|
fmt.Printf(" Warning: rollback failed: %v\n", clearErr)
|
||||||
|
}
|
||||||
|
}
|
||||||
return fmt.Errorf("sending plugin mail to dog: %w", err)
|
return fmt.Errorf("sending plugin mail to dog: %w", err)
|
||||||
}
|
}
|
||||||
|
|
||||||
// Assign work to dog (updates state to working)
|
// Success - output result
|
||||||
workDesc := fmt.Sprintf("plugin:%s", p.Name)
|
if dogDispatchJSON {
|
||||||
if err := mgr.AssignWork(targetDog.Name, workDesc); err != nil {
|
return json.NewEncoder(os.Stdout).Encode(result)
|
||||||
return fmt.Errorf("assigning work to dog: %w", err)
|
|
||||||
}
|
}
|
||||||
|
|
||||||
|
fmt.Printf("%s Found plugin: %s\n", style.Bold.Render("✓"), p.Name)
|
||||||
|
if p.RigName != "" {
|
||||||
|
fmt.Printf(" Location: %s/plugins/%s\n", p.RigName, p.Name)
|
||||||
|
} else {
|
||||||
|
fmt.Printf(" Location: plugins/%s (town-level)\n", p.Name)
|
||||||
|
}
|
||||||
|
if dogCreated {
|
||||||
|
fmt.Printf("%s Created dog %s (pool was empty)\n", style.Bold.Render("✓"), targetDog.Name)
|
||||||
|
}
|
||||||
|
fmt.Printf("%s Dispatching to dog: %s\n", style.Bold.Render("🐕"), targetDog.Name)
|
||||||
fmt.Printf("%s Plugin dispatched (non-blocking)\n", style.Bold.Render("✓"))
|
fmt.Printf("%s Plugin dispatched (non-blocking)\n", style.Bold.Render("✓"))
|
||||||
fmt.Printf(" Dog: %s\n", targetDog.Name)
|
fmt.Printf(" Dog: %s\n", targetDog.Name)
|
||||||
fmt.Printf(" Work: %s\n", workDesc)
|
fmt.Printf(" Work: %s\n", workDesc)
|
||||||
@@ -750,31 +805,23 @@ func runDogDispatch(cmd *cobra.Command, args []string) error {
|
|||||||
return nil
|
return nil
|
||||||
}
|
}
|
||||||
|
|
||||||
// generateDogNameForDispatch creates a unique dog name for pool expansion.
|
// dogDispatchResult is the JSON output for gt dog dispatch.
|
||||||
func generateDogNameForDispatch(mgr *dog.Manager) string {
|
type dogDispatchResult struct {
|
||||||
names := []string{"alpha", "bravo", "charlie", "delta", "echo", "foxtrot", "golf", "hotel"}
|
Plugin string `json:"plugin"`
|
||||||
|
PluginRig string `json:"plugin_rig,omitempty"`
|
||||||
|
PluginPath string `json:"plugin_path"`
|
||||||
|
Dog string `json:"dog"`
|
||||||
|
DogCreated bool `json:"dog_created,omitempty"`
|
||||||
|
Work string `json:"work"`
|
||||||
|
DryRun bool `json:"dry_run,omitempty"`
|
||||||
|
}
|
||||||
|
|
||||||
dogs, _ := mgr.List()
|
// ifStr returns ifTrue if cond is true, otherwise ifFalse.
|
||||||
existing := make(map[string]bool)
|
func ifStr(cond bool, ifTrue, ifFalse string) string {
|
||||||
for _, d := range dogs {
|
if cond {
|
||||||
existing[d.Name] = true
|
return ifTrue
|
||||||
}
|
}
|
||||||
|
return ifFalse
|
||||||
for _, name := range names {
|
|
||||||
if !existing[name] {
|
|
||||||
return name
|
|
||||||
}
|
|
||||||
}
|
|
||||||
|
|
||||||
// Fallback: numbered dogs
|
|
||||||
for i := 1; i <= 100; i++ {
|
|
||||||
name := fmt.Sprintf("dog%d", i)
|
|
||||||
if !existing[name] {
|
|
||||||
return name
|
|
||||||
}
|
|
||||||
}
|
|
||||||
|
|
||||||
return fmt.Sprintf("dog%d", len(dogs)+1)
|
|
||||||
}
|
}
|
||||||
|
|
||||||
// formatPluginMailBody formats the plugin as instructions for the dog.
|
// formatPluginMailBody formats the plugin as instructions for the dog.
|
||||||
|
|||||||
Reference in New Issue
Block a user