perf(#6060): make CreateWithRetry sleep injectable for tests

[fullsend-ai-coder]

Summary

• Add an exported RetrySleep variable to internal/sandbox/sandbox.go (defaults to time.Sleep) and replace the hardcoded time.Sleep(backoff) call in CreateWithRetry with RetrySleep(backoff)
• Override sandbox.RetrySleep with a no-op in usePreScriptStub (test helper in internal/cli/prescript_run_test.go), so the two tests that exercise the sandbox-creation failure/retry path no longer sleep for ~15s each
• Both tests (TestRunAgent_NoPreScript_StillRelaysSkippedFalse and TestRunAgent_PreScriptNoSkip_ProceedsToSandboxAndRelaysFalse) now complete in < 0.1s

Test plan

• go test ./internal/sandbox/... -race — all sandbox tests pass
• go test ./internal/cli/... -run 'TestRunAgent_PreScript|TestRunAgent_NoPreScript|TestRunPreScript' -race — all pre-script tests pass
• go vet ./internal/sandbox/... ./internal/cli/... — no issues
• The two previously-slow tests complete in < 0.1s each (down from ~15s)

---

Closes #6060

Post-script verification

• Branch is not main/master (agent/6060-sandbox-retry-sleep)
• Secret scan passed (gitleaks — 483ad9bd31407310940d21003a6cfe1b6b52b5c8..HEAD)
• PR body secret scan passed (gitleaks — no-git)

[fullsend-ai-review]

🤖 Finished Review · ✅ Success · Started 7:55 PM UTC · Completed 8:06 PM UTC

Commit: d01a1bf · View workflow run →

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

[fullsend-ai-review]

Looks good to me

Previous run

Review

Findings

Low

• [documentation-style] internal/sandbox/sandbox.go:41 — The RetrySleep variable documentation includes implementation guidance ("restore the original in a cleanup function") that is inconsistent with the established pattern for injectable test functions in this codebase. Similar variables (BuildWASMFn, CopyWASMExecFn, WranglerWhoamiFn in internal/dispatch/cf/provisioner.go) use "Override in tests to..." phrasing without prescribing the restoration mechanism.
  Remediation: Simplify to // RetrySleep is the function called between retry attempts in CreateWithRetry. // It defaults to time.Sleep. Override in tests to avoid real delays.

Previous run (2)

Review

Findings

Low

• [documentation-style] internal/sandbox/sandbox.go:41 — The RetrySleep variable documentation includes implementation guidance ("restore the original in a cleanup function") that is inconsistent with the established pattern for injectable test functions in this codebase. Similar variables (BuildWASMFn, CopyWASMExecFn, WranglerWhoamiFn in internal/dispatch/cf/provisioner.go) use "Override in tests to..." phrasing without prescribing the restoration mechanism.
  Remediation: Simplify to // RetrySleep is the function called between retry attempts in CreateWithRetry. // It defaults to time.Sleep. Override in tests to avoid real delays.

---

Labels: PR modifies sandbox retry infrastructure (internal/sandbox/sandbox.go)

[fullsend-ai-review]

🤖 Finished Review · ✅ Success · Started 8:47 PM UTC · Completed 8:58 PM UTC

Commit: 4ba983c · View workflow run →

[ralphbean]

LGTM. One nit inline.

[ralphbean]

/fs-fix

[fullsend-ai-coder]

🤖 Finished Fix · ✅ Success · Started 10:26 PM UTC · Completed 10:35 PM UTC

Commit: 4ba983c · View workflow run →

[fullsend-ai-coder]

🔧 Fix agent — iteration 1 (human-triggered)

Addressed 2 review findings: renamed RetrySleep to RetrySleepFn to match the codebase XxxFn naming convention (ralphbean nit), and simplified the doc comment to match the established 'Override in tests to...' pattern (bot finding). All tests pass.

Fixed (2):

1. Rename RetrySleep to RetrySleepFn to match XxxFn naming convention (internal/sandbox/sandbox.go): Renamed exported var from RetrySleep to RetrySleepFn to match the established XxxFn suffix convention used by BuildWASMFn, CopyWASMExecFn, and WranglerWhoamiFn in internal/dispatch/cf/provisioner.go. Updated all references in sandbox.go, sandbox_test.go, and prescript_run_test.go.
2. Simplify RetrySleep doc comment to match established pattern (internal/sandbox/sandbox.go): Simplified the doc comment from prescriptive 'restore the original in a cleanup function' to the established 'Override in tests to avoid real delays' pattern used by other injectable test functions.

Tests: passed

Next steps:

• /fs-review — request a re-review of the changes
• /fs-fix <your instruction> — run another fix pass with specific guidance
• Push commits directly — review re-runs automatically on push
  _{Updated by fullsend fix agent}

[fullsend-ai-review]

🤖 Finished Review · ✅ Success · Started 10:37 PM UTC · Completed 10:47 PM UTC

Commit: 972e943 · View workflow run →

[fullsend-ai-retro]

🤖 Finished Retro · ✅ Success · Started 5:38 PM UTC · Completed 5:55 PM UTC

Commit: 972e943 · View workflow run →

[fullsend-ai-retro]

Retro: PR #6066 — make CreateWithRetry sleep injectable for tests

Pipeline: Issue #6060 → triage (4 min) → code (12 min) → review → human review → /fs-fix → re-review → merged after 3 merge-queue attempts.

Timeline

Time (Aug 10 UTC)	Event
19:36	ralphbean files #6060 — CreateWithRetry uses real time.Sleep in tests
19:37–19:41	Triage agent runs (31424936623), recommends injectable sleep, notes createOnce has same issue
19:41–19:53	Code agent runs (31425339448), creates PR with RetrySleep var
19:53–20:06	Review agent runs (31426356439), approves with 1 low finding (doc comment style)
22:25	ralphbean approves with nit: RetrySleep should be RetrySleepFn to match BuildWASMFn, CopyWASMExecFn, etc.
22:25–22:36	Fix agent runs (31438258130), renames variable and updates doc comment
22:36–22:47	Post-fix review (31438986254), approves clean
Aug 11–13	Two merge-queue ejections from flaky E2E tests; merged on 3rd attempt Aug 13

What went well

• Triage was fast and accurate (4 min), correctly identifying the problem and recommending the right approach.
• Code agent produced a clean, minimal PR — 3 files, 29 additions, 4 deletions, well-focused.
• Review agent correctly dismissed a false positive — the challenger sub-agent removed a race-condition finding about the exported mutable var, recognizing BuildWASMFn et al. as established precedent.
• Fix agent successfully addressed both findings — the review agent's doc-comment finding and the human's naming nit — in a single 10-minute run.
• Total agent time was efficient — ~49 minutes across all stages for a clean, mergeable result.

What the human caught that the agent missed

The review agent identified BuildWASMFn, CopyWASMExecFn, and WranglerWhoamiFn as reference variables and used them to (a) evaluate doc-comment style and (b) dismiss the race-condition false positive. But it never noticed that all three names end in Fn while the new variable was named RetrySleep — the most obvious inconsistency. The agent had the right data and checked one consistency dimension (documentation) but missed another (naming).

The root cause is that the Fn suffix convention is entirely undocumented — not in AGENTS.md, CLAUDE.md, docs/contributing/go-code.md, or any other file. The codebase has 9+ instances with zero violations, but no written rule. Without documentation, the agent must infer the convention purely from code patterns.

Existing-issue evidence (proposals skipped)

• E2E flakiness (#1879, #6129): This PR's two merge-queue ejections (GitHub API timeout in behaviour test scaffold, triage dispatch timeout in admin E2E) add further evidence that flaky E2E tests impose significant delays on unrelated PRs. PR perf(#6059): make LiveGCFClient poll delay injectable for tests #6062 experienced the same failures concurrently.
• Code agent not following all triage recommendations (#2185): The triage agent recommended also addressing createOnce's hardcoded time.Sleep(readyPoll) on line 606, but the code agent only implemented the CreateWithRetry fix. Minor instance of the pattern tracked in Code agent should follow triage's recommended implementation strategy, not just its scope #2185.

Autonomy readiness

The review agent's findings fully covered the doc-comment style issue and correctly filtered a false positive — solid performance. The single human finding was a non-blocking naming nit. If the Fn suffix convention were documented in go-code.md (proposal 1 below), the review agent would have an authoritative reference to check against, likely closing this gap. For this class of change (small, well-tested, injectable-variable refactors), the review agent is close to fully autonomous — the remaining gap is convention awareness, which is addressable through documentation.

Proposals filed

• Document Fn suffix convention for injectable function variables in go-code.md (in fullsend-ai/fullsend)
• Style-conventions sub-agent should check naming consistency when comparing against reference declarations (in fullsend-ai/agents)