Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions Core/Resgrid.Model/CheckInRecord.cs
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,12 @@ public class CheckInRecord : IEntity

public string Note { get; set; }

/// <summary>
/// Client-supplied idempotency key (the offline outbox event id). When set, a replayed check-in carrying the
/// same key returns the original record instead of inserting a duplicate. See offline-first-architecture.md.
/// </summary>
public string IdempotencyKey { get; set; }

[NotMapped]
public string TableName => "CheckInRecords";

Expand Down
14 changes: 14 additions & 0 deletions Core/Resgrid.Model/IChangeTracked.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
using System;

namespace Resgrid.Model
{
/// <summary>
/// Entities that carry a <see cref="ModifiedOn"/> change cursor. It is stamped on every insert and update
/// and drives two offline-first concerns: the delta-sync "changed since" query (pull) and last-write-wins
/// conflict resolution on reconnect. See <c>docs/architecture/offline-first-architecture.md</c>.
/// </summary>
public interface IChangeTracked
{
DateTime? ModifiedOn { get; set; }
}
}
13 changes: 11 additions & 2 deletions Core/Resgrid.Model/IncidentCommand/CommandStructureNode.cs
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ namespace Resgrid.Model
/// A live lane / span-of-control node on the command board (Division, Group, Branch, Staging, ...).
/// Initially seeded from a <c>CommandDefinitionRole</c> then per-incident editable.
/// </summary>
public class CommandStructureNode : IEntity
public class CommandStructureNode : IEntity, IChangeTracked
{
public string CommandStructureNodeId { get; set; }

Expand All @@ -36,6 +36,12 @@ public class CommandStructureNode : IEntity
/// <summary>The CommandDefinitionRole this node was seeded from, if any.</summary>
public int? SourceRoleId { get; set; }

/// <summary>Soft-delete tombstone so a lane removed offline propagates on delta sync (null = live).</summary>
public DateTime? DeletedOn { get; set; }

/// <summary>Change cursor for offline delta sync + last-write-wins; stamped on every write.</summary>
public DateTime? ModifiedOn { get; set; }

[NotMapped]
public string TableName => "CommandStructureNodes";

Expand All @@ -61,7 +67,7 @@ public object IdValue
/// Assigns a resource to a command structure node. Polymorphic: the resource may be an own-department
/// unit/person, a linked (mutual-aid) department unit/person, or an incident ad-hoc unit/person.
/// </summary>
public class ResourceAssignment : IEntity
public class ResourceAssignment : IEntity, IChangeTracked
{
public string ResourceAssignmentId { get; set; }

Expand All @@ -85,6 +91,9 @@ public class ResourceAssignment : IEntity

public DateTime? ReleasedOn { get; set; }

/// <summary>Change cursor for offline delta sync + last-write-wins; stamped on every write.</summary>
public DateTime? ModifiedOn { get; set; }

[NotMapped]
public string TableName => "ResourceAssignments";

Expand Down
10 changes: 8 additions & 2 deletions Core/Resgrid.Model/IncidentCommand/IncidentAdHocResources.cs
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ namespace Resgrid.Model
/// An incident-scoped, ad-hoc unit created on the fly for resources not in Resgrid (e.g. a mutual-aid
/// crew from a non-Resgrid agency, or a unit formed from on-scene personnel). Not a real department Unit.
/// </summary>
public class IncidentAdHocUnit : IEntity
public class IncidentAdHocUnit : IEntity, IChangeTracked
{
public string IncidentAdHocUnitId { get; set; }

Expand All @@ -34,6 +34,9 @@ public class IncidentAdHocUnit : IEntity

public DateTime? ReleasedOn { get; set; }

/// <summary>Change cursor for offline delta sync + last-write-wins; stamped on every write.</summary>
public DateTime? ModifiedOn { get; set; }

[NotMapped]
public string TableName => "IncidentAdHocUnits";

Expand All @@ -59,7 +62,7 @@ public object IdValue
/// An incident-scoped, ad-hoc person created on the fly for resources not in Resgrid. May ride an ad-hoc
/// (or real) unit for accountability via <see cref="RidingResourceKind"/> + <see cref="RidingResourceId"/>.
/// </summary>
public class IncidentAdHocPersonnel : IEntity
public class IncidentAdHocPersonnel : IEntity, IChangeTracked
{
public string IncidentAdHocPersonnelId { get; set; }

Expand Down Expand Up @@ -88,6 +91,9 @@ public class IncidentAdHocPersonnel : IEntity

public DateTime? ReleasedOn { get; set; }

/// <summary>Change cursor for offline delta sync + last-write-wins; stamped on every write.</summary>
public DateTime? ModifiedOn { get; set; }

[NotMapped]
public string TableName => "IncidentAdHocPersonnel";

Expand Down
5 changes: 4 additions & 1 deletion Core/Resgrid.Model/IncidentCommand/IncidentCommand.cs
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ namespace Resgrid.Model
/// A live incident-command instance established on a specific <c>Call</c>. Seeded (optionally) from a
/// <c>CommandDefinition</c> template and then freely editable by the Commander for the life of the incident.
/// </summary>
public class IncidentCommand : IEntity
public class IncidentCommand : IEntity, IChangeTracked
{
public string IncidentCommandId { get; set; }

Expand Down Expand Up @@ -40,6 +40,9 @@ public class IncidentCommand : IEntity

public DateTime? ClosedOn { get; set; }

/// <summary>Change cursor for offline delta sync + last-write-wins; stamped on every write.</summary>
public DateTime? ModifiedOn { get; set; }

[NotMapped]
public string TableName => "IncidentCommands";

Expand Down
38 changes: 38 additions & 0 deletions Core/Resgrid.Model/IncidentCommand/IncidentCommandChanges.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
using System.Collections.Generic;

namespace Resgrid.Model
{
/// <summary>
/// Delta payload for offline sync: every change-tracked incident-command row whose <see cref="IChangeTracked.ModifiedOn"/>
/// (or, for the append-only timeline, OccurredOn) is newer than the client's last sync cursor, scoped to a department.
/// Soft-deleted / closed / released rows ARE included (with their state columns set) so the client can remove or
/// update them locally. The client stores <see cref="ServerTimestampMs"/> and passes it back as the next `since`.
/// Ad-hoc resources are not change-tracked and are pulled separately (full refetch). See
/// docs/architecture/offline-first-architecture.md.
Comment on lines +10 to +11

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.

📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win

Doc comment contradicts the model and the actual delta behavior.

The remark says ad-hoc resources "are not change-tracked and are pulled separately (full refetch)", yet this payload exposes AdHocUnits/AdHocPersonnel (Lines 32-34) and GetAdHocChangesSinceAsync filters them by ModifiedOn (delta, not full refetch). This will mislead clients implementing the sync contract.

📝 Suggested doc fix
-		/// Soft-deleted / closed / released rows ARE included (with their state columns set) so the client can remove or
-		/// update them locally. The client stores <see cref="ServerTimestampMs"/> and passes it back as the next `since`.
-		/// Ad-hoc resources are not change-tracked and are pulled separately (full refetch). See
-		/// docs/architecture/offline-first-architecture.md.
+		/// Soft-deleted / closed / released rows ARE included (with their state columns set) so the client can remove or
+		/// update them locally. The client stores <see cref="ServerTimestampMs"/> and passes it back as the next `since`.
+		/// Ad-hoc resources are change-tracked via ModifiedOn and delta-pulled alongside command rows (aggregated by
+		/// SyncController). See docs/architecture/offline-first-architecture.md.
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
/// Ad-hoc resources are not change-tracked and are pulled separately (full refetch). See
/// docs/architecture/offline-first-architecture.md.
/// Soft-deleted / closed / released rows ARE included (with their state columns set) so the client can remove or
/// update them locally. The client stores <see cref="ServerTimestampMs"/> and passes it back as the next `since`.
/// Ad-hoc resources are change-tracked via ModifiedOn and delta-pulled alongside command rows (aggregated by
/// SyncController). See docs/architecture/offline-first-architecture.md.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@Core/Resgrid.Model/IncidentCommand/IncidentCommandChanges.cs` around lines 10
- 11, The XML doc comment on IncidentCommandChanges contradicts the actual sync
behavior exposed by AdHocUnits, AdHocPersonnel, and GetAdHocChangesSinceAsync.
Update the remark to describe these ad-hoc resources as delta-tracked by
ModifiedOn and returned through the incremental changes payload, rather than
saying they are not change-tracked or require a full refetch. Keep the wording
aligned with the model fields and the filtering logic so clients can rely on the
sync contract.

/// </summary>
public class IncidentCommandChanges
{
/// <summary>Server clock (Unix epoch ms) captured at the start of the read; the client's next-sync cursor.</summary>
public long ServerTimestampMs { get; set; }

public List<IncidentCommand> Commands { get; set; } = new List<IncidentCommand>();

public List<CommandStructureNode> Nodes { get; set; } = new List<CommandStructureNode>();

public List<ResourceAssignment> Assignments { get; set; } = new List<ResourceAssignment>();

public List<TacticalObjective> Objectives { get; set; } = new List<TacticalObjective>();

public List<IncidentTimer> Timers { get; set; } = new List<IncidentTimer>();

public List<IncidentMapAnnotation> Annotations { get; set; } = new List<IncidentMapAnnotation>();

public List<IncidentRoleAssignment> Roles { get; set; } = new List<IncidentRoleAssignment>();

public List<IncidentAdHocUnit> AdHocUnits { get; set; } = new List<IncidentAdHocUnit>();

public List<IncidentAdHocPersonnel> AdHocPersonnel { get; set; } = new List<IncidentAdHocPersonnel>();

public List<CommandLogEntry> TimelineEntries { get; set; } = new List<CommandLogEntry>();
}
}
5 changes: 4 additions & 1 deletion Core/Resgrid.Model/IncidentCommand/IncidentRole.cs
Original file line number Diff line number Diff line change
Expand Up @@ -149,7 +149,7 @@ public static IncidentCapabilities GetCapabilities(IncidentRoleType role)
/// Assigns a Resgrid user to a functional incident-command role for a specific incident (Call). Incident-scoped,
/// not a department-wide claim. Optionally scoped to a structure node for supervisors.
/// </summary>
public class IncidentRoleAssignment : IEntity
public class IncidentRoleAssignment : IEntity, IChangeTracked
{
public string IncidentRoleAssignmentId { get; set; }

Expand All @@ -174,6 +174,9 @@ public class IncidentRoleAssignment : IEntity

public DateTime? RemovedOn { get; set; }

/// <summary>Change cursor for offline delta sync + last-write-wins; stamped on every write.</summary>
public DateTime? ModifiedOn { get; set; }

[NotMapped]
public string TableName => "IncidentRoleAssignments";

Expand Down
15 changes: 12 additions & 3 deletions Core/Resgrid.Model/IncidentCommand/IncidentTacticals.cs
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
namespace Resgrid.Model
{
/// <summary>A tactical objective / benchmark for an incident (e.g. "Primary search complete").</summary>
public class TacticalObjective : IEntity
public class TacticalObjective : IEntity, IChangeTracked
{
public string TacticalObjectiveId { get; set; }

Expand All @@ -32,6 +32,9 @@ public class TacticalObjective : IEntity

public int SortOrder { get; set; }

/// <summary>Change cursor for offline delta sync + last-write-wins; stamped on every write.</summary>
public DateTime? ModifiedOn { get; set; }

[NotMapped]
public string TableName => "TacticalObjectives";

Expand All @@ -57,7 +60,7 @@ public object IdValue
/// A scene / benchmark / role timer for an incident. Personnel accountability (PAR) is handled by the
/// Checkin feature, not by these timers.
/// </summary>
public class IncidentTimer : IEntity
public class IncidentTimer : IEntity, IChangeTracked
{
public string IncidentTimerId { get; set; }

Expand Down Expand Up @@ -89,6 +92,9 @@ public class IncidentTimer : IEntity

public DateTime? AcknowledgedOn { get; set; }

/// <summary>Change cursor for offline delta sync + last-write-wins; stamped on every write.</summary>
public DateTime? ModifiedOn { get; set; }

[NotMapped]
public string TableName => "IncidentTimers";

Expand All @@ -111,7 +117,7 @@ public object IdValue
}

/// <summary>A real-time map annotation (markup) on the tactical map, synced across devices.</summary>
public class IncidentMapAnnotation : IEntity
public class IncidentMapAnnotation : IEntity, IChangeTracked
{
public string IncidentMapAnnotationId { get; set; }

Expand All @@ -138,6 +144,9 @@ public class IncidentMapAnnotation : IEntity

public DateTime? DeletedOn { get; set; }

/// <summary>Change cursor for offline delta sync + last-write-wins; stamped on every write.</summary>
public DateTime? ModifiedOn { get; set; }

[NotMapped]
public string TableName => "IncidentMapAnnotations";

Expand Down
7 changes: 7 additions & 0 deletions Core/Resgrid.Model/Services/IIncidentCommandService.cs
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,13 @@ public interface IIncidentCommandService
Task<IncidentCommandBoard> GetCommandBoardAsync(int departmentId, int callId);
Task<List<PersonnelCallCheckInStatus>> GetAccountabilityForCallAsync(int departmentId, int callId);

/// <summary>
/// Offline-first delta pull: returns every change-tracked incident-command row (and append-only timeline entry)
/// for the department whose ModifiedOn/OccurredOn is newer than <paramref name="sinceUtc"/>. Includes
/// soft-deleted/closed/released rows so a reconnecting client can reconcile removals. See the offline-first doc.
/// </summary>
Task<IncidentCommandChanges> GetChangesSinceAsync(int departmentId, System.DateTime sinceUtc);

/// <summary>
/// Sweeps personnel accountability (PAR) for the call and raises <c>CriticalParDetectedEvent</c> once per
/// member each time they transition into the Critical (overdue) state. Idempotent via a timeline marker —
Expand Down
7 changes: 7 additions & 0 deletions Core/Resgrid.Model/Services/IIncidentResourcesService.cs
Original file line number Diff line number Diff line change
Expand Up @@ -26,5 +26,12 @@ public interface IIncidentResourcesService

/// <summary>Forms a new ad-hoc unit and attaches the given ad-hoc personnel to it as its roster.</summary>
Task<IncidentAdHocUnit> FormUnitFromPersonnelAsync(IncidentAdHocUnit unit, List<string> adHocPersonnelIds, string userId, CancellationToken cancellationToken = default(CancellationToken));

/// <summary>
/// Offline-first delta pull for ad-hoc resources: returns the department's ad-hoc units and personnel whose
/// ModifiedOn is newer than <paramref name="sinceUtc"/> (released rows included so the client reconciles them).
/// Aggregated into the unified /Sync/Changes payload by SyncController. See offline-first-architecture.md.
/// </summary>
Task<(List<IncidentAdHocUnit> Units, List<IncidentAdHocPersonnel> Personnel)> GetAdHocChangesSinceAsync(int departmentId, System.DateTime sinceUtc);
}
}
30 changes: 29 additions & 1 deletion Core/Resgrid.Services/CheckInTimerService.cs
Original file line number Diff line number Diff line change
Expand Up @@ -320,8 +320,36 @@ public async Task<List<ResolvedCheckInTimer>> ResolveAllTimersForCallAsync(Call

public async Task<CheckInRecord> PerformCheckInAsync(CheckInRecord record, CancellationToken cancellationToken = default)
{
// Offline idempotency: when the client supplies its outbox event id, a replayed check-in returns the
// original record instead of inserting a duplicate. Dedup is in-memory over the call's (small) check-in
// set, so only replayed events — which carry a key — pay the extra read; live UI check-ins skip it.
if (!string.IsNullOrWhiteSpace(record.IdempotencyKey))
{
var existing = await _recordRepository.GetByCallIdAsync(record.CallId);
var duplicate = existing?.FirstOrDefault(r => r.IdempotencyKey == record.IdempotencyKey);
if (duplicate != null)
return duplicate;
}
Comment thread
coderabbitai[bot] marked this conversation as resolved.

record.Timestamp = DateTime.UtcNow;
var saved = await _recordRepository.SaveOrUpdateAsync(record, cancellationToken);

CheckInRecord saved;
try
{
saved = await _recordRepository.SaveOrUpdateAsync(record, cancellationToken);
}
catch (Exception) when (!string.IsNullOrWhiteSpace(record.IdempotencyKey))
{
// The in-memory pre-check above is check-then-insert and races under concurrent replays; the filtered
// unique index UX_CheckInRecords_Department_IdempotencyKey is the real guard. If we lost the race,
// adopt the winner — same idempotent result as the pre-check — rather than surfacing a 500 (which would
// just trigger another retry).
var existing = await _recordRepository.GetByCallIdAsync(record.CallId);
var winner = existing?.FirstOrDefault(r => r.IdempotencyKey == record.IdempotencyKey);
if (winner != null)
return winner;
Comment on lines +341 to +350

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.

🩺 Stability & Availability | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Verify which provider/repository exception types represent unique constraint violations
# so the catch filter can be narrowed without guessing.
rg -nP -C3 '(SaveOrUpdateAsync|SqlException|PostgresException|DbUpdateException|unique constraint|duplicate key|2601|2627|23505)' Core Providers -g '*.cs'

Repository: Resgrid/Core

Length of output: 50369


🏁 Script executed:

python3 - <<'PY'
code_snippet = """
			try
			{
				saved = await _recordRepository.SaveOrUpdateAsync(record, cancellationToken);
			}
			catch (Exception) when (!string.IsNullOrWhiteSpace(record.IdempotencyKey))
			{
				var existing = await _recordRepository.GetByCallIdAsync(record.CallId);
				var winner = existing?.FirstOrDefault(r => r.IdempotencyKey == record.IdempotencyKey);
				if (winner != null)
					return winner;
				throw;
			}
"""

# Analyze the behavior of `throw` in this specific context
analysis = """
The snippet uses `catch (Exception)` without specifying a variable name.
Inside the block:
1. `var existing = await ...` is an I/O call. If this call fails, an *new* exception is awaited.
   - If it throws an `OperationCanceledException`, that exception bubbles up immediately. The original `Exception` is effectively lost/swapped for the new failure if the await fails, or the new one propagates.
   - If it succeeds but `winner` is null, the code hits `throw;`.
2. C# `throw;` with no arguments re-throws the *original* exception caught by the `catch` block (the one from `SaveOrUpdateAsync`).
3. Therefore, if `winner` is null, the original exception propagates correctly.

However, the risk remains if `SaveOrUpdateAsync` throws an exception that is NOT a unique violation (e.g., FK constraint violation "23503", Serialization error, or Timeout), and `existing` query *successfully* returns a record (perhaps from a different idempotency key or just a stale read).

If `winner` is found (even if it's the wrong idempotency key? No, the code checks `r.IdempotencyKey == record.IdempotencyKey`), then it returns the winner.
The logic `if (winner != null)` only returns if a record matches the key.
If the original exception was a Timeout, but `winner` happens to exist in the DB with the matching key (from a previous retry that partially completed), the current code treats it as a success.

CRITICAL ISSUE: The exception filter `when (!string.IsNullOrWhiteSpace(record.IdempotencyKey))` applies to the ENTRY of the catch block.
It does NOT guard against:
1. Non-unique exceptions (like Foreign Keys) where a matching record happens to exist.
2. It is safer to check the exception type explicitly.
"""
print(analysis)
PY

Repository: Resgrid/Core

Length of output: 1853


Limit retry adoption strictly to unique-key violations.

The current retry logic catches all exceptions regardless of type, risking the masking of valid failures (e.g., timeouts, foreign key violations) as successful replays if a matching row happens to exist. For example, if a transaction times out but a partial write created the record, the code returns success even though the original operation failed with a non-unique-key error.

Refine the exception handling to filter only for PostgreSQL unique constraint violations (SqlState == "23505") to ensure other errors propagate immediately.

Proposed Fix
try
{
    saved = await _recordRepository.SaveOrUpdateAsync(record, cancellationToken);
}
catch (Npgsql.PostgresException ex) when (ex.SqlState == "23505" && !string.IsNullOrWhiteSpace(record.IdempotencyKey))
{
    var existing = await _recordRepository.GetByCallIdAsync(record.CallId, cancellationToken);
    var winner = existing?.FirstOrDefault(r => r.IdempotencyKey == record.IdempotencyKey);
    if (winner != null)
        return winner;

    throw;
}
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@Core/Resgrid.Services/CheckInTimerService.cs` around lines 341 - 350, The
replay-adoption catch in CheckInTimerService’s save flow is too broad because it
catches every exception and can mask real failures as successful idempotent
replays. Narrow the exception filter around the SaveOrUpdateAsync path to only
handle PostgreSQL unique-constraint violations by catching
Npgsql.PostgresException and checking SqlState == "23505" together with a
non-empty record.IdempotencyKey; keep the existing winner lookup logic, but let
all other exceptions propagate immediately so non-unique-key errors are not
swallowed.

throw;
}

// Real-time board refresh is best-effort: the check-in is already persisted, so a CQRS/Redis
// publish failure must not fail the check-in — that would 500 the caller and a retry would
Expand Down
Loading
Loading