feat: add live feedback during pushes (sideband streaming)

[jescalada]

Description

Implements sideband streaming by intercepting git-receive-pack POST requests to stream directly to the user.

On successful pushes

This is what a git push looks like (note that I added a randomized delay between each step to make the execution obvious).

git-proxy-sideband-streaming-demo.mp4

On failed checks

Blocked pushes currently stop at the step that failed, although it's easier to visualize the push going through the chain:

I've been wondering about whether we should edit the Processor's exec function to take in the SidebandProgressWriter. This would allow processors to send arbitrary messages to the user in their implementation, rather than just the current step being executed.

For a separate PR I'd like to make the chain execute all the steps (or at least the steps that aren't dependent on previous ones) and use stream the results of failed checks before finally blocking the push at the end.

Related Issue

Resolves #1485
Related discussion: #1556

Checklist

General

• I have read the CONTRIBUTING.md guidelines
• Commit messages follow Conventional Commits format
• I have a FINOS CLA on file

Documentation

• Documentation has been added/updated for any new features

Configuration

• If configuration schema (config.schema.json) was modified:
  • TypeScript types regenerated (npm run generate-config-types)
  • Schema reference docs regenerated (npm run gen-schema-doc)

Tests

• Tests have been added/updated for new functionality
• Unit tests pass (npm test)
• Linting and formatting pass (npm run lint and npm run format:check)
• Type checks pass (npm run check-types)

[netlify]

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

Name	Link
🔨 Latest commit	b4f7917
🔍 Latest deploy log	https://app.netlify.com/projects/endearing-brigadeiros-63f9d0/deploys/6a4ccd933ac343000878ceef

[github-actions]

Dependency Review

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

Scanned Files

None

[codecov]

Codecov Report

❌ Patch coverage is 86.32479% with 48 lines in your changes missing coverage. Please review.
✅ Project coverage is 85.75%. Comparing base (3186da2) to head (b4f7917).

Files with missing lines	Patch %	Lines
src/proxy/routes/index.ts	82.19%	33 Missing and 1 partial ⚠️
src/proxy/chain.ts	62.50%	12 Missing ⚠️
src/config/index.ts	80.00%	1 Missing ⚠️
src/proxy/sideband.ts	99.09%	0 Missing and 1 partial ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #1637      +/-   ##
==========================================
- Coverage   85.87%   85.75%   -0.12%     
==========================================
  Files          84       85       +1     
  Lines        8109     8448     +339     
  Branches     1376     1457      +81     
==========================================
+ Hits         6964     7245     +281     
- Misses       1117     1172      +55     
- Partials       28       31       +3     

☔ View full report in Codecov by Harness.
📢 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]

Pinging @finos/git-proxy-maintainers for thoughts on this 👍🏼

I intend to refactor things a bit to make the feedback more flexible. Should we pass the sideband writer into each Processor's exec function so that user-facing messages are defined there instead of the chain?

I think it might be a good chance to also refactor the chain to run all the steps, give feedback on whether each check passed or failed and finally reject the push at the end if anything failed.

[andypols]

@jescalada That's really nice. I like that a lot.

I know its a different issue - is there a way to get rid of the fatal error from git when the proxy blocks the the push? Assume not?

[jescalada]

@andypols It's not hard to fix, actually. It requires reporting the status correctly to band 1, which we weren't doing previously. It even lets us add a rejection reason next to the ! [remote rejected] bit:

Thanks for the feedback!

[kriswest]

This looks like a great improvements, thanks @jescalada and @coopernetes (for the inspiration).

I DO think you should refactor to make sure all steps run (although do remember that some are dependent on previous steps - you can't scan the diff until its been retrieved) so you may end up needing to express some dependencies or a flag indicating that you shouldn't run subsequent steps if the current step fails. I believe @andypols talked about a similar change he made in a fork to convert some of the errors into advisories for reviewers to consider during review?? Perhaps we need to create a a scale or results pass -> advisory -> error?

[kriswest]

P.S> replacing the fatal error with a custom message was also a great improvement

[jescalada]

@kriswest I've implemented the full chain run in #1640 - where I added an array of "collectible steps" (non-critical ones) that are reported at the end of the push in case of failure. Meanwhile, critical steps (getDiff, parsePush, etc.) end the push immediately as before.