Skip to content

Audit deployment timeouts as DeploymentTimedOut, not DeploymentFailed#429

Merged
ppXD merged 1 commit into
mainfrom
feat/deployment-timedout-audit-category
Jun 11, 2026
Merged

Audit deployment timeouts as DeploymentTimedOut, not DeploymentFailed#429
ppXD merged 1 commit into
mainfrom
feat/deployment-timedout-audit-category

Conversation

@ppXD

@ppXD ppXD commented Jun 10, 2026

Copy link
Copy Markdown
Collaborator

Summary

  • A wall-clock deployment timeout now pauses the deployment and preserves its checkpoint for resume (shipped in the resumable-timeout change). Recording its audit event as DeploymentFailed misrepresented this recoverable outcome, so a timed-out deployment showed up in the audit feed as a failure.
  • Adds EventCategory.DeploymentTimedOut = 19 and repoints DeploymentAuditEventHandler.OnDeploymentTimedOutAsync to it, so the audit feed distinguishes a timeout (paused/resumable) from a genuine failure.

Why it's non-breaking

  • 19 is additive — no existing value (1–18) is renumbered, and EventCategory stays short-backed. The event.category column is a plain smallint with no CHECK constraint / enum type, so no migration is needed.
  • The new EventCategoryRegistry descriptor is mandatory, not optional: the read path (EventService.ToDtoEventCategoryRegistry.Describe) throws on an unmapped category, and the EveryEventCategory_HasANonEmptyDescriptor drift test enforces it. Both are satisfied by the added descriptor.
  • No switch/exhaustive match over EventCategory anywhere in src/ lacks a default. The handler diff is surgical — only OnDeploymentTimedOutAsync changed; OnDeploymentFailedAsync and the other 8 mappings are untouched.

Pairs with

The resumable-timeout change (separate PR on this milestone) is what makes a timeout paused/resumable. This PR is independently correct — the DeploymentTimedOutEvent is emitted on timeout regardless — and the two together make task state and audit category fully consistent.

Test plan

  • Unit: EventCategoryRegistryTests — display-name InlineData + 19 numbering pin; DeploymentAuditEventFactoryTests — passthrough InlineData. (23/23)
  • Integration (real Postgres): DeploymentAuditEventHandlerTests.TimedOutEvent_IsAuditedAsDeploymentTimedOut asserts the timeout records DeploymentTimedOut; FailedEvent_PersistsDeploymentFailed retained as the contrast proving genuine failures are unaffected. (7/7)
  • dotnet build src/Squid.Core — 0 errors.
  • Adversarial review (independent agent): ship, no blocking issues.

Follow-up (frontend, separate repo)

SquidWeb feat/deployment-timedout-event-tone tones the new category as warning (orange) — opened alongside.

@ppXD ppXD added this to the 1.8.18 milestone Jun 10, 2026
A wall-clock timeout now pauses the deployment and preserves its
checkpoint for resume rather than failing it irrecoverably. Recording
the audit event as DeploymentFailed misrepresented this recoverable
outcome.

Adds EventCategory.DeploymentTimedOut (19 — additive, no renumber of the
persisted smallint, no migration; plain smallint column accepts it) with
its EventCategoryRegistry descriptor (mandatory: the read path and the
drift test both require every category to have one), and repoints
DeploymentAuditEventHandler.OnDeploymentTimedOutAsync to it.
@ppXD
ppXD force-pushed the feat/deployment-timedout-audit-category branch from 79321bf to b229ed7 Compare June 11, 2026 01:17
@ppXD
ppXD merged commit 9edf099 into main Jun 11, 2026
15 checks passed
@ppXD
ppXD deleted the feat/deployment-timedout-audit-category branch June 11, 2026 01:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant