fix: cancel in-progress deploy when webhook triggers new deploy (closes #38) #52

clawbot · 2026-02-16T07:12:34+01:00

clawbot commented

2026-02-16 07:12:34 +01:00

Summary

Fixes the race condition between manual and webhook deploys (issue #38).

When a webhook-triggered deploy starts for an app that already has a deploy in progress, the existing deploy is now cancelled via context cancellation before the new deploy begins.

Changes

internal/service/deploy/deploy.go: Replace simple mutex-only locking with active deploy tracking (cancel func + done channel per app). Deploy() now accepts cancelExisting bool parameter. Added cancelActiveDeploy() and checkCancelled() helpers.
internal/service/webhook/webhook.go: Pass cancelExisting: true when triggering deploys from webhooks.
internal/handlers/app.go: Pass cancelExisting: false for manual deploys (preserves existing ErrDeploymentInProgress behavior).
internal/models/deployment.go: Add DeploymentStatusCancelled status.
Tests: Comprehensive tests for cancellation mechanics (cancel and wait, blocks until done, allows new deploy after cancel).

How it works

Each call to Deploy() registers a cancellable context and done channel in activeDeploys map
When a webhook deploy calls Deploy(ctx, app, eventID, true), it first calls cancelActiveDeploy() which cancels the in-progress deploy's context and blocks until it finishes
The cancelled deploy detects context cancellation, marks itself as cancelled status, and returns ErrDeployCancelled
The new deploy then acquires the lock and proceeds normally

## Summary Fixes the race condition between manual and webhook deploys (issue #38). When a webhook-triggered deploy starts for an app that already has a deploy in progress, the existing deploy is now cancelled via context cancellation before the new deploy begins. ## Changes - **`internal/service/deploy/deploy.go`**: Replace simple mutex-only locking with active deploy tracking (cancel func + done channel per app). `Deploy()` now accepts `cancelExisting bool` parameter. Added `cancelActiveDeploy()` and `checkCancelled()` helpers. - **`internal/service/webhook/webhook.go`**: Pass `cancelExisting: true` when triggering deploys from webhooks. - **`internal/handlers/app.go`**: Pass `cancelExisting: false` for manual deploys (preserves existing ErrDeploymentInProgress behavior). - **`internal/models/deployment.go`**: Add `DeploymentStatusCancelled` status. - **Tests**: Comprehensive tests for cancellation mechanics (cancel and wait, blocks until done, allows new deploy after cancel). ## How it works 1. Each call to `Deploy()` registers a cancellable context and done channel in `activeDeploys` map 2. When a webhook deploy calls `Deploy(ctx, app, eventID, true)`, it first calls `cancelActiveDeploy()` which cancels the in-progress deploy's context and blocks until it finishes 3. The cancelled deploy detects context cancellation, marks itself as `cancelled` status, and returns `ErrDeployCancelled` 4. The new deploy then acquires the lock and proceeds normally

sneak was assigned by clawbot

2026-02-16 07:12:34 +01:00

clawbot added 1 commit 2026-02-16 07:12:34 +01:00

fix: cancel in-progress deploy when webhook triggers new deploy (closes #38 ) 3f499163a7

When a webhook-triggered deploy starts for an app that already has a deploy
in progress, the existing deploy is now cancelled via context cancellation
before the new deploy begins. This prevents silently lost webhook deploys.

Changes:
- Add per-app active deploy tracking with cancel func and done channel
- Deploy() accepts cancelExisting param: true for webhook, false for manual
- Cancelled deployments are marked with new 'cancelled' status
- Add ErrDeployCancelled sentinel error
- Add DeploymentStatusCancelled model constant
- Add comprehensive tests for cancellation mechanics

clawbot commented

2026-02-16 07:12:51 +01:00

✅ Test Results

All tests pass:

ok   git.eeqj.de/sneak/upaas/internal/database        coverage: 1.6%
ok   git.eeqj.de/sneak/upaas/internal/docker           coverage: 2.8%
ok   git.eeqj.de/sneak/upaas/internal/handlers         coverage: 23.5%
ok   git.eeqj.de/sneak/upaas/internal/middleware        coverage: 54.5%
ok   git.eeqj.de/sneak/upaas/internal/models            coverage: 53.1%
ok   git.eeqj.de/sneak/upaas/internal/service/app       coverage: 82.8%
ok   git.eeqj.de/sneak/upaas/internal/service/auth      coverage: 62.7%
ok   git.eeqj.de/sneak/upaas/internal/service/deploy    coverage: 3.7%
ok   git.eeqj.de/sneak/upaas/internal/service/webhook   coverage: 93.3%
ok   git.eeqj.de/sneak/upaas/internal/ssh               coverage: 78.6%

✅ Lint Results

No new lint issues. Only pre-existing testpackage issue on tail_validation_test.go.

## ✅ Test Results All tests pass: ``` ok git.eeqj.de/sneak/upaas/internal/database coverage: 1.6% ok git.eeqj.de/sneak/upaas/internal/docker coverage: 2.8% ok git.eeqj.de/sneak/upaas/internal/handlers coverage: 23.5% ok git.eeqj.de/sneak/upaas/internal/middleware coverage: 54.5% ok git.eeqj.de/sneak/upaas/internal/models coverage: 53.1% ok git.eeqj.de/sneak/upaas/internal/service/app coverage: 82.8% ok git.eeqj.de/sneak/upaas/internal/service/auth coverage: 62.7% ok git.eeqj.de/sneak/upaas/internal/service/deploy coverage: 3.7% ok git.eeqj.de/sneak/upaas/internal/service/webhook coverage: 93.3% ok git.eeqj.de/sneak/upaas/internal/ssh coverage: 78.6% ``` ## ✅ Lint Results No new lint issues. Only pre-existing `testpackage` issue on `tail_validation_test.go`.

clawbot referenced this pull request

2026-02-16 07:14:13 +01:00

fix: cancel in-progress deploy on new webhook trigger (closes #38) #53

sneak merged commit ebcae55302 into main

2026-02-16 09:06:41 +01:00

sneak referenced this issue from a commit

2026-02-16 09:06:42 +01:00

Merge pull request 'fix: cancel in-progress deploy when webhook triggers new deploy (closes #38)' (#52) from clawbot/upaas:fix/deploy-race-condition-38 into main

clawbot referenced this pull request

2026-02-16 09:12:09 +01:00

Update TODO.md with current status (closes #54) #55

clawbot referenced this pull request