Skip to content

sanitize agent should allow done updates to canceled pipelines#6394

Merged
6543 merged 14 commits into
woodpecker-ci:mainfrom
6543-forks:fix-cancel
Apr 8, 2026
Merged

sanitize agent should allow done updates to canceled pipelines#6394
6543 merged 14 commits into
woodpecker-ci:mainfrom
6543-forks:fix-cancel

Conversation

@6543

@6543 6543 commented Apr 6, 2026

Copy link
Copy Markdown
Member

again ... :/ ...I think #6391 will help us not having regressions that much 😅 ...

what the original pull did not do... it only checked the current state without checking what the new state should switch to.
we have to verify that and allow more update paths without introducing more bugs and still have it hardened to avoid getting bugs into the state caused by rouge/buggy agents...

this also simplifies the code

cause: #6308

@6543 6543 added the regression fix a bug that was not released yet label Apr 6, 2026
@6543 6543 self-assigned this Apr 6, 2026
@6543 6543 added the wip label Apr 6, 2026
@6543 6543 changed the title Fix cancel sanitize agent should allow done updates to canceled pipelines Apr 8, 2026
@6543 6543 marked this pull request as ready for review April 8, 2026 11:42
@6543 6543 removed the wip label Apr 8, 2026
@6543 6543 removed their assignment Apr 8, 2026
@6543 6543 requested a review from a team April 8, 2026 11:44

@lafriks lafriks left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

on small comment but otherwise LGTM

Comment thread server/rpc/sanitize.go Outdated
@6543 6543 requested a review from lafriks April 8, 2026 14:21
@6543 6543 enabled auto-merge (squash) April 8, 2026 15:07
@6543 6543 merged commit 7efe8bf into woodpecker-ci:main Apr 8, 2026
7 checks passed
@codecov

codecov Bot commented Apr 8, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 83.60656% with 10 lines in your changes missing coverage. Please review.
✅ Project coverage is 35.44%. Comparing base (a688600) to head (23f7043).
⚠️ Report is 1 commits behind head on main.

Files with missing lines Patch % Lines
server/pipeline/step_status.go 70.00% 5 Missing and 1 partial ⚠️
server/rpc/sanitize.go 89.47% 3 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #6394      +/-   ##
==========================================
- Coverage   35.45%   35.44%   -0.01%     
==========================================
  Files         427      427              
  Lines       28646    28650       +4     
==========================================
- Hits        10156    10155       -1     
- Misses      17561    17565       +4     
- Partials      929      930       +1     

☔ 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.

@6543 6543 deleted the fix-cancel branch April 8, 2026 15:27
@6543

6543 commented Apr 8, 2026

Copy link
Copy Markdown
Member Author

note the e2e test in #6391 already catches this bug and faills before this pull ...

-> https://ci.woodpecker-ci.org/repos/3780/pipeline/33253/46

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

regression fix a bug that was not released yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants