diff --git a/.abcd/work/issues/open/iss-2611060835372141-testasilentagentisahangwithrecovery-sets.md b/.abcd/work/issues/open/iss-2611060835372141-testasilentagentisahangwithrecovery-sets.md deleted file mode 100644 index 547056ea3d..0000000000 --- a/.abcd/work/issues/open/iss-2611060835372141-testasilentagentisahangwithrecovery-sets.md +++ /dev/null @@ -1,15 +0,0 @@ ---- -schema_version: 1 -id: "iss-2611060835372141" -slug: "testasilentagentisahangwithrecovery-sets" -severity: "minor" -category: "tech-debt" -source: "agent-observation" -found_during: "v0.16.2 cut" -origin: researcher-authored -production_mode: hand-written -found_at: "internal/core/implement/loop/terminal_longrun_test.go" -remedy: "Apply the short limit to the hanging launch alone." ---- - -TestASilentAgentIsAHangWithRecovery sets a 5-second runner limit that also covers the ordinary launches after the hang (`terminal_longrun_test.go:790`), so a loaded machine could kill one of those and fail the check for exactly one suspect file. It has not been seen failing; a failure would cost a queue entry. diff --git a/.abcd/work/issues/resolved/iss-2611060835372141-testasilentagentisahangwithrecovery-sets.md b/.abcd/work/issues/resolved/iss-2611060835372141-testasilentagentisahangwithrecovery-sets.md new file mode 100644 index 0000000000..f517424e85 --- /dev/null +++ b/.abcd/work/issues/resolved/iss-2611060835372141-testasilentagentisahangwithrecovery-sets.md @@ -0,0 +1,23 @@ +--- +schema_version: 1 +id: "iss-2611060835372141" +slug: "testasilentagentisahangwithrecovery-sets" +severity: "minor" +category: "tech-debt" +source: "agent-observation" +found_during: "v0.16.2 cut" +origin: researcher-authored +production_mode: hand-written +found_at: "internal/core/implement/loop/terminal_longrun_test.go" +remedy: "Apply the short limit to the hanging launch alone." +resolution: "TestASilentAgentIsAHangWithRecovery now drives in two runs: the first, under the 200ms hang limit, ends at its bound once the hang has taken its script line; the second resumes under the ordinary 5-minute limit and lands the lane. A test-local transcript store asserts every finished launch ran under the ordinary limit." +impact: internal +resolved_by: + commit: "a1def8d883b458608a485bf0f145942a778934d5" +--- + +TestASilentAgentIsAHangWithRecovery sets a 5-second runner limit that also covers the ordinary launches after the hang (`terminal_longrun_test.go:790`), so a loaded machine could kill one of those and fail the check for exactly one suspect file. It has not been seen failing; a failure would cost a queue entry. + +## Grounds + +- pursued: no launch that ends by itself runs under the short limit any more, so a loaded machine cannot fail the check by killing one; the detector failing again (a finished launch recorded under a limit other than ordinaryLaunchLimit), or the test taking minutes because a hang reached the second run, would show it wrong diff --git a/internal/core/implement/loop/terminal_longrun_test.go b/internal/core/implement/loop/terminal_longrun_test.go index 101e9c2131..e540143c3c 100644 --- a/internal/core/implement/loop/terminal_longrun_test.go +++ b/internal/core/implement/loop/terminal_longrun_test.go @@ -802,10 +802,9 @@ func TestASilentAgentIsAHangWithRecovery(t *testing.T) { } } } - o := w.bounded(t, 30*24*time.Hour) - o.Runners.Timeout = 5 * time.Second + limits := &launchLimits{} var worktree string - o.BeforeStage = func() error { + seeWorktree := func() error { if st, _, err := readDrain(w.repo.Root()); err == nil && len(st.Lanes) > 0 { if run, err := ReadState(w.repo.Root(), st.Lanes[0].RunID); err == nil && run.Lanes[0].Worktree != "" { worktree = run.Lanes[0].Worktree @@ -813,10 +812,41 @@ func TestASilentAgentIsAHangWithRecovery(t *testing.T) { } return nil } + // The short limit bears on the hanging launch alone (iss-2611060835372141): the + // first run, under it, ends at its bound once the hang has taken its line of the + // script, and the run that takes the lane up again launches under the ordinary + // limit, which a loaded machine cannot reach. The script's own count says the hang + // took its line: a hang killed before it did is launched again under the short + // limit, as often as it takes, and never reaches the second run. Each such kill + // costs the first run a wait, so its bound is one no number of them a machine + // produces can use up: only the count ends it. + turns := func() string { + raw, _ := os.ReadFile(filepath.Join(w.env.log, "script.n")) + return strings.TrimSpace(string(raw)) + } + o := w.bounded(t, 30*24*time.Hour) + o.Runners.Timeout, o.Drain.JudgeTimeout = hangLimit, ordinaryLaunchLimit + o.Runners.Transcripts, o.Drain.Transcripts, o.BeforeStage = limits, limits, seeWorktree + w.onSleep = func(time.Duration) { + if turns() == "2" { + w.clock.t = o.Until + } + } res, err := RunTerminal(context.Background(), w.repo.Root(), o) if err != nil { t.Fatal(err) } + if res.Ended != TerminalUntil || turns() != "2" { + t.Fatalf("the first run ends at its bound after the hang: %+v, %s turns taken\n%s", res, turns(), w.said()) + } + w.onSleep = nil + o = w.bounded(t, 30*24*time.Hour) + o.Runners.Timeout, o.Drain.JudgeTimeout = ordinaryLaunchLimit, ordinaryLaunchLimit + o.Runners.Transcripts, o.Drain.Transcripts, o.BeforeStage = limits, limits, seeWorktree + res, err = RunTerminal(context.Background(), w.repo.Root(), o) + if err != nil { + t.Fatal(err) + } if res.Ended != DrainStoppedCap || len(lanesDone(t, w.repo)) != 1 { t.Fatalf("the lane lands on the retry: %+v\n%s", res, w.said()) } @@ -834,6 +864,20 @@ func TestASilentAgentIsAHangWithRecovery(t *testing.T) { if first["exit_code"] != float64(-1) || first["silent_for_seconds"] == nil { t.Fatalf("the suspect says what was seen: %+v", first) } + // The hang, killed, leaves no transcript; every launch that finished ran under + // the ordinary limit. + if len(limits.got) == 0 || slices.ContainsFunc(limits.got, func(d time.Duration) bool { return d != ordinaryLaunchLimit }) { + t.Fatalf("the short limit bears on the hanging launch alone: the launches that finished ran under %v", limits.got) + } +} + +// launchLimits is a transcript store that keeps the limit each launch it is handed, +// one that finished, ran under. +type launchLimits struct{ got []time.Duration } + +func (l *launchLimits) Store(_ string, req runner.Request, _ runner.Answer, _ []byte) error { + l.got = append(l.got, req.Timeout) + return nil } // TestEvidenceHoldsNoSecretOrPath: what the harness wrote goes through the