Make prompt debug creation concurrency safe
This commit is contained in:
File diff suppressed because it is too large
Load Diff
@@ -26,7 +26,7 @@ func TestExecuteComparisonProfilesRunsOrderedProfilesConcurrently(t *testing.T)
|
||||
Prepared: prepared, Inspection: comparisonInspection(prompt, profiles), ComparisonID: "comparison_daily", Executor: executor,
|
||||
})
|
||||
}()
|
||||
waitForProfileStarts(t, executor, profiles)
|
||||
waitForProfileStarts(t, executor, profiles, results)
|
||||
if executor.maximumInFlight() < 2 {
|
||||
t.Fatalf("maximum in-flight executions = %d, want overlap", executor.maximumInFlight())
|
||||
}
|
||||
@@ -64,7 +64,7 @@ func TestExecuteComparisonProfilesContinuesAfterProfileFailure(t *testing.T) {
|
||||
Prepared: prepared, Inspection: comparisonInspection(prompt, profiles), ComparisonID: "comparison_daily", Executor: executor,
|
||||
})
|
||||
}()
|
||||
waitForProfileStarts(t, executor, profiles)
|
||||
waitForProfileStarts(t, executor, profiles, results)
|
||||
for _, profile := range profiles {
|
||||
executor.release(profile.ProfileID)
|
||||
}
|
||||
@@ -90,7 +90,7 @@ func TestExecuteComparisonProfilesPropagatesCancellationAndJoins(t *testing.T) {
|
||||
Prepared: prepared, Inspection: comparisonInspection(prompt, profiles), ComparisonID: "comparison_daily", Executor: executor,
|
||||
})
|
||||
}()
|
||||
waitForProfileStarts(t, executor, profiles)
|
||||
waitForProfileStarts(t, executor, profiles, results)
|
||||
cancel()
|
||||
result := <-results
|
||||
if !result.Canceled || executor.inFlightCount() != 0 {
|
||||
@@ -120,7 +120,7 @@ func TestExecuteComparisonProfilesUsesDistinctDeterministicDebugReferences(t *te
|
||||
Prepared: prepared, Inspection: comparisonInspection(prompt, profiles), ComparisonID: "comparison_daily", DebugWriter: debugWriter, Executor: executor,
|
||||
})
|
||||
}()
|
||||
waitForProfileStarts(t, executor, profiles)
|
||||
waitForProfileStarts(t, executor, profiles, results)
|
||||
for _, profile := range profiles {
|
||||
executor.release(profile.ProfileID)
|
||||
}
|
||||
@@ -142,13 +142,14 @@ func TestExecuteComparisonProfilesUsesDistinctDeterministicDebugReferences(t *te
|
||||
}
|
||||
|
||||
type barrierExecutor struct {
|
||||
mu sync.Mutex
|
||||
started chan string
|
||||
releases map[string]chan struct{}
|
||||
requests map[string]promptexec.ExecuteRequest
|
||||
errors map[string]error
|
||||
inFlight int
|
||||
maximum int
|
||||
mu sync.Mutex
|
||||
started chan string
|
||||
callbackFailures chan error
|
||||
releases map[string]chan struct{}
|
||||
requests map[string]promptexec.ExecuteRequest
|
||||
errors map[string]error
|
||||
inFlight int
|
||||
maximum int
|
||||
}
|
||||
|
||||
func newBarrierExecutor(profiles []ComparisonProfileInspection) *barrierExecutor {
|
||||
@@ -157,7 +158,7 @@ func newBarrierExecutor(profiles []ComparisonProfileInspection) *barrierExecutor
|
||||
releases[profile.ProfileID] = make(chan struct{})
|
||||
}
|
||||
return &barrierExecutor{
|
||||
started: make(chan string, len(profiles)), releases: releases,
|
||||
started: make(chan string, len(profiles)), callbackFailures: make(chan error, len(profiles)), releases: releases,
|
||||
requests: make(map[string]promptexec.ExecuteRequest, len(profiles)), errors: map[string]error{},
|
||||
}
|
||||
}
|
||||
@@ -173,6 +174,7 @@ func (e *barrierExecutor) InspectProfile(context.Context, string) (promptexec.Pr
|
||||
func (e *barrierExecutor) Execute(ctx context.Context, req promptexec.ExecuteRequest, callback promptexec.PreparationCallback) (*promptexec.Execution, error) {
|
||||
stamp := time.Date(2026, 5, 29, 15, 0, 0, 0, time.UTC)
|
||||
if err := callback(promptexec.Preparation{PromptID: req.PromptID, PromptVersion: req.PromptVersion, PromptHash: "prompt-hash", ProfileID: req.ProfileID, BackendID: "backend-" + req.ProfileID, ModelName: "model-" + req.ProfileID, StartedAt: stamp, EndedAt: stamp}, nil); err != nil {
|
||||
e.callbackFailures <- err
|
||||
return nil, err
|
||||
}
|
||||
e.mu.Lock()
|
||||
@@ -224,6 +226,16 @@ func (e *barrierExecutor) release(profileID string) {
|
||||
close(e.releases[profileID])
|
||||
}
|
||||
|
||||
func (e *barrierExecutor) releaseAll() {
|
||||
for _, release := range e.releases {
|
||||
select {
|
||||
case <-release:
|
||||
default:
|
||||
close(release)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
func (e *barrierExecutor) maximumInFlight() int {
|
||||
e.mu.Lock()
|
||||
defer e.mu.Unlock()
|
||||
@@ -236,11 +248,28 @@ func (e *barrierExecutor) inFlightCount() int {
|
||||
return e.inFlight
|
||||
}
|
||||
|
||||
func waitForProfileStarts(t *testing.T, executor *barrierExecutor, profiles []ComparisonProfileInspection) {
|
||||
func waitForProfileStarts(t *testing.T, executor *barrierExecutor, profiles []ComparisonProfileInspection, results <-chan comparisonExecutionResult) {
|
||||
t.Helper()
|
||||
timeout := time.NewTimer(5 * time.Second)
|
||||
defer timeout.Stop()
|
||||
seen := map[string]struct{}{}
|
||||
for range profiles {
|
||||
profileID := <-executor.started
|
||||
var profileID string
|
||||
select {
|
||||
case profileID = <-executor.started:
|
||||
case err := <-executor.callbackFailures:
|
||||
executor.releaseAll()
|
||||
select {
|
||||
case result := <-results:
|
||||
t.Fatalf("comparison profile preparation failed before executor entry: %v; result: %#v", err, result)
|
||||
case <-timeout.C:
|
||||
t.Fatalf("comparison profile preparation failed before executor entry: %v; comparison did not finish", err)
|
||||
}
|
||||
case result := <-results:
|
||||
t.Fatalf("comparison completed before all profiles started: %#v", result)
|
||||
case <-timeout.C:
|
||||
t.Fatal("timed out waiting for comparison profile starts")
|
||||
}
|
||||
if _, duplicate := seen[profileID]; duplicate {
|
||||
t.Fatalf("duplicate execution start for %q", profileID)
|
||||
}
|
||||
|
||||
@@ -269,7 +269,23 @@ func ensureSecureDirectory(path string) error {
|
||||
info, err := os.Lstat(current)
|
||||
if os.IsNotExist(err) {
|
||||
if err := os.Mkdir(current, debugDirectoryMode); err != nil {
|
||||
return err
|
||||
if !os.IsExist(err) {
|
||||
return err
|
||||
}
|
||||
info, err = os.Lstat(current)
|
||||
if err != nil {
|
||||
return err
|
||||
}
|
||||
if info.Mode()&os.ModeSymlink != 0 {
|
||||
return fmt.Errorf("directory component %q must not be a symlink", current)
|
||||
}
|
||||
if !info.IsDir() {
|
||||
return fmt.Errorf("directory component %q is not a directory", current)
|
||||
}
|
||||
if err := os.Chmod(current, debugDirectoryMode); err != nil {
|
||||
return err
|
||||
}
|
||||
continue
|
||||
}
|
||||
if err := os.Chmod(current, debugDirectoryMode); err != nil {
|
||||
return err
|
||||
|
||||
@@ -1,10 +1,12 @@
|
||||
package promptdebug
|
||||
|
||||
import (
|
||||
"fmt"
|
||||
"os"
|
||||
"path/filepath"
|
||||
"runtime"
|
||||
"strings"
|
||||
"sync"
|
||||
"testing"
|
||||
"time"
|
||||
|
||||
@@ -91,6 +93,73 @@ func TestPromptDebugWriterAtomicallyReplacesArtifacts(t *testing.T) {
|
||||
}
|
||||
}
|
||||
|
||||
func TestPromptDebugWriterCreatesSharedMissingAncestorsConcurrently(t *testing.T) {
|
||||
root := filepath.Join(t.TempDir(), "debug")
|
||||
writer, err := NewPromptDebugWriter(root)
|
||||
if err != nil {
|
||||
t.Fatalf("NewPromptDebugWriter() error = %v", err)
|
||||
}
|
||||
|
||||
const writerCount = 8
|
||||
start := make(chan struct{})
|
||||
type writeResult struct {
|
||||
directory string
|
||||
err error
|
||||
}
|
||||
results := make(chan writeResult, writerCount)
|
||||
var writers sync.WaitGroup
|
||||
for index := 0; index < writerCount; index++ {
|
||||
writers.Add(1)
|
||||
go func(index int) {
|
||||
defer writers.Done()
|
||||
<-start
|
||||
directory, err := writer.WritePreparation(PromptDebugRef{
|
||||
ReportID: report.Daily, ValidDate: "2026-05-29", RunID: fmt.Sprintf("run-%02d", index),
|
||||
}, promptDebugPreparationFixture(), nil)
|
||||
results <- writeResult{directory: directory, err: err}
|
||||
}(index)
|
||||
}
|
||||
close(start)
|
||||
|
||||
finished := make(chan struct{})
|
||||
go func() {
|
||||
writers.Wait()
|
||||
close(finished)
|
||||
}()
|
||||
select {
|
||||
case <-finished:
|
||||
case <-time.After(5 * time.Second):
|
||||
t.Fatal("concurrent prompt debug writes did not finish")
|
||||
}
|
||||
close(results)
|
||||
|
||||
directories := map[string]struct{}{}
|
||||
for result := range results {
|
||||
if result.err != nil {
|
||||
t.Fatalf("WritePreparation() error = %v", result.err)
|
||||
}
|
||||
if _, duplicate := directories[result.directory]; duplicate {
|
||||
t.Fatalf("duplicate debug directory %q", result.directory)
|
||||
}
|
||||
directories[result.directory] = struct{}{}
|
||||
if _, err := os.Stat(filepath.Join(result.directory, "preparation.json")); err != nil {
|
||||
t.Fatalf("preparation artifact %q: %v", result.directory, err)
|
||||
}
|
||||
}
|
||||
if len(directories) != writerCount {
|
||||
t.Fatalf("debug directories = %#v", directories)
|
||||
}
|
||||
if runtime.GOOS != "windows" {
|
||||
assertPromptDebugMode(t, root, debugDirectoryMode)
|
||||
assertPromptDebugMode(t, filepath.Join(root, "daily"), debugDirectoryMode)
|
||||
assertPromptDebugMode(t, filepath.Join(root, "daily", "2026-05-29"), debugDirectoryMode)
|
||||
for directory := range directories {
|
||||
assertPromptDebugMode(t, directory, debugDirectoryMode)
|
||||
assertPromptDebugMode(t, filepath.Join(directory, "preparation.json"), debugFileMode)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
func TestPromptDebugWriterDisabledDoesNotAccessFilesystem(t *testing.T) {
|
||||
writer, err := NewPromptDebugWriter("")
|
||||
if err != nil {
|
||||
|
||||
Reference in New Issue
Block a user