Skip to content

ADR-0026: Decrement Action Symmetry Discipline

Accepted Cross-Project Universal

Date: 2026-05-15 Amended: 2026-05-15 — atomic-conditional shape (Option E) supersedes the initially-accepted caller-side discipline (Option A) after the reviewer of PR #240 surfaced a TOCTOU race in the caller-side SELECT-then-decrement pattern. The amendment narrative is preserved below in §Options Considered §E and §Amendment Log.

Context

A class of Actions exists whose sole job is to mutate one side of a bookkeeping pair — typically an aggregate counter increment or decrement. In emmie, the canonical pair is App\Actions\Model\PresenceData\IncrementPresenceCountAction and App\Actions\Model\PresenceData\DecrementPresenceCountAction, which mutate presence_data.registered (a BIGINT UNSIGNED column). The pattern generalizes to any Action that performs a counter mutation as its terminal effect — audit-count aggregates, soft-delete-tombstone counters, reservation-quota counters, etc.

These Actions carry an implicit contract: every decrement call must be balanced by an earlier increment call against the same key. When the contract holds, the counter accurately reflects truth. When it breaks (over-decrement), the counter underflows. For BIGINT UNSIGNED columns the database raises SQLSTATE 22003 (immediate, loud, surfaces in production logs). For signed columns the counter silently drifts negative, masking the bug until a downstream consumer notices implausible totals.

