ci(#6656): reset pool repo git history on allocation - #6665
Conversation
Pool repos (test-repo-01 through test-repo-12) accumulate git history across hundreds of behaviour test runs without cleanup, growing to 1.9 GB. The pre-review shallow-clone deepening step takes 12+ minutes fetching this bloated history, pushing against the dispatch timeout. Add a resetRepo step at the start of doEnsure that deletes the existing repo before ensureRepoExists recreates it. This gives each suite run a fresh single-commit repo. The alreadyInstalled optimization is removed since repos are always freshly created — settle (Actions workflow readiness) now runs unconditionally. Closes #6656
|
🤖 Finished Review · ✅ Success · Started 8:38 PM UTC · Completed 8:55 PM UTC Commit: Runtime: claude · Model: opus → claude-opus-4-6 · Effort: high · Cost: $4.79 |
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
|
/fs-fix fix behavior test failures in CI: https://github.com/fullsend-ai/fullsend/actions/runs/33011272114/job/98317906901?pr=6665 I saw a 404 error there. Is there a problem where we delete the repo and do not wait sufficiently long (with expontential retry and backoff) for it to be recreated and settled in the github API? |
|
Risk Assessment: moderate (2/5) DetailsAnchored to prior score of 2. Tier 1 signals unchanged: 2-file change with 0.50 test ratio, no protected/security/CI/dependency risks, trusted bot author. Tier 2 slightly elevated by fix-commit density (8 in 90 days). Linked issue is fresh and narrowly scoped. Previous runRisk Assessment: moderate (2/5) DetailsAnchored to prior score of 2. Tier 1 signals unchanged: 2-file change with 0.50 test ratio, no protected/security/CI/dependency risks, trusted bot author. Tier 2 elevated by fix-commit density (5-7 in 90 days). Linked issue is fresh and narrowly scoped. Previous run (2)Risk Assessment: moderate (2/5) DetailsSmall, well-tested 2-file change to behaviour-test infrastructure by a trusted bot author. No protected paths, security-sensitive files, CI workflows, or dependency changes. Test ratio is ideal at 0.50. Elevated from low to moderate by high recent churn (11 commits in 30 days with 8 fix/iteration commits in 90 days). Linked issue is fresh, well-understood, and fix is narrowly scoped. |
ReviewFindingsMedium
Low
Previous runReviewFindingsMedium
Low
Labels: PR modifies behaviour test repo allocation infrastructure under pkg/behaviourtest/ Previous run (2)ReviewFindingsMedium
Low
|
![fullsend-ai-review[bot]](https://avatars.githubusercontent.com/in/3478501?s=60&v=4){kind=small}
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
fullsend-ai-review
Bot
added
the
requires-manual-review
|
🤖 Finished Fix · ✅ Success · Started 8:56 PM UTC · Completed 9:12 PM UTC Commit: Runtime: claude · Model: opus → claude-opus-4-6 · Effort: high · Cost: $3.95 |
…nd create After resetRepo deletes a pool repo, the GitHub API may still return the repo due to eventual consistency. Without waiting for propagation, ensureRepoExists can see the stale repo, skip re-creation, and subsequent installFullsend operations fail with 404 on the disappearing repo. Add awaitDeletion (polls GetRepo until 404) and awaitCreation (polls GetRepo until success) with exponential backoff (1s, 2s, 4s, 8s) after delete and create respectively. awaitDeletion is non-fatal if max attempts are exhausted; awaitCreation fails to prevent operating on an unavailable repo. Update stubClient to track delete/create state transitions so tests reflect realistic API behaviour. Add 10 unit tests covering the new await functions (immediate confirmation, retry-until-confirmed, max-attempt exhaustion, error propagation, context cancellation). Addresses review feedback on #6665
🔧 Fix agent — iteration 1 (human-triggered)Added exponential-backoff waits after repo delete and create to handle GitHub API eventual consistency. After resetRepo deletes a pool repo, awaitDeletion polls GetRepo until 404 (up to 5 attempts, ~15s). After ensureRepoExists creates a repo, awaitCreation polls GetRepo until success. This prevents the 404 errors seen in CI where installFullsend operated on a repo that hadn't fully propagated. Fixed (3):
Tests: passedNext steps:
|
|
🤖 Review · Commit: |
|
/fs-review |
|
🤖 Finished Review · ✅ Success · Started 9:16 PM UTC · Completed 9:36 PM UTC Commit: Runtime: claude · Model: opus → claude-opus-4-6 · Effort: high · Cost: $5.93 |
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
|
/fs-fix The behaviour test failures show that resetRepo needs to clean up fork repos too. When test-repo-01 is deleted and recreated, test-repo-01-fork becomes orphaned. The fork creation step then fails because the fork repo exists but isn't a valid fork of the new source. We should delete repos matching the test-repo-*-fork pattern as part of the reset. |
|
fix is failing due to fullsend-ai/agents#1062 |
|
Fix is landing here: fullsend-ai/agents#1063, then we can re-run /fs-fix here as above. |
|
/fs-fix The behaviour test failures show that resetRepo needs to clean up fork repos too. When test-repo-01 is deleted and recreated, test-repo-01-fork becomes orphaned. The fork creation step then fails because the fork repo exists but isn't a valid fork of the new source. We should delete repos matching the test-repo-*-fork pattern as part of the reset. |
|
🤖 Finished Fix · ✅ Success · Started 2:05 AM UTC · Completed 2:17 AM UTC Commit: Runtime: claude · Model: opus → claude-opus-4-6 · Effort: high · Cost: $2.92 |
resetRepo now deletes {repoName}-fork before deleting the source repo.
When a pool repo is deleted and recreated, any existing fork becomes
orphaned — the fork creation step then fails because the fork repo
exists but isn't a valid fork of the new source. Deleting the fork
first prevents this.
Adds 4 unit tests for fork cleanup: fork deleted before source, fork
missing is skipped, fork delete error propagated, fork delete NotFound
continues to source.
Addresses review feedback on #6665
🔧 Fix agent — iteration 2 (human-triggered)Added fork repo cleanup to resetRepo so that {repoName}-fork is deleted before the source repo, preventing orphaned forks that break subsequent fork creation. Includes 4 new unit tests. All 73 tests pass with -race. Fixed (3):
Tests: passedNext steps:
|
|
🤖 Finished Review · ✅ Success · Started 2:19 AM UTC · Completed 2:37 AM UTC Commit: Runtime: claude · Model: opus → claude-opus-4-6 · Effort: high · Cost: $5.71 |
|
🤖 Finished Retro · ✅ Success · Started 12:13 PM UTC · Completed 12:27 PM UTC Commit: Runtime: claude · Model: opus → claude-opus-4-6 · Effort: high · Cost: $5.15 |
Retro: PR #6665 — Reset pool repo git history on allocationTimelineIssue #6656 was filed as a research task to diagnose a 12+ minute shallow-clone regression in E2E behaviour tests. After a premature first code run (which produced PR #6659, a tactical timeout bump — already covered by the retro on that PR), ralphbean posted the root cause (1.9 GB accumulated git history in pool repos) and manually re-triaged. The second code run produced PR #6665 with a delete-and-recreate approach for pool repos. The PR went through 2 human-directed fix iterations:
After both fixes, all 24 E2E behaviour scenarios passed. Two human reviewers approved and the PR was merged ~16.5 hours after issue creation. AnalysisReview quality: The review agent's findings were useful but operated on a different dimension than the human's. The agent focused on code quality (test fidelity, naming conventions, assertion gaps), while the human caught runtime behavior issues (API eventual consistency delays, fork dependency cascading effects) through E2E test observation. The review agent correctly identified the eventual-consistency risk but rated it [low] — a defensible rating since the code handled Rework rate: 3 commits (initial + 2 fixes). Fix #1 (backoff) was arguably avoidable if the code agent had applied resilience patterns similar to the existing Token cost: Approximately $34 total across all agent runs. The premature first code run ($3.85 + $0.64 triage) is covered by agents#1055. Evidence for existing issues
What went well
|
2 participants
Summary
Reset pool repo git history at the start of each behaviour test suite run to prevent the 12+ minute shallow-clone deepening step that was causing fork PR workflow timeouts.
Changes
resetRepomethod torepoEnsurerthat deletes existing pool repos beforeensureRepoExistsrecreates them, giving each suite run a fresh single-commit repoalreadyInstalledoptimization indoEnsuresince repos are always freshly created after reset — settle (Actions workflow readiness) now runs unconditionallyresetRepocovering: existing repo deletion, missing repo skip, GetRepo error propagation, DeleteRepo error propagation, and ErrNotFound race idempotencyTesting
pkg/behaviourtest/drivers/installpass with-raceresetRepofunction has 100% test coveragedoEnsurefunction has 92.9% coverage (above 80% threshold)go vetpassesgofmtpassesCloses #6656
Post-script verification
agent/6656-reset-pool-repo-history)2666f7d4d409964a1990678c686ec35e7c98b206..HEAD)