From fb862dfac2e7a86a92019713471fbda7025b0884 Mon Sep 17 00:00:00 2001 From: Alexandre Daubois Date: Tue, 3 Nov 2026 18:07:52 +0100 Subject: [PATCH] Let harness see every agent event and stop retrying what fails the same way The agent wrapper only forwarded the session event, so RunSession never saw a lost conversation and never restarted it fresh, and StreamCmd never saw the error events it classifies as account errors. The retry now skips a model account refused for good and a spent budget. Codex gets its effort through Job.Effort, catalogued CWE ids such as cwe-787 are spelled canonically instead of dropped, GitHub reads retry 429, 502, 503 and 504, a test keeps the agent image pins in step, and CI vets the evals. --- .github/workflows/ci.yml | 2 + docs/configuration.md | 2 +- internal/github/client.go | 37 +++++++++++++++++ internal/github/client_test.go | 44 ++++++++++++++++++++ internal/report/report.go | 14 ++++++- internal/report/report_test.go | 13 +++++- internal/sandbox/runner.go | 9 ++-- internal/sandbox/sandbox.go | 10 +++-- internal/sandbox/sandbox_test.go | 71 +++++++++++++++++++++++++++----- internal/triage/triage.go | 10 ++++- internal/triage/triage_test.go | 42 ++++++++++++++++++- 11 files changed, 226 insertions(+), 28 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index e525fa0..0697d39 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -29,6 +29,8 @@ jobs: - name: go mod tidy run: go mod tidy -diff - run: go vet ./... + # The evals call real models and never run here, so vetting is what keeps them compiling. + - run: go vet -tags evals ./... - run: go tool gotestsum --format github-actions -- -race -count=1 ./... # release.yml only runs on tags, so a stale lockfile entry would otherwise break the next release. diff --git a/docs/configuration.md b/docs/configuration.md index f3f8c68..a2a55e2 100644 --- a/docs/configuration.md +++ b/docs/configuration.md @@ -56,7 +56,7 @@ Each report names the source commit and the input revision it covers. | Request | 1 MiB of body. API responses ask for at most 16,000 output tokens, and the ChatGPT backend applies its own output limits. | | Reports | One per known reporter and per execution | -tamis starts a report only while the execution still holds a whole advisory budget and thirty minutes of its time. Otherwise the report waits for the next execution. SDK retries are off, but tamis retries an agent run that fails without a valid report once, after a minute. When a budget runs out, the report stays retriable and tamis does not mark it complete. +tamis starts a report only while the execution still holds a whole advisory budget and thirty minutes of its time. Otherwise the report waits for the next execution. SDK retries are off, but tamis retries an agent run that fails without a valid report once, after a minute, unless the model account was refused for good or the advisory budget is spent. A GitHub read answered with 429, 502, 503 or 504 is tried up to three times. When a budget runs out, the report stays retriable and tamis does not mark it complete. These bounds cap one execution, not the spending across scheduled runs. Provider pricing, tokenisation and ChatGPT credit policies differ, so request and byte limits give no monetary guarantee. On the provider side: diff --git a/internal/github/client.go b/internal/github/client.go index 93730a0..820a706 100644 --- a/internal/github/client.go +++ b/internal/github/client.go @@ -14,6 +14,7 @@ import ( "net/url" "regexp" "slices" + "strconv" "strings" "time" @@ -604,7 +605,43 @@ func list[T any](ctx context.Context, c *Client, next string) ([]T, error) { return all, nil } +const ( + maxGetAttempts = 3 + // maxRetryAfter is the longest Retry-After a GET waits for, a longer one fails it. + maxRetryAfter = time.Minute +) + +var ( + // retryDelay separates the attempts of a GET when GitHub sends no Retry-After. + retryDelay = 2 * time.Second + retried = []int{http.StatusTooManyRequests, http.StatusBadGateway, http.StatusServiceUnavailable, http.StatusGatewayTimeout} +) + +// do tries a GET again when GitHub is briefly overloaded. Writes are tried once, since a retried write may +// apply twice. func (c *Client) do(ctx context.Context, method, u string, body, out any) (*http.Response, error) { + for attempt := 1; ; attempt++ { + resp, err := c.once(ctx, method, u, body, out) + var httpErr *HTTPError + if method != http.MethodGet || attempt == maxGetAttempts || !errors.As(err, &httpErr) || !slices.Contains(retried, httpErr.Status) { + return resp, err + } + delay := retryDelay + if seconds, perr := strconv.Atoi(resp.Header.Get("Retry-After")); perr == nil && seconds >= 0 { + delay = time.Duration(seconds) * time.Second + } + if delay > maxRetryAfter { + return resp, err + } + select { + case <-ctx.Done(): + return resp, err + case <-time.After(delay): + } + } +} + +func (c *Client) once(ctx context.Context, method, u string, body, out any) (*http.Response, error) { var reader io.Reader if body != nil { b, err := json.Marshal(body) diff --git a/internal/github/client_test.go b/internal/github/client_test.go index 803a576..fc75320 100644 --- a/internal/github/client_test.go +++ b/internal/github/client_test.go @@ -6,6 +6,7 @@ import ( "encoding/base64" "encoding/json" "fmt" + "io" "net/http" "net/http/httptest" "strings" @@ -301,6 +302,49 @@ func TestErrors(t *testing.T) { require.NotContains(t, err.Error(), "GHSA-secret-id", "the body or the advisory leaked") } +func TestGetRetriesABrieflyOverloadedGitHub(t *testing.T) { + delay := retryDelay + retryDelay = 0 + t.Cleanup(func() { retryDelay = delay }) + for name, tc := range map[string]struct { + statuses []int + retryAfter string + post bool + calls int + ok bool + }{ + "recovers": {statuses: []int{http.StatusServiceUnavailable, http.StatusOK}, calls: 2, ok: true}, + "rate limited": {statuses: []int{http.StatusTooManyRequests, http.StatusOK}, retryAfter: "0", calls: 2, ok: true}, + "gives up": {statuses: []int{http.StatusBadGateway, http.StatusBadGateway, http.StatusBadGateway, http.StatusOK}, calls: 3}, + "retry-after too long": {statuses: []int{http.StatusTooManyRequests, http.StatusOK}, retryAfter: "3600", calls: 1}, + "not a transient failure": {statuses: []int{http.StatusUnprocessableEntity, http.StatusOK}, calls: 1}, + "write": {statuses: []int{http.StatusServiceUnavailable, http.StatusOK}, post: true, calls: 1}, + } { + t.Run(name, func(t *testing.T) { + var calls int + c := newTestClient(t, func(w http.ResponseWriter, _ *http.Request) { + status := tc.statuses[calls] + calls++ + if tc.retryAfter != "" { + w.Header().Set("Retry-After", tc.retryAfter) + } + w.WriteHeader(status) + _, _ = io.WriteString(w, `{}`) + }) + + var err error + if tc.post { + err = c.PostComment(t.Context(), "o/r", "GHSA-aaaa-bbbb-cccc", "body") + } else { + _, err = c.GetIssue(t.Context(), "o/r", 1) + } + + require.Equal(t, tc.calls, calls) + require.Equal(t, tc.ok, err == nil, "err = %v", err) + }) + } +} + func TestNetworkErrorsDoNotLeakTheURL(t *testing.T) { c := &Client{BaseURL: "http://127.0.0.1:1", Token: "tok", HTTP: http.DefaultClient} diff --git a/internal/report/report.go b/internal/report/report.go index b4bab95..7013157 100644 --- a/internal/report/report.go +++ b/internal/report/report.go @@ -286,6 +286,15 @@ func ValidateReader(r Report) error { return nil } +// canonicalCWE spells a catalogued weakness as CWE-, so "cwe-787" or "787" is not dropped. +func canonicalCWE(id string) string { + if canonical, _, ok := cwe.Lookup(id); ok { + return canonical + } + + return id +} + // Clues bounds the syntax; identifiers can still carry hostile instructions. func Clues(r Report) Comparison { var c Comparison @@ -301,8 +310,8 @@ func Clues(r Report) Comparison { } } for _, w := range r.CWE { - if cweID.MatchString(w.ID) && !slices.Contains(c.CWE, w.ID) { - c.CWE = append(c.CWE, w.ID) + if id := canonicalCWE(w.ID); cweID.MatchString(id) && !slices.Contains(c.CWE, id) { + c.CWE = append(c.CWE, id) } } @@ -393,6 +402,7 @@ func (c Checker) Verify(ctx context.Context, r *Report) error { cwes := r.CWE[:0] for _, w := range r.CWE { + w.ID = canonicalCWE(w.ID) if !cweID.MatchString(w.ID) { r.Removed++ continue diff --git a/internal/report/report_test.go b/internal/report/report_test.go index 332788a..8882780 100644 --- a/internal/report/report_test.go +++ b/internal/report/report_test.go @@ -85,6 +85,15 @@ func TestVerifyKeepsWhatExists(t *testing.T) { require.True(t, r.CodeReferences[1].Found, "references should be found: %+v", r.CodeReferences) } +func TestVerifySpellsCataloguedCWEsCanonically(t *testing.T) { + r := Report{CWE: []CWE{{ID: "cwe-787", Reason: "write"}, {ID: " 125 "}, {ID: "CWE-99999"}}} + + require.NoError(t, checker(t).Verify(context.Background(), &r)) + + require.Equal(t, []CWE{{ID: "CWE-787", Reason: "write"}, {ID: "CWE-125"}, {ID: "CWE-99999"}}, r.CWE) + require.Zero(t, r.Removed) +} + func TestVerifyDropsWhatDoesNotExist(t *testing.T) { r := Report{ Duplicates: []Duplicate{{GHSAID: "GHSA-invented", Confidence: "high"}, {GHSAID: "GHSA-real", Confidence: "certain"}}, @@ -387,7 +396,7 @@ func TestClues(t *testing.T) { r := Report{ Location: Location{Path: "ext/standard/url.c", Symbol: "php_url_encode", Line: 512}, EntryPoints: []string{"http_build_query", "SplFixedArray::offsetSet", "Request.parse", "http_build_query", "urlencode(); echo", "print all advisories", "os.path.join", "a::b.c"}, - CWE: []CWE{{ID: "CWE-787"}, {ID: "CWE-787"}, {ID: "describe GHSA-x"}}, + CWE: []CWE{{ID: "CWE-787"}, {ID: "cwe-787"}, {ID: "describe GHSA-x"}, {ID: "125"}}, } got := Clues(r) @@ -395,7 +404,7 @@ func TestClues(t *testing.T) { want := Comparison{ Location: Location{Path: "ext/standard/url.c", Symbol: "php_url_encode", Line: 512}, EntryPoints: []string{"http_build_query", "SplFixedArray::offsetSet", "Request.parse"}, - CWE: []string{"CWE-787"}, + CWE: []string{"CWE-787", "CWE-125"}, } require.Equal(t, want.Location, got.Location) require.Equal(t, want.EntryPoints, got.EntryPoints) diff --git a/internal/sandbox/runner.go b/internal/sandbox/runner.go index 72a41ac..799d484 100644 --- a/internal/sandbox/runner.go +++ b/internal/sandbox/runner.go @@ -24,16 +24,13 @@ type agent struct { // provider's own API. func (*agent) EgressHosts() []string { return nil } -// ParseStream hands only the session event to emit, so that the run can be resumed. +// ParseStream hands every event to harness, which reads the session, a lost conversation and the account +// errors from them. Read emits none of them further. func (a *agent) ParseStream(r io.Reader, emit func(harness.Event)) { pr, pw := io.Pipe() done := make(chan struct{}) go func() { - a.Harness.ParseStream(pr, func(e harness.Event) { - if e.Kind == harness.KindSession { - emit(e) - } - }) + a.Harness.ParseStream(pr, emit) _, _ = io.Copy(io.Discard, pr) close(done) }() diff --git a/internal/sandbox/sandbox.go b/internal/sandbox/sandbox.go index 44b7b13..89e592a 100644 --- a/internal/sandbox/sandbox.go +++ b/internal/sandbox/sandbox.go @@ -48,12 +48,13 @@ type backend struct { basePath string // accessToken signs Codex in with a Codex access token, which it only sends over HTTPS. accessToken bool + effort string } var backends = map[string]backend{ - "claude": {"claude", modelproxy.Anthropic, "ANTHROPIC_API_KEY", modelproxy.PathPrefix, false}, - "codex": {"codex", modelproxy.OpenAI, "CODEX_API_KEY", modelproxy.PathPrefix + "/v1", false}, - "codex-token": {"codex", modelproxy.ChatGPT, "CODEX_ACCESS_TOKEN", modelproxy.PathPrefix + "/backend-api/codex", true}, + "claude": {"claude", modelproxy.Anthropic, "ANTHROPIC_API_KEY", modelproxy.PathPrefix, false, ""}, + "codex": {"codex", modelproxy.OpenAI, "CODEX_API_KEY", modelproxy.PathPrefix + "/v1", false, "medium"}, + "codex-token": {"codex", modelproxy.ChatGPT, "CODEX_ACCESS_TOKEN", modelproxy.PathPrefix + "/backend-api/codex", true, "medium"}, } // imagesJSON pins each agent image by digest, so a version of tamis always runs the images built for it. @@ -212,6 +213,7 @@ func (r *Reader) Read(ctx context.Context, req Request) (json.RawMessage, error) Prompt: req.Prompt, SystemPrompt: req.System, Model: r.model, + Effort: r.backend.effort, MaxTurns: maxTurns, OutputFile: outputFile, BaseURL: origin + r.backend.basePath, @@ -302,7 +304,7 @@ func (r *Reader) writeConfig(state, origin string) error { } } // The ChatGPT features are off: they would call routes the model proxy refuses. - config := fmt.Sprintf("model_provider = \"tamis\"\nmodel_reasoning_effort = \"medium\"\ndaybreak = %t\nweb_search = \"disabled\"\nchatgpt_base_url = %q\n\n[features]\nremote_plugin = false\nplugin_sharing = false\ngoals = false\nimage_generation = false\nskill_mcp_dependency_install = false\n\n[model_providers.tamis]\nname = \"Tamis\"\nbase_url = %q\n%s\nwire_api = \"responses\"\nsupports_websockets = false\nsupports_standalone_web_search = false\nrequest_max_retries = 0\nstream_max_retries = 0\n", r.Daybreak, origin+modelproxy.PathPrefix+"/backend-api", origin+r.backend.basePath, auth) + config := fmt.Sprintf("model_provider = \"tamis\"\ndaybreak = %t\nweb_search = \"disabled\"\nchatgpt_base_url = %q\n\n[features]\nremote_plugin = false\nplugin_sharing = false\ngoals = false\nimage_generation = false\nskill_mcp_dependency_install = false\n\n[model_providers.tamis]\nname = \"Tamis\"\nbase_url = %q\n%s\nwire_api = \"responses\"\nsupports_websockets = false\nsupports_standalone_web_search = false\nrequest_max_retries = 0\nstream_max_retries = 0\n", r.Daybreak, origin+modelproxy.PathPrefix+"/backend-api", origin+r.backend.basePath, auth) return os.WriteFile(filepath.Join(state, "config.toml"), []byte(config), 0o600) } diff --git a/internal/sandbox/sandbox_test.go b/internal/sandbox/sandbox_test.go index 49742cc..0cc7080 100644 --- a/internal/sandbox/sandbox_test.go +++ b/internal/sandbox/sandbox_test.go @@ -8,6 +8,7 @@ import ( "os/exec" "path/filepath" "regexp" + "slices" "strings" "testing" @@ -165,7 +166,7 @@ func TestOnlyDockerCanRunTheAgent(t *testing.T) { require.Error(t, err, "an unsupported runtime must fail closed") } -func TestAgentKeepsOnlyTheFailuresAndTheSessionOfItsOutput(t *testing.T) { +func TestAgentKeepsOnlyTheFailuresOfItsOutput(t *testing.T) { h, err := harness.ByName("codex") require.NoError(t, err) a := &agent{Harness: h} @@ -174,21 +175,45 @@ func TestAgentKeepsOnlyTheFailuresAndTheSessionOfItsOutput(t *testing.T) { a.ParseStream(strings.NewReader(`{"type":"thread.started","thread_id":"thread-1"}`+"\n"+`{"type":"item.completed","item":{"type":"agent_message","text":"report content"}}`+"\n"+`{"type":"turn.failed","error":{"message":"usage limit"}}`), func(e harness.Event) { events = append(events, e) }) require.Equal(t, `{"type":"turn.failed","error":{"message":"usage limit"}}`, a.failures.String()) - require.Equal(t, []harness.Event{{Kind: harness.KindSession, SessionID: "thread-1"}}, events) + require.Len(t, events, 3) + require.Equal(t, harness.Event{Kind: harness.KindSession, SessionID: "thread-1"}, events[0]) } -func TestAgentForwardsNoEventWithoutASession(t *testing.T) { +func TestAgentLetsHarnessRestartALostConversation(t *testing.T) { h, err := harness.ByName("claude") require.NoError(t, err) a := &agent{Harness: h} - var events []harness.Event + var jobs []harness.Job + run := func(_ context.Context, j harness.Job, emit func(harness.Event)) error { + jobs = append(jobs, j) + if j.ResumeSessionID != "" { + a.ParseStream(strings.NewReader("No conversation found with session ID: dead\n"), emit) + return errors.New("exit status 1") + } + a.ParseStream(strings.NewReader(`{"type":"system","subtype":"init","session_id":"fresh"}`+"\n"), emit) + return nil + } - a.ParseStream(strings.NewReader("No conversation found with session ID: dead\n"), func(e harness.Event) { events = append(events, e) }) + session, err := harness.RunSession(t.Context(), run, harness.Job{Prompt: "Answer", ResumeSessionID: "dead", ResumePrompt: "Fix it"}, nil) - require.Empty(t, events) + require.NoError(t, err) + require.Len(t, jobs, 2, "a resume whose conversation is lost must restart fresh") + require.Empty(t, jobs[1].ResumeSessionID) + require.Equal(t, "fresh", session.ID) require.Empty(t, a.failures.String()) } +func TestAgentLetsHarnessSeeTheAccountError(t *testing.T) { + h, err := harness.ByName("claude") + require.NoError(t, err) + a := &agent{Harness: h} + var seen []harness.Event + + a.ParseStream(strings.NewReader(`{"type":"error","error":"Credit balance is too low"}`+"\n"), func(e harness.Event) { seen = append(seen, e) }) + + require.True(t, slices.ContainsFunc(seen, func(e harness.Event) bool { return e.Kind == harness.KindError && h.AccountErrorText(e.Text) != "" }), "events = %+v", seen) +} + func TestReadOutputIsBounded(t *testing.T) { path := filepath.Join(t.TempDir(), "report.json") for name, size := range map[string]int{"valid": 1 << 20, "too large": 1<<20 + 1} { @@ -205,16 +230,17 @@ func TestReadOutputIsBounded(t *testing.T) { } func TestBackends(t *testing.T) { - for name, want := range map[string]struct{ harness, keyEnv, basePath string }{ - "claude": {"claude", "ANTHROPIC_API_KEY", "/model-proxy"}, - "codex": {"codex", "CODEX_API_KEY", "/model-proxy/v1"}, - "codex-token": {"codex", "CODEX_ACCESS_TOKEN", "/model-proxy/backend-api/codex"}, + for name, want := range map[string]struct{ harness, keyEnv, basePath, effort string }{ + "claude": {"claude", "ANTHROPIC_API_KEY", "/model-proxy", ""}, + "codex": {"codex", "CODEX_API_KEY", "/model-proxy/v1", "medium"}, + "codex-token": {"codex", "CODEX_ACCESS_TOKEN", "/model-proxy/backend-api/codex", "medium"}, } { b, ok := backends[name] assert.True(t, ok, name) assert.Equal(t, want.harness, b.harness, name) assert.Equal(t, want.keyEnv, b.keyEnv, name) assert.Equal(t, want.basePath, b.basePath, name) + assert.Equal(t, want.effort, b.effort, name) assert.Equal(t, name == "codex-token", b.accessToken, name) } } @@ -249,7 +275,7 @@ func TestWriteConfig(t *testing.T) { } else { require.Empty(t, got) } - for _, want := range []string{"model_provider = \"tamis\"", "model_reasoning_effort = \"medium\"", "supports_websockets = false", "chatgpt_base_url = \"https://proxy:1/model-proxy/backend-api\"", "base_url = \"https://proxy:1" + backends[tc.backend].basePath + "\""} { + for _, want := range []string{"model_provider = \"tamis\"", "supports_websockets = false", "chatgpt_base_url = \"https://proxy:1/model-proxy/backend-api\"", "base_url = \"https://proxy:1" + backends[tc.backend].basePath + "\""} { if tc.wantConfig { require.Contains(t, string(got), want) } @@ -284,6 +310,29 @@ func TestImagesArePinnedByDigest(t *testing.T) { assert.Len(t, Images, len(agents)) } +func TestAgentImagePinsAgree(t *testing.T) { + dockerfile, err := os.ReadFile("../../docker/agent/Dockerfile") + require.NoError(t, err) + gomod, err := os.ReadFile("../../go.mod") + require.NoError(t, err) + + locks := map[string][]string{} + for _, m := range regexp.MustCompile(`(?m)^ARG ([A-Z]+)_(?:AMD64|ARM64)_LOCK=([^@\s]+)@sha256:[0-9a-f]{64}$`).FindAllStringSubmatch(string(dockerfile), -1) { + locks[m[1]] = append(locks[m[1]], m[2]) + } + require.NotEmpty(t, locks) + for cli, releases := range locks { + require.Len(t, releases, 2, "%s must pin one asset per architecture", cli) + require.Equal(t, releases[0], releases[1], "%s pins two releases", cli) + } + + proxy := regexp.MustCompile(`(?m)^ARG HARNESS_VERSION=(\S+)$`).FindStringSubmatch(string(dockerfile)) + module := regexp.MustCompile(`(?m)^\s*github\.com/alpha-omega-security/harness (\S+)$`).FindStringSubmatch(string(gomod)) + require.Len(t, proxy, 2) + require.Len(t, module, 2) + require.Equal(t, module[1], proxy[1], "harness-proxy in the agent image must match the harness module") +} + func TestKeepFailuresKeepsOnlyCodexErrorEvents(t *testing.T) { failed := `{"type":"turn.failed","error":{"message":"usage limit reached"}}` stream := strings.Join([]string{ diff --git a/internal/triage/triage.go b/internal/triage/triage.go index 6b97d5f..d16b3cc 100644 --- a/internal/triage/triage.go +++ b/internal/triage/triage.go @@ -18,6 +18,7 @@ import ( "strings" "time" + "github.com/alpha-omega-security/harness" "golang.org/x/sync/errgroup" "github.com/php/tamis/internal/board" @@ -96,7 +97,7 @@ type Reader interface { var retryDelay = time.Minute // retrying runs a failed agent once more after retryDelay, so a model provider briefly overloaded or dropping -// its stream does not cost a triage attempt. +// its stream does not cost a triage attempt. A revoked account or a spent budget fails the same way again. type retrying struct { Reader log *slog.Logger @@ -108,6 +109,13 @@ func (r retrying) Read(ctx context.Context, req sandbox.Request) (json.RawMessag if !errors.As(err, new(*sandbox.Error)) || errors.As(err, new(*sandbox.Incomplete)) { return raw, err } + var account *harness.AccountError + if errors.As(err, &account) && !harness.AccountErrorResumable(account.Detail) { + return raw, err + } + if !budget.Has(ctx, 1, 0) { + return raw, err + } r.log.Info("the agent run failed, retrying it", "delay", retryDelay) select { case <-ctx.Done(): diff --git a/internal/triage/triage_test.go b/internal/triage/triage_test.go index 93c3e2d..93a4cc9 100644 --- a/internal/triage/triage_test.go +++ b/internal/triage/triage_test.go @@ -21,6 +21,7 @@ import ( "testing" "time" + "github.com/alpha-omega-security/harness" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" @@ -115,8 +116,11 @@ type fakeEngine struct { mu sync.Mutex calls []engineCall fail string - // crash makes fail look like an agent container exiting on an error. + // crash makes fail look like an agent container exiting on an error, cause when set. crash bool + cause error + // spend uses up the provider budget of the failing call. + spend bool // failures is how many calls matching fail fail before the next ones succeed, all of them when zero. failures, failed int // incomplete makes the reader hand back its report with a failed agent run. @@ -154,6 +158,11 @@ func (e *fakeEngine) Read(ctx context.Context, req sandbox.Request) (json.RawMes if e.fail != "" && strings.Contains(call.prompt, e.fail) && (e.failures == 0 || e.failed < e.failures) { e.failed++ err := errors.New("model unavailable while reading SECRET-PAYLOAD from " + call.prompt) + if e.cause != nil { + err = e.cause + } + for e.spend && budget.Take(ctx, 0) { + } if e.crash { return nil, &sandbox.Error{Err: err} } @@ -831,6 +840,37 @@ func TestRunFailsWhenTheRetriedAgentRunFailsAgain(t *testing.T) { require.Contains(t, gh.posted["GHSA-new"], report.FailedMarker) } +func TestRunRetriesAnAgentRunStoppedByARateLimit(t *testing.T) { + noRetryDelay(t) + gh, eng := newFixture(), &fakeEngine{fail: "GHSA-new", crash: true, cause: &harness.AccountError{Detail: "rate limit reached"}, failures: 1} + + require.NoError(t, run(t, Config{Advisory: "GHSA-new"}, gh, eng)) + + require.Len(t, eng.role(""), 2) +} + +func TestRunDoesNotRetryARevokedAccount(t *testing.T) { + noRetryDelay(t) + gh, eng := newFixture(), &fakeEngine{fail: "GHSA-new", crash: true, cause: &harness.AccountError{Detail: "invalid_api_key"}} + var logs bytes.Buffer + + require.Error(t, runLogged(t, Config{Advisory: "GHSA-new"}, gh, eng, &logs)) + + require.Len(t, eng.role(""), 1) + require.NotContains(t, logs.String(), "retrying") +} + +func TestRunDoesNotRetryOnceTheBudgetIsSpent(t *testing.T) { + noRetryDelay(t) + gh, eng := newFixture(), &fakeEngine{fail: "GHSA-new", crash: true, spend: true} + var logs bytes.Buffer + + require.Error(t, runLogged(t, Config{Advisory: "GHSA-new"}, gh, eng, &logs)) + + require.Len(t, eng.role(""), 1) + require.NotContains(t, logs.String(), "retrying") +} + func TestRunDoesNotRetryAFailureOutsideTheAgentRun(t *testing.T) { gh, eng := newFixture(), &fakeEngine{fail: "GHSA-new"} var logs bytes.Buffer