fix: improve handling of repository clones in the .remote folder

[kriswest]

resolves #1386

Resolves a number of issues with .remote folder handling (git clones used for diffs), by:

• Deleting the .remote folder on start-up - clones are not currently supposed to persist so this should have been cleaned up
• Creating the .remote folder on start-up
• Not deleting the entire .remote folder when a single push is processed - there could be concurrent pushes to other repos/SHAs that are in progress (due to the async implementation of filtering) - now the individual clone is cleaned up
• ensuring successful clones are always cleaned up at the end of chain processing (where they were not previously if an error occurred)
• ensuring failed clones are cleaned up immediately in pull Remote
• rationalizing the naming of clearBareClone and audit as post-processors, rather than push-action processors (clearBareClone is no longer a direct part of the push chain, but invoked after processing if pullRemote ran successfully - which might one day be part of other chains)
• Improving the implementation of the relevant tests.

[netlify]

👷 Deploy Preview for endearing-brigadeiros-63f9d0 processing.

Name	Link
🔨 Latest commit	8bb0316
🔍 Latest deploy log	https://app.netlify.com/projects/endearing-brigadeiros-63f9d0/deploys/6989b031abb6c70008a458d9

[netlify]

✅ Deploy Preview for endearing-brigadeiros-63f9d0 canceled.

Name	Link
🔨 Latest commit	27e93ff
🔍 Latest deploy log	https://app.netlify.com/projects/endearing-brigadeiros-63f9d0/deploys/6989f1b606fde50008702207

[github-actions]

Dependency Review

✅ No vulnerabilities or license issues or OpenSSF Scorecard issues found.

Scanned Files

None

[codecov]

Codecov Report

❌ Patch coverage is 79.10448% with 14 lines in your changes missing coverage. Please review.
✅ Project coverage is 81.19%. Comparing base (6fb63d0) to head (27e93ff).
⚠️ Report is 8 commits behind head on main.

Files with missing lines	Patch %	Lines
src/proxy/processors/push-action/pullRemote.ts	66.66%	11 Missing ⚠️
.../proxy/processors/post-processor/clearBareClone.ts	78.57%	3 Missing ⚠️

❌ Your patch check has failed because the patch coverage (79.10%) is below the target coverage (80.00%). You can increase the patch coverage or adjust the target coverage.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #1393      +/-   ##
==========================================
- Coverage   81.34%   81.19%   -0.15%     
==========================================
  Files          65       66       +1     
  Lines        4648     4670      +22     
  Branches      792      797       +5     
==========================================
+ Hits         3781     3792      +11     
- Misses        852      863      +11     
  Partials       15       15              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[jescalada]

LGTM after fixing up Fabio's comment! 👍🏼

I think the Architecture doc might need some tweaking after this. We could go ahead and merge #1391 and then update a few things on docs/Architecture.md:

• The clearBareClone description and potential error
• The Action Chains section to show the clearBareClone as a cleanup step rather than an intermediate one
• The pullRemote entry should mention that errors may be thrown on concurrent requests

[kriswest]

CI failed with: @jescalada @coopernetes CI failed with:

  Unable to find image 'mongo:7.0' locally
  7.0: Pulling from library/mongo
  docker: toomanyrequests: You have reached your unauthenticated pull rate limit. https://www.docker.com/increase-rate-limit.

Is the matrix unnecessarily large and should we be authenticating?

[jescalada]

@kriswest I think this is due to running the build CI on several PRs/branches at the same time. We may want to address the flakiness (probably fails around 5-10% of the time at the moment).

Maybe authenticating is the easiest fix? The DOCKER_PASSWORD env is set and ready to be used, like we're doing in docker-publish.yml. Just adding this snippet into the CI.yml might fix it:

- name: Log in to Docker Hub
  if: github.repository == 'finos/git-proxy'
  uses: docker/login-action@c94ce9fb468520275223c153574b00df6fe4bcc9 # v3
  with:
    username: finos
    password: ${{ secrets.DOCKER_PASSWORD }}

[kriswest]

I also note the e2e tests depend on test-repo in @coopernetes namespace:

git-proxy/tests/e2e/push.test.ts

Line 271 in 2452f10

(r: any) => r.url === 'https://git-server:8443/coopernetes/test-repo.git',

Should we fix that? Its a real repo: https://github.com/coopernetes/test-repo
If we need a real repo thats not finos/git-proxy we could ask help@finos.org to host one under @finos-labs for us (with https://github.com/orgs/finos/teams/git-proxy-maintainers as the maintainers for it).

[coopernetes]

The names are real but the repos themselves are fake created with the local git server.

git-proxy/localgit/init-repos.sh

Lines 57 to 87 in 6fb63d0

	# Create repositories with simple content
	echo "=== Creating coopernetes/test-repo.git ==="
	create_bare_repo "coopernetes" "test-repo.git"
	add_content_to_repo "coopernetes" "test-repo.git"
	# Create a simple README
	cat > README.md << 'EOF'
	# Test Repository
	This is a test repository for the git proxy, simulating coopernetes/test-repo.
	EOF
	# Create a simple text file
	cat > hello.txt << 'EOF'
	Hello World from test-repo!
	EOF
	git add .
	git commit -m "Initial commit with basic content"
	git push origin main
	echo "=== Creating finos/git-proxy.git ==="
	create_bare_repo "finos" "git-proxy.git"
	add_content_to_repo "finos" "git-proxy.git"
	# Create a simple README
	cat > README.md << 'EOF'
	# Git Proxy
	This is a test instance of the FINOS Git Proxy project for isolated e2e testing.
	EOF

The e2e tests are meant to be completely self contained with no external dependencies (other than the image pulls which it sounds like we need a fix in the workflow to resolve flakiness).

[kriswest]

I think the Architecture doc might need some tweaking after this. We could go ahead and merge #1391 and then update a few things on docs/Architecture.md:

• The clearBareClone description and potential error
• The Action Chains section to show the clearBareClone as a cleanup step rather than an intermediate one
• The pullRemote entry should mention that errors may be thrown on concurrent requests

Upto you on merge order - can either merge this now and update the doc or merge the doc and then update this... You may be quicker on updating the doc as we've got some deployments happening today which will delay egress checks this end. I'll go add some suggestions to the doc in the meantime.

[jescalada]

@kriswest Suggestions on the doc are much appreciated! 😃 I'll go ahead and merge that PR afterwards.

[kriswest]

The names are real but the repos themselves are fake created with the local git server.

localgit/init-repos.sh

The e2e tests are meant to be completely self contained with no external dependencies (other than the image pulls which it sounds like we need a fix in the workflow to resolve flakiness).

Thanks for the clarification @coopernetes - I'll try and read up on how that works as I haven't had a chance yet (I've just tried and failed to run it locally, no idea yet why I can't run it).

Could we make ir more intuitive by tweaking the URL? e.g. coopernetes -> localGitServer ?

[coopernetes]

The names are real but the repos themselves are fake created with the local git server.
localgit/init-repos.sh
The e2e tests are meant to be completely self contained with no external dependencies (other than the image pulls which it sounds like we need a fix in the workflow to resolve flakiness).

Thanks for the clarification @coopernetes - I'll try and read up on how that works as I haven't had a chance yet (I've just tried and failed to run it locally, no idea yet why I can't run it).

Could we make ir more intuitive by tweaking the URL? e.g. coopernetes -> localGitServer ?

Yep, no issue there. The original rationale was that these tests could work on a live Git Proxy server. It worked somewhat but wasn't fully tested in a variety of environments.

To make it clear, we can replace those references with names that clearly indicate they're not live/real public repositories and they are testing use only.