fix(subagents): parallelize child executable lookup
- commit
- 64addb9ec7165c30de75c2ea9ec5d261618dfffd
- author
- brice · · 2026-08-05 05:22 UTC
- parent
- 3ef5f9b
- signature
- GPG Verified
CD88F9E564E0102646FB6077A1A07C7C7C3E349D· arch-gpg Verified for the committer email
2 files · +77 -26
M packages/agent/subagents/runner.go +45 -14
| @@ -166,14 +166,25 @@ func subagentWorkerArgs(opts subagentWorkerArgsOpts) []string { | |||
| 166 | 166 | ||
| 167 | 167 | // resolveSubagentExecutable finds a runnable zot binary without assuming a | |
| 168 | 168 | // particular installation directory. A parent process can outlive the file | |
| 169 | - | // it was started from (for example after a reinstall), so the current | |
| 170 | - | // executable and argv[0] are tried first and PATH is used as the fallback. | |
| 171 | - | // Keeping the lookup on PATH is important: it covers GOBIN, user-local bin | |
| 169 | + | // it was started from (for example after a reinstall), so all known | |
| 170 | + | // candidates are checked concurrently and the first successful lookup wins. | |
| 171 | + | // The cancellation signal stops candidates that have not finished yet. PATH | |
| 172 | + | // is one of the candidates because it covers GOBIN, user-local bin | |
| 172 | 173 | // directories, package-manager locations, and system installs without | |
| 173 | 174 | // hard-coding any of them. | |
| 174 | - | func resolveSubagentExecutable(self, argv0 string, lookPath func(string) (string, error)) (string, error) { | |
| 175 | + | func resolveSubagentExecutable(ctx context.Context, self, argv0 string, lookPath func(context.Context, string) (string, error)) (string, error) { | |
| 176 | + | if ctx == nil { | |
| 177 | + | ctx = context.Background() | |
| 178 | + | } | |
| 175 | 179 | if lookPath == nil { | |
| 176 | - | lookPath = exec.LookPath | |
| 180 | + | lookPath = func(ctx context.Context, candidate string) (string, error) { | |
| 181 | + | select { | |
| 182 | + | case <-ctx.Done(): | |
| 183 | + | return "", ctx.Err() | |
| 184 | + | default: | |
| 185 | + | } | |
| 186 | + | return exec.LookPath(candidate) | |
| 187 | + | } | |
| 177 | 188 | } | |
| 178 | 189 | ||
| 179 | 190 | candidates := make([]string, 0, 5) | |
| @@ -198,16 +209,36 @@ func resolveSubagentExecutable(self, argv0 string, lookPath func(string) (string | |||
| 198 | 209 | } | |
| 199 | 210 | addCandidate("zot") | |
| 200 | 211 | ||
| 201 | - | var lookupErrors []string | |
| 212 | + | if len(candidates) == 0 { | |
| 213 | + | return "", fmt.Errorf("locate zot executable: no executable candidates") | |
| 214 | + | } | |
| 215 | + | ||
| 216 | + | type lookupResult struct { | |
| 217 | + | candidate string | |
| 218 | + | path string | |
| 219 | + | err error | |
| 220 | + | } | |
| 221 | + | childCtx, cancel := context.WithCancel(ctx) | |
| 222 | + | defer cancel() | |
| 223 | + | results := make(chan lookupResult, len(candidates)) | |
| 202 | 224 | for _, candidate := range candidates { | |
| 203 | - | resolved, err := lookPath(candidate) | |
| 204 | - | if err == nil { | |
| 205 | - | return resolved, nil | |
| 206 | - | } | |
| 207 | - | lookupErrors = append(lookupErrors, fmt.Sprintf("%q: %v", candidate, err)) | |
| 225 | + | go func(candidate string) { | |
| 226 | + | path, err := lookPath(childCtx, candidate) | |
| 227 | + | if err == nil && path == "" { | |
| 228 | + | err = fmt.Errorf("empty executable path") | |
| 229 | + | } | |
| 230 | + | results <- lookupResult{candidate: candidate, path: path, err: err} | |
| 231 | + | }(candidate) | |
| 208 | 232 | } | |
| 209 | - | if len(lookupErrors) == 0 { | |
| 210 | - | return "", fmt.Errorf("locate zot executable: no executable candidates") | |
| 233 | + | ||
| 234 | + | lookupErrors := make([]string, 0, len(candidates)) | |
| 235 | + | for range candidates { | |
| 236 | + | result := <-results | |
| 237 | + | if result.err == nil { | |
| 238 | + | cancel() | |
| 239 | + | return result.path, nil | |
| 240 | + | } | |
| 241 | + | lookupErrors = append(lookupErrors, fmt.Sprintf("%q: %v", result.candidate, result.err)) | |
| 211 | 242 | } | |
| 212 | 243 | return "", fmt.Errorf("locate zot executable: %s", strings.Join(lookupErrors, "; ")) | |
| 213 | 244 | } | |
| @@ -257,7 +288,7 @@ func (r *execRunner) Run(ctx context.Context, sink Sink) error { | |||
| 257 | 288 | if len(os.Args) > 0 { | |
| 258 | 289 | argv0 = os.Args[0] | |
| 259 | 290 | } | |
| 260 | - | exe, err := resolveSubagentExecutable(self, argv0, exec.LookPath) | |
| 291 | + | exe, err := resolveSubagentExecutable(ctx, self, argv0, nil) | |
| 261 | 292 | if err != nil { | |
| 262 | 293 | if selfErr != nil { | |
| 263 | 294 | return fmt.Errorf("locate zot executable (os.Executable: %v): %w", selfErr, err) | |
| @@ -166,14 +166,25 @@ func subagentWorkerArgs(opts subagentWorkerArgsOpts) []string { | |||||
| 166 | 166 | ||||
| 167 | // resolveSubagentExecutable finds a runnable zot binary without assuming a | 167 | // resolveSubagentExecutable finds a runnable zot binary without assuming a | ||
| 168 | // particular installation directory. A parent process can outlive the file | 168 | // particular installation directory. A parent process can outlive the file | ||
| 169 | - | // it was started from (for example after a reinstall), so the current | 169 | + | // it was started from (for example after a reinstall), so all known |
| 170 | - | // executable and argv[0] are tried first and PATH is used as the fallback. | 170 | + | // candidates are checked concurrently and the first successful lookup wins. |
| 171 | - | // Keeping the lookup on PATH is important: it covers GOBIN, user-local bin | 171 | + | // The cancellation signal stops candidates that have not finished yet. PATH |
| 172 | + | // is one of the candidates because it covers GOBIN, user-local bin | |||
| 172 | // directories, package-manager locations, and system installs without | 173 | // directories, package-manager locations, and system installs without | ||
| 173 | // hard-coding any of them. | 174 | // hard-coding any of them. | ||
| 174 | - | func resolveSubagentExecutable(self, argv0 string, lookPath func(string) (string, error)) (string, error) { | 175 | + | func resolveSubagentExecutable(ctx context.Context, self, argv0 string, lookPath func(context.Context, string) (string, error)) (string, error) { |
| 176 | + | if ctx == nil { | |||
| 177 | + | ctx = context.Background() | |||
| 178 | + | } | |||
| 175 | if lookPath == nil { | 179 | if lookPath == nil { | ||
| 176 | - | lookPath = exec.LookPath | 180 | + | lookPath = func(ctx context.Context, candidate string) (string, error) { |
| 181 | + | select { | |||
| 182 | + | case <-ctx.Done(): | |||
| 183 | + | return "", ctx.Err() | |||
| 184 | + | default: | |||
| 185 | + | } | |||
| 186 | + | return exec.LookPath(candidate) | |||
| 187 | + | } | |||
| 177 | } | 188 | } | ||
| 178 | 189 | ||||
| 179 | candidates := make([]string, 0, 5) | 190 | candidates := make([]string, 0, 5) | ||
| @@ -198,16 +209,36 @@ func resolveSubagentExecutable(self, argv0 string, lookPath func(string) (string | |||||
| 198 | } | 209 | } | ||
| 199 | addCandidate("zot") | 210 | addCandidate("zot") | ||
| 200 | 211 | ||||
| 201 | - | var lookupErrors []string | 212 | + | if len(candidates) == 0 { |
| 213 | + | return "", fmt.Errorf("locate zot executable: no executable candidates") | |||
| 214 | + | } | |||
| 215 | + | ||||
| 216 | + | type lookupResult struct { | |||
| 217 | + | candidate string | |||
| 218 | + | path string | |||
| 219 | + | err error | |||
| 220 | + | } | |||
| 221 | + | childCtx, cancel := context.WithCancel(ctx) | |||
| 222 | + | defer cancel() | |||
| 223 | + | results := make(chan lookupResult, len(candidates)) | |||
| 202 | for _, candidate := range candidates { | 224 | for _, candidate := range candidates { | ||
| 203 | - | resolved, err := lookPath(candidate) | 225 | + | go func(candidate string) { |
| 204 | - | if err == nil { | 226 | + | path, err := lookPath(childCtx, candidate) |
| 205 | - | return resolved, nil | 227 | + | if err == nil && path == "" { |
| 206 | - | } | 228 | + | err = fmt.Errorf("empty executable path") |
| 207 | - | lookupErrors = append(lookupErrors, fmt.Sprintf("%q: %v", candidate, err)) | 229 | + | } |
| 230 | + | results <- lookupResult{candidate: candidate, path: path, err: err} | |||
| 231 | + | }(candidate) | |||
| 208 | } | 232 | } | ||
| 209 | - | if len(lookupErrors) == 0 { | 233 | + | |
| 210 | - | return "", fmt.Errorf("locate zot executable: no executable candidates") | 234 | + | lookupErrors := make([]string, 0, len(candidates)) |
| 235 | + | for range candidates { | |||
| 236 | + | result := <-results | |||
| 237 | + | if result.err == nil { | |||
| 238 | + | cancel() | |||
| 239 | + | return result.path, nil | |||
| 240 | + | } | |||
| 241 | + | lookupErrors = append(lookupErrors, fmt.Sprintf("%q: %v", result.candidate, result.err)) | |||
| 211 | } | 242 | } | ||
| 212 | return "", fmt.Errorf("locate zot executable: %s", strings.Join(lookupErrors, "; ")) | 243 | return "", fmt.Errorf("locate zot executable: %s", strings.Join(lookupErrors, "; ")) | ||
| 213 | } | 244 | } | ||
| @@ -257,7 +288,7 @@ func (r *execRunner) Run(ctx context.Context, sink Sink) error { | |||||
| 257 | if len(os.Args) > 0 { | 288 | if len(os.Args) > 0 { | ||
| 258 | argv0 = os.Args[0] | 289 | argv0 = os.Args[0] | ||
| 259 | } | 290 | } | ||
| 260 | - | exe, err := resolveSubagentExecutable(self, argv0, exec.LookPath) | 291 | + | exe, err := resolveSubagentExecutable(ctx, self, argv0, nil) |
| 261 | if err != nil { | 292 | if err != nil { | ||
| 262 | if selfErr != nil { | 293 | if selfErr != nil { | ||
| 263 | return fmt.Errorf("locate zot executable (os.Executable: %v): %w", selfErr, err) | 294 | return fmt.Errorf("locate zot executable (os.Executable: %v): %w", selfErr, err) | ||
M packages/agent/subagents/runner_test.go +32 -12
| @@ -194,10 +194,8 @@ func TestSubagentWorkerArgs(t *testing.T) { | |||
| 194 | 194 | } | |
| 195 | 195 | } | |
| 196 | 196 | ||
| 197 | - | func TestResolveSubagentExecutablePrefersCurrentExecutable(t *testing.T) { | |
| 198 | - | var calls []string | |
| 199 | - | got, err := resolveSubagentExecutable("/current/bin/zot", "/old/bin/zot", func(candidate string) (string, error) { | |
| 200 | - | calls = append(calls, candidate) | |
| 197 | + | func TestResolveSubagentExecutableUsesCurrentWhenAvailable(t *testing.T) { | |
| 198 | + | got, err := resolveSubagentExecutable(context.Background(), "/current/bin/zot", "/old/bin/zot", func(_ context.Context, candidate string) (string, error) { | |
| 201 | 199 | if candidate == "/current/bin/zot" { | |
| 202 | 200 | return candidate, nil | |
| 203 | 201 | } | |
| @@ -209,15 +207,10 @@ func TestResolveSubagentExecutablePrefersCurrentExecutable(t *testing.T) { | |||
| 209 | 207 | if got != "/current/bin/zot" { | |
| 210 | 208 | t.Fatalf("executable = %q, want current executable", got) | |
| 211 | 209 | } | |
| 212 | - | if len(calls) != 1 || calls[0] != "/current/bin/zot" { | |
| 213 | - | t.Fatalf("lookup calls = %v, want only the current executable", calls) | |
| 214 | - | } | |
| 215 | 210 | } | |
| 216 | 211 | ||
| 217 | 212 | func TestResolveSubagentExecutableFallsBackToPath(t *testing.T) { | |
| 218 | - | var calls []string | |
| 219 | - | got, err := resolveSubagentExecutable("/removed/.local/bin/zot", "/removed/.local/bin/zot", func(candidate string) (string, error) { | |
| 220 | - | calls = append(calls, candidate) | |
| 213 | + | got, err := resolveSubagentExecutable(context.Background(), "/removed/.local/bin/zot", "/removed/.local/bin/zot", func(_ context.Context, candidate string) (string, error) { | |
| 221 | 214 | if candidate == "zot" { | |
| 222 | 215 | return "/active/go/bin/zot", nil | |
| 223 | 216 | } | |
| @@ -229,8 +222,35 @@ func TestResolveSubagentExecutableFallsBackToPath(t *testing.T) { | |||
| 229 | 222 | if got != "/active/go/bin/zot" { | |
| 230 | 223 | t.Fatalf("executable = %q, want PATH fallback", got) | |
| 231 | 224 | } | |
| 232 | - | if len(calls) != 2 || calls[0] != "/removed/.local/bin/zot" || calls[1] != "zot" { | |
| 233 | - | t.Fatalf("lookup calls = %v, want stale path followed by PATH lookup", calls) | |
| 225 | + | } | |
| 226 | + | ||
| 227 | + | func TestResolveSubagentExecutableCancelsOtherLookupsAfterFirstSuccess(t *testing.T) { | |
| 228 | + | slowStarted := make(chan struct{}) | |
| 229 | + | slowCanceled := make(chan struct{}) | |
| 230 | + | got, err := resolveSubagentExecutable(context.Background(), "/current/bin/zot", "/old/bin/zot", func(ctx context.Context, candidate string) (string, error) { | |
| 231 | + | switch candidate { | |
| 232 | + | case "/current/bin/zot": | |
| 233 | + | close(slowStarted) | |
| 234 | + | <-ctx.Done() | |
| 235 | + | close(slowCanceled) | |
| 236 | + | return "", ctx.Err() | |
| 237 | + | case "zot": | |
| 238 | + | <-slowStarted | |
| 239 | + | return "/active/go/bin/zot", nil | |
| 240 | + | default: | |
| 241 | + | return "", os.ErrNotExist | |
| 242 | + | } | |
| 243 | + | }) | |
| 244 | + | if err != nil { | |
| 245 | + | t.Fatal(err) | |
| 246 | + | } | |
| 247 | + | if got != "/active/go/bin/zot" { | |
| 248 | + | t.Fatalf("executable = %q, want first successful lookup", got) | |
| 249 | + | } | |
| 250 | + | select { | |
| 251 | + | case <-slowCanceled: | |
| 252 | + | case <-time.After(time.Second): | |
| 253 | + | t.Fatal("slower lookup was not canceled") | |
| 234 | 254 | } | |
| 235 | 255 | } | |
| 236 | 256 | ||
| @@ -194,10 +194,8 @@ func TestSubagentWorkerArgs(t *testing.T) { | |||||
| 194 | } | 194 | } | ||
| 195 | } | 195 | } | ||
| 196 | 196 | ||||
| 197 | - | func TestResolveSubagentExecutablePrefersCurrentExecutable(t *testing.T) { | 197 | + | func TestResolveSubagentExecutableUsesCurrentWhenAvailable(t *testing.T) { |
| 198 | - | var calls []string | 198 | + | got, err := resolveSubagentExecutable(context.Background(), "/current/bin/zot", "/old/bin/zot", func(_ context.Context, candidate string) (string, error) { |
| 199 | - | got, err := resolveSubagentExecutable("/current/bin/zot", "/old/bin/zot", func(candidate string) (string, error) { | |||
| 200 | - | calls = append(calls, candidate) | |||
| 201 | if candidate == "/current/bin/zot" { | 199 | if candidate == "/current/bin/zot" { | ||
| 202 | return candidate, nil | 200 | return candidate, nil | ||
| 203 | } | 201 | } | ||
| @@ -209,15 +207,10 @@ func TestResolveSubagentExecutablePrefersCurrentExecutable(t *testing.T) { | |||||
| 209 | if got != "/current/bin/zot" { | 207 | if got != "/current/bin/zot" { | ||
| 210 | t.Fatalf("executable = %q, want current executable", got) | 208 | t.Fatalf("executable = %q, want current executable", got) | ||
| 211 | } | 209 | } | ||
| 212 | - | if len(calls) != 1 || calls[0] != "/current/bin/zot" { | |||
| 213 | - | t.Fatalf("lookup calls = %v, want only the current executable", calls) | |||
| 214 | - | } | |||
| 215 | } | 210 | } | ||
| 216 | 211 | ||||
| 217 | func TestResolveSubagentExecutableFallsBackToPath(t *testing.T) { | 212 | func TestResolveSubagentExecutableFallsBackToPath(t *testing.T) { | ||
| 218 | - | var calls []string | 213 | + | got, err := resolveSubagentExecutable(context.Background(), "/removed/.local/bin/zot", "/removed/.local/bin/zot", func(_ context.Context, candidate string) (string, error) { |
| 219 | - | got, err := resolveSubagentExecutable("/removed/.local/bin/zot", "/removed/.local/bin/zot", func(candidate string) (string, error) { | |||
| 220 | - | calls = append(calls, candidate) | |||
| 221 | if candidate == "zot" { | 214 | if candidate == "zot" { | ||
| 222 | return "/active/go/bin/zot", nil | 215 | return "/active/go/bin/zot", nil | ||
| 223 | } | 216 | } | ||
| @@ -229,8 +222,35 @@ func TestResolveSubagentExecutableFallsBackToPath(t *testing.T) { | |||||
| 229 | if got != "/active/go/bin/zot" { | 222 | if got != "/active/go/bin/zot" { | ||
| 230 | t.Fatalf("executable = %q, want PATH fallback", got) | 223 | t.Fatalf("executable = %q, want PATH fallback", got) | ||
| 231 | } | 224 | } | ||
| 232 | - | if len(calls) != 2 || calls[0] != "/removed/.local/bin/zot" || calls[1] != "zot" { | 225 | + | } |
| 233 | - | t.Fatalf("lookup calls = %v, want stale path followed by PATH lookup", calls) | 226 | + | |
| 227 | + | func TestResolveSubagentExecutableCancelsOtherLookupsAfterFirstSuccess(t *testing.T) { | |||
| 228 | + | slowStarted := make(chan struct{}) | |||
| 229 | + | slowCanceled := make(chan struct{}) | |||
| 230 | + | got, err := resolveSubagentExecutable(context.Background(), "/current/bin/zot", "/old/bin/zot", func(ctx context.Context, candidate string) (string, error) { | |||
| 231 | + | switch candidate { | |||
| 232 | + | case "/current/bin/zot": | |||
| 233 | + | close(slowStarted) | |||
| 234 | + | <-ctx.Done() | |||
| 235 | + | close(slowCanceled) | |||
| 236 | + | return "", ctx.Err() | |||
| 237 | + | case "zot": | |||
| 238 | + | <-slowStarted | |||
| 239 | + | return "/active/go/bin/zot", nil | |||
| 240 | + | default: | |||
| 241 | + | return "", os.ErrNotExist | |||
| 242 | + | } | |||
| 243 | + | }) | |||
| 244 | + | if err != nil { | |||
| 245 | + | t.Fatal(err) | |||
| 246 | + | } | |||
| 247 | + | if got != "/active/go/bin/zot" { | |||
| 248 | + | t.Fatalf("executable = %q, want first successful lookup", got) | |||
| 249 | + | } | |||
| 250 | + | select { | |||
| 251 | + | case <-slowCanceled: | |||
| 252 | + | case <-time.After(time.Second): | |||
| 253 | + | t.Fatal("slower lookup was not canceled") | |||
| 234 | } | 254 | } | ||
| 235 | } | 255 | } | ||
| 236 | 256 | ||||