The contract has now broken three times in 2026 on the same Action class:

  1. PR #233 (merged 2026-05-15) — DeleteVacationAction decremented presence_data.registered for every day in the vacation period instead of every actually-deleted DailyRegistration. Fixed at the caller level (fetch-then-decrement-per-row pattern).
  2. PR #240 / EMMIE-0272 (merged 2026-05-15) — UpdateVacationAction over-decremented on cross-vacation and cross-schedule overlap edges. Fixed at the caller level (if (PresenceData.registered > 0) guard before the decrement call).
  3. EMMIE-0273 (filed 2026-05-15, awaiting this ADR's disposition) — ForceDeleteDailyRegistrationAction reachable under a narrower trigger; same bug class. Fix awaiting doctrine call.

Each bug survived its merging PR's review because the unit-test suite pinned the asymmetric behavior with shouldReceive(...)->never() assertions on the increment-side counterpart Action (GenerateVacationRegistrationsActionTest:129-201) without an integration test exercising the end-to-end count invariant. The Surveyor's 2026-05-15 deep-dive on the increment/decrement pair concluded: "All UpdateVacationActionTest cases are pure-mock; the lack of an integration test pair to #233's [email protected] underflow is itself the root-cause of why this bug class survived PR #233's review." The test-architecture finding is being audited separately by a Quartermaster mission whose enforcement-queue proposal will feed back into this ADR's Implementation section once it lands.

The structural question is not "where should each individual fix live" — all three landed at the caller level by reflex, mirroring the precedent. The question is whether the caller-level discipline is the doctrine (every caller of every decrement Action is responsible for symmetry, enforced architecturally) or whether the decrement Action itself should defend against caller asymmetry (centralize the floor at the action layer, callers can be trusted naive). The repetition across three sites within ten weeks is the trigger: a pattern that recurs deserves enforcement at the lowest level of the war-room enforcement escalation ladder, and the choice of which level depends on this disposition.

Options Considered

A — Caller-side discipline enforced by architecture test

Every caller of a decrement Action of this class is responsible for verifying the counter is at a non-zero value before invoking the decrement. The decrement Action itself stays as-is: a single SQL decrement(...) with no internal guard. A Pest architecture test enforces that every *->decrement invocation against presence_data.registered-equivalent columns is either:

  • Preceded by a > 0 check on the same column in the same method, OR
  • Carries a per-line @see ADR-0026 allow-list marker (mirroring the ADR-0022/0023 per-line marker pattern).

When a counter underflows because a caller skipped the guard, SQLSTATE 22003 fires loud and the bug is immediately visible. No drift masking.

Pros:

  • Aligns with war-room doctrine #1 (explicit over implicit) — the decrement Action's signature advertises a counter operation; adding internal arithmetic safety would mask that.
  • No drift masking. Bugs in the symmetry contract fail loud in production.
  • Pushes enforcement to Level 1 of the war-room enforcement ladder (architecture test, CI gate).
  • Mirrors emmie's established per-line marker discipline (ScheduleMutationChokepointTest.php §G.3 enforces strict-comparator markers; same shape).
  • The pattern composes — future decrement-Action classes inherit the discipline automatically once the arch test's column-name regex is extended.

Cons:

  • Every caller carries 5+ lines of guard code. Duplicated across the four current sites and any future site.
  • Architecture test must be authored and maintained. Initial cost: ~1 day of arch-test work plus retrofit on the 4 existing sites (3 of which are already in compliance post-#233/#240, leaving EMMIE-0273 as the only retrofit).
  • A caller can technically bypass with a per-line marker, requiring reviewer discipline at marker-grant time (same maintenance cost as ADR-0022/0023's marker scheme).

B — Action-level floor guard

Modify DecrementPresenceCountAction::execute() to perform an internal floor check: read the current registered value, only decrement if > 0. Callers can invoke the action naive; the action absorbs the underflow trap. No architecture test required.

Pros:

  • Defense in depth — one defensive layer makes all callers safe regardless of upstream symmetry violations.
  • Less ceremony at call sites.
  • Future callers automatically inherit the protection without doctrinal training.

Cons:

  • Drift masking. A counter that silently floor-guards is a counter where "1 decrement attempted, 0 applied" is invisible. The bug class that produced #233 and EMMIE-0272 was only detectable because SQLSTATE 22003 fired loud in production. With a floor guard, the same upstream asymmetry would silently underflow to a value that looks correct but is coincidentally so — the Surveyor's empirical SQL queries against presence_data_recompute_log (post-EMMIE-0271 deploy) would still surface drift, but reactive detection requires log queries instead of production exception reports.
  • Audit-trail muddiness. ISO 27001 (A.8.15), NEN 7510, and AVG require tamper-detectable audit. A counter that silently absorbs decrements without logging them violates the explicit-action-trail principle. (Emmie-specific concern; cross-project ADR but the compliance burden is asymmetric across territories.)
  • Conflicts with war-room doctrine #1 (explicit over implicit) — the Action's name (Decrement…) advertises a counter operation; internal arithmetic safety violates the contract.
  • Symmetric increment-side asymmetry (the equivalent over-increment bug class) would still be possible and would not be caught by this defense. The ADR would need a counterpart for IncrementPresenceCountAction (ceiling guard?) — schema-dependent, awkward.

C — Hybrid: action-level guard + skip-audit log

Same as B but the action writes an entry to a dedicated *_skip_log table whenever a decrement was attempted but skipped due to the floor guard. Audit trail preserved; drift detectable retrospectively via log query.

Pros:

  • Defense in depth (B's win) without B's drift-masking cost.
  • The skip-log becomes the operational signal — high skip rates flag an upstream asymmetry without the bug having to manifest as an underflow exception.
  • Compatible with ISO 27001 / NEN 7510 audit requirements at the territory level (the log is the audit artifact).

Cons:

  • New table per counter (presence_data_skip_log; later audit_count_skip_log, quota_skip_log, ...). Schema growth scales with counter classes.
  • Action gains write responsibility beyond its single-purpose contract (log write + decrement). Violates ADR-0011 (Action Class Architecture)'s single-responsibility principle.
  • Operational reaction loop required — someone has to actually query the skip log periodically. Without monitoring infrastructure, the log silently fills.
  • Caller-side discipline still applies in practice (skip-log entries are bug evidence, not bug absence) — so the doctrine work is the same, just deferred to the log-review step.

D — Status quo (no enforcement, no centralization)

Leave the code as-is. Each bug is fixed individually as it surfaces. No architecture test, no centralized guard.

Pros:

  • Zero ADR-implementation cost today.

Cons:

  • Bug class already recurred three times in ten weeks. The fourth recurrence (Ambulatory channel siblings, per Surveyor §Scope Limitations) is plausibly imminent.
  • The war-room enforcement escalation ladder explicitly rejects this — recurring patterns deserve enforcement at the lowest available level, not passive monitoring (Level 6, weakest).

E — Action-layer atomic conditional UPDATE (added in 2026-05-15 amendment)

The original ADR considered "action-level guard" too narrowly. Option B as described above presupposed a SELECT-then-conditional-decrement implementation, which inherits the same TOCTOU race the caller-side guard has (the reviewer of PR #240 identified this race in the caller-side shape and proposed the atomic refinement). Option E is the third variant within the action-level family: collapse the guard into the UPDATE statement itself.

Modify DecrementPresenceCountAction::execute() to issue a single conditional UPDATE statement that includes the floor predicate in its WHERE clause:

php
$this->presenceData->newQuery()
    ->where('date', $date->toDateString())
    ->where('day', $date->dayOfWeekIso)
    ->where('registered', '>', 0)
    ->decrement('registered');

Single SQL statement, no read-modify-write split, no race window. Callers invoke the Action naively — no guard, no extra dependency, no per-site ceremony.

Pros:

  • Race-free by construction. Single conditional UPDATE; the database engine atomically evaluates the registered > 0 predicate and decrements in one operation. No TOCTOU window. Concurrent flows that both attempt to decrement against the same row are serialized at the row-level write lock, and the second flow's UPDATE simply matches zero rows when the floor predicate fails.
  • Drift detection is not lost. The UPDATE statement returns affected-row count. "Decrement attempted but no row matched" is a detectable signal — distinct from Option B-naïve's silent SELECT-skip. Higher-order monitoring (a future Pest behavior test, an opt-in logger, or post-deploy presence_data_recompute_log queries) can surface this without doctrinal commitment in this ADR.
  • No caller-side ceremony. All current and future callers of decrement Actions inherit the protection. The four call sites identified in PR #233 / #240 / EMMIE-0273 are protected simultaneously by a single Action-layer change. EMMIE-0273 collapses to a test pair, not a code change.
  • No 6th-constructor-dependency violation. The architecture test ActionTest > it should not have more than 5 constructor dependencies (tests/Architecture/ActionTest.php:226-240, hard cap <= 5) was tripped by the Option A caller-side guard requiring a PresenceData injection. Option E removes the new dependency entirely.
  • Aligns with the Action-pattern doctrine. DecrementPresenceCountAction's single responsibility is "decrement the aggregate counter for a date." A WHERE registered > 0 filter is part of the operation's semantics, not an external concern — the Action's execute() is still one SQL statement and one method.

Cons:

  • Reframes the original "drift masking" rejection of Option B. The original ADR rejected action-layer guards because of drift masking. The amendment narrative below explains why Option E does not share that concern.
  • Reverts work-in-flight. PR #240 already shipped the Option A caller-side guard. The amendment requires reverting that guard, applying the atomic-conditional inside DecrementPresenceCountAction, and reverting the UpdateVacationAction 6th-constructor injection. Six files in the territory, single revision commit.
  • Arch test scope shifts. The originally-planned DecrementActionSymmetryTest enforced caller-side guards; the amendment shifts enforcement to "the decrement Action's UPDATE statement must carry a WHERE column > 0 predicate." Functionally a unit-test or arch-test concern — discussed in §Decision §3 below.

Option comparison (post-amendment)

OptionVerdictReason
A — Caller-side + arch testSuperseded by EInitial acceptance reversed: contains TOCTOU race in the SELECT-then-decrement pair; trips the >5 deps arch rule; per-caller ceremony scales poorly.
B — SELECT-then-decrement at Action layerRejectedShares Option A's TOCTOU race; drift-masks via silent SELECT-skip.
C — Hybrid action-level + skip logRejectedSchema growth + Action responsibility creep; Option E's affected-row-count signal supersedes the skip log's value proposition without the schema cost.
D — Status quoRejectedBug class already recurred 3×; enforcement at Level 6 doctrine-violating.
E — Atomic conditional UPDATE at Action layerAccepted (2026-05-15 amendment)Race-free by construction; no caller-side ceremony; protects all 4 sibling callers; no >5-deps violation; affected-row count preserves drift-detection signal; single-statement implementation maintains Action-pattern doctrine.

Decision

Adopt Option E — atomic conditional UPDATE at the Action layer. (2026-05-15 amendment; supersedes the initially-accepted Option A.)

1. Decrement Action contract

Decrement Actions of this class carry the floor predicate inside their UPDATE statement. Single SQL operation. The Action's execute() remains one method, one statement, single-responsibility:

php
final readonly class DecrementPresenceCountAction
{
    public function __construct(private PresenceData $presenceData) {}

    public function execute(CarbonImmutable $date): void
    {
        $this->presenceData->newQuery()
            ->where('date', $date->toDateString())
            ->where('day', $date->dayOfWeekIso)
            ->where('registered', '>', 0)
            ->decrement('registered');
    }
}

The WHERE registered > 0 predicate is part of the UPDATE statement itself. MySQL's row-level write lock is acquired at UPDATE-time and the predicate is evaluated against current-read (not snapshot-read), so concurrent decrement attempts against a row at registered = 0 atomically result in zero affected rows — no race, no underflow, no silent SELECT-then-skip gap.

The where('day', $date->dayOfWeekIso) filter mirrors the existing (date, day) composite identifying a presence_data row; both predicates are required for the UPDATE to match the intended row.

2. Caller invocation pattern

Callers invoke decrement Actions naively — no pre-read, no guard, no transaction-scoped lookup:

php
foreach ($toBeDeletedDailyRegistrations as $toBeDeletedDailyRegistration) {
    $this->decrementPresenceData->execute($toBeDeletedDailyRegistration->starts_at);
    $toBeDeletedDailyRegistration->forceDelete();
}

The terminal row operation (forceDelete, etc.) stays unconditional — row deletion and count bookkeeping remain separable concerns. Callers do not need to inject PresenceData (or any aggregate model) solely for the purpose of guarding the decrement call.

3. Enforcement

The doctrine enforces at the Action layer, not the caller. Two complementary checks:

§3.a — Pest behavior test on each decrement Action. Author one unit test per Decrement*Action asserting the "decrement against column = 0 produces zero affected rows and does not underflow" invariant. Canonical shape (emmie):

php
it('should not underflow when registered is already zero', function (): void {
    PresenceData::factory()->create(['date' => '2026-06-01', 'day' => 1, 'registered' => 0]);
    expect(fn () => app(DecrementPresenceCountAction::class)->execute(CarbonImmutable::parse('2026-06-01')))
        ->not->toThrow(QueryException::class);
    expect(PresenceData::where('date', '2026-06-01')->value('registered'))->toBe(0);
});

This test pins the contract at the Action's behavior level — what the Action actually does on the zero-row case. Stronger than a regex scan; failure mode is empirical, not structural.

§3.b — Pest architecture test tests/Architecture/DecrementActionSymmetryTest.php rejects caller-side SELECT-then-decrement patterns. The arch test scans backend/app/ for the failure mode the amendment closes: a $this->*PresenceData->newQuery()->where(...)->first() followed by a conditional call to $this->decrement*->execute(...) in the same method scope is the disallowed shape. Allow-list mechanism: per-line @see ADR-0026 markers grant exemption where the lookup is provably needed for a different reason (e.g., reading the value for an audit log entry, not for guarding the decrement). Marker grants are reviewer-discipline gates.

The arch test additionally asserts each Decrement*Action::execute() implementation contains the where(.., '>', 0) predicate against its aggregate column. Inverting (adding a SELECT-then-conditional inside the Action) is doctrine violation and CI-rejected.

4. Increment-side counterpart

Out of scope. The symmetric over-increment bug class has not manifested. A separate ADR amendment may follow if/when it does.

5. Drift detection

The atomic conditional preserves drift detection without requiring a skip-log schema. Affected-row count is available from the QueryBuilder return value; behavior-test §3.a pins the zero-rows case as expected; recompute-audit infrastructure (e.g., EMMIE-0271's presence_data_recompute_log) catches residual drift from upstream asymmetric paths. Higher-order monitoring (logging the affected-row-count from Decrement*Action::execute(), alerting on sustained high skip rates) is opt-in and not mandated by this ADR; territories with strict ISO 27001 / NEN 7510 audit requirements may opt in via a thin wrapper or middleware, governed in their own territory doctrine.

Consequences

Positive

  • Race-free by construction. The atomic conditional UPDATE eliminates the TOCTOU window the caller-side Option A inherited. Concurrent decrement attempts on a row at the floor result in zero affected rows on the racing flow; no underflow, no exception, no data corruption.
  • All four current sibling callers protected by a single Action-layer change. PR #233 (DeleteVacationAction), PR #240 (UpdateVacationAction), EMMIE-0273 (ForceDeleteDailyRegistrationAction), and the fragile-but-not-currently-reachable UndoMarkUneditedRegistrationsAsLocationClosedAction all inherit the floor with no caller-side change. EMMIE-0273's scope collapses to its integration test pair.
  • No 6th-constructor-dependency violation. The original Option A required PresenceData injection at each caller site, which tripped tests/Architecture/ActionTest.php:226-240's <= 5 cap on UpdateVacationAction. Option E removes the new dep entirely; the architecture test continues to pass without an allowlist entry.
  • Drift detection signal preserved. The atomic UPDATE returns affected-row count; "decrement attempted but no row matched" remains observable for any future operational monitoring layer (an opt-in log wrapper, a Pest behavior test, post-deploy recompute-log queries) without baking a skip-log schema into doctrine.
  • Action-pattern doctrinal alignment. DecrementPresenceCountAction::execute() remains a single SQL statement, single responsibility ("conditionally decrement the aggregate counter for a date"). The > 0 predicate is part of the operation's semantics, not an external concern.
  • Doctrine enforces at the layer where the bug originates. Three of the four sibling-caller bugs all manifested at the same DecrementPresenceCountAction::execute() SQL boundary. Fixing the boundary closes the bug class; pushing enforcement to callers (Option A) duplicates the defense across N sites and creates per-site failure modes.

Negative

  • Reverses work-in-flight. PR #240 already shipped Option A's caller-side guard (commit 8ffae54f44). The amendment requires reverting the guard, applying the atomic-conditional inside DecrementPresenceCountAction, and reverting the UpdateVacationAction 6th-constructor injection. ~6 files changed in territory; single revision commit on the existing branch.
  • Reframes the original "drift masking" rejection. The 2026-05-15-AM analysis rejected action-layer guards (Option B) on the grounds that internal floor checks mask drift. That analysis was correct for Option B-naïve (SELECT-then-skip silently) and wrong as a generalization — the atomic-conditional shape was not considered. The amendment narrative below captures the correction.
  • Arch test scope shifted, not eliminated. The originally-planned DecrementActionSymmetryTest enforced caller-side guards. The amended scope enforces (i) the atomic-conditional shape in each Decrement*Action::execute() implementation, and (ii) rejection of caller-side SELECT-then-decrement patterns. Functionally similar effort; different regex.
  • Drift detection still requires recompute infrastructure for historical residue. The atomic guard prevents new underflows from shipping but does not detect drift from pre-doctrine bugs. EMMIE-0271's recompute migration handles that residue.
  • Cross-territory transferability is universal in reasoning but emmie-specific in current footprint. When a second territory introduces a counter-mutation Action, the §3 arch test must be ported and the canonical Action shape replicated.
  • F-1/F-2 from the 2026-05-15 Quartermaster sweep remain separate. Pure-mock test architecture concerns (the seed case at GenerateVacationRegistrationsActionTest:171/:370) are not addressed by this ADR. F-2's DailyRegistration integration-pair backfill (Quartermaster prevention multiplier 3.0×) is the complementary enforcement layer ADR-0026 §3 does not cover.

Amendment Log

2026-05-26 — §D.1 scan target retired. EMMIE-0296 PR2 deleted App\Actions\Model\PresenceData\DecrementPresenceCountAction and its arch test tests/Architecture/DecrementActionSymmetryTest.php. The ADR's general principle (atomic-conditional decrement on aggregate counters) remains in force for any future Decrement*Action against an aggregate counter column. The named example simply no longer has a live implementation pinned to it.

2026-05-15-PM — Option E supersedes Option A. PR #240's reviewer (Krypt0nBull3t) surfaced a TOCTOU race in the caller-side SELECT-then-decrement pattern that Option A required (UpdateVacationAction:91-109 post-EMMIE-0272 commit 8ffae54f44). MySQL InnoDB REPEATABLE READ: the SELECT inside the surrounding transaction takes a snapshot but does not lock the row; UPDATE uses current-read; two concurrent transactions can both pass the guard, both apply the decrement, the second underflows. The reviewer proposed the atomic-conditional refinement (single UPDATE with WHERE registered > 0), which closes the race at the SQL layer, removes the new constructor dep that tripped the >5 deps arch rule, and protects all four sibling callers in one shot.

The General's original ADR analysis missed the concurrency lens. The doctrinal rejection of "action-level guards" implicitly assumed a SELECT-then-skip implementation and conflated it with the drift-masking property of silent absorption. The atomic conditional is a third variant — race-free AND drift-signal-preserving (via affected-row-count introspection). Once Option E was considered explicitly, the comparison against Option A was structurally one-sided.

The amendment was deliberated within hours of the original acceptance. Status remains Accepted; the ADR is canonically Option E from this point forward. Option A is preserved in §Options Considered for narrative continuity.

Implementation

Phase 1 — Amendment-revision retrofit on PR #240

  1. Revert the caller-side guard in UpdateVacationAction:91-109 (added in PR #240 commit 8ffae54f44). Remove the 6th PresenceData constructor argument; remove the cascade in tests/Unit/.../UpdateVacationActionTest.php. Revert the GenerateVacationRegistrationsActionTest:129 caption per the reviewer's minor comment.
  2. Apply the atomic conditional to DecrementPresenceCountAction::execute() per §Decision §1.
  3. Add the §3.a behavior test on DecrementPresenceCountActionTest — the zero-row no-underflow invariant.
  4. Author tests/Architecture/DecrementActionSymmetryTest.php per §Decision §3.b. Asserts the atomic-conditional shape on each Decrement*Action::execute() and rejects caller-side SELECT-then-decrement patterns. EMMIE-0272's integration test pair (Edges 1/2/3/4/6) should remain unchanged and continue to pass — the bug class is closed at a different layer; the end-to-end count invariant the integration tests pin is unchanged.
  5. Verify EMMIE-0273 closes in the same patch. ForceDeleteDailyRegistrationAction:30 is now protected by the Action-layer atomic conditional with no caller-side change. EMMIE-0273's test pair (the underflow regression integration test) may either ship in PR #240 or in a follow-up ticket-pair PR — Medic discretion based on PR #240's resulting size.

Phase 2 — Quartermaster sweep integration

  1. F-2 (DailyRegistration integration-pair backfill, Quartermaster prevention multiplier 3.0×) and F-5 (Level 1 integration-pair-existence arch test) remain separate workstreams complementary to this ADR.

Phase 3 — Cross-territory propagation

  1. When a second territory introduces a counter-mutation Action, port the atomic-conditional shape and the §3 arch test.

CLAUDE.md projection (per ADR-0015) — amended

Add to the territory's CLAUDE.md under "War Room ADR Projections":

markdown
### ADR-0026: Decrement Action Symmetry Discipline

> **Status:** Accepted 2026-05-15 — atomic-conditional shape (Option E) per amendment same day. Phase 1 retrofit lands via the PR #240 revision; closes EMMIE-0273 in the same patch via Action-layer protection.

- **Rule:** Decrement Actions of the counter-mutation class (`Decrement*PresenceCountAction`, `Decrement*Action` for any aggregate counter) carry the floor predicate INSIDE the UPDATE statement itself: `->where('column', '>', 0)->decrement('column')`. Single SQL operation; race-free by construction; no read-modify-write split.
- **Rule:** Callers invoke decrement Actions naively. No pre-read, no `lockForUpdate`, no `if (column > 0)` guard at the call site. The Action's UPDATE handles the floor atomically.
- **Rule:** `tests/Architecture/DecrementActionSymmetryTest.php` enforces (i) the atomic-conditional shape on each `Decrement*Action::execute()` implementation, and (ii) rejection of caller-side `$model->newQuery()->where(...)->first()` followed by conditional `$this->decrement*->execute()` patterns. Per-line `@see ADR-0026` markers grant exemption where the lookup serves a different purpose (e.g., audit log entry).
- **Rule:** Each `Decrement*Action` carries a Pest behavior test pinning the zero-row no-underflow invariant: `it('should not underflow when {column} is already zero')`. Asserts no exception thrown and column value unchanged.
- **Why:** The atomic UPDATE closes the TOCTOU window a SELECT-then-decrement pair leaves open (the failure mode the 2026-05-15-PM amendment was triggered by). Drift detection is preserved via affected-row-count introspection — Option E does not silently absorb; it succeeds-with-zero-rows-affected, which is observable for monitoring layers. Action-layer protection covers all current and future callers in one place, satisfying the enforcement-escalation-ladder principle of "fix at the lowest level where the bug originates."

Architecture documentation for contributors and collaborators.