[AchoArnold]

Summary / SPEC decisions

• Retain MongoDB heartbeat history for 60 days (approximately two months) with the named single-field BSON-date TTL index heartbeats_retention; preserve the compound history query index.
• Stop monitoring when history is missing. A newly stored heartbeat can restart inactive monitoring. Restart failures explicitly log, preserve the saved heartbeat response, and continue online handling.
• Keep empty-queue eligibility in HeartbeatMonitor.RequiresCheck(). Active monitoring must not acquire duplicate restart chains.
• Scope: heartbeat retention/lifecycle, directly coupled repositories/events/queue behavior, focused regressions, and the requested Clean Code/SOLID repository instructions. No merge, Dependabot changes, dependency changes, or extra CAS index.

Lifecycle correctness

Database-atomic queue-marker CAS coordinates replicas. Committed, pending and leased running ownership distinguish acknowledged successors from unfinished work. Failed finalization/claim takeover remains retryable; a restored incoming marker is not accepted as its own committed successor. Missing-history stop does not resurrect an acknowledged marker and rechecks for an overlapping returning heartbeat.

Running workers reload ownership and current history after missed-heartbeat dispatch. Superseded workers cannot continue from stale snapshots. Restart also checks current persisted history before queuing.

Internal heartbeat-version CAS records online arrivals and conditionally applies offline transitions independently of check lease timestamps. GORM uses a context-bounded row-lock transaction returning the genuine pre-update state; MongoDB uses FindOneAndUpdate returning the prior document. Queue CAS filters _id and expected queue_id; version updates additionally filter user/expected heartbeat identity. MongoDB's built-in unique _id_ index already narrows these to one document. The existing user_id/owner index remains for owner queries.

Offline delivery and retry correctness

A synchronous offline filter validates monitor identity, latest history and heartbeat identity before any email/webhook subscriber. The monitor version is observed before reading history; a newer recorded arrival changes the atomic offline predicate instead of being overwritten. Missing, deleted, stale or unverifiable events do not reach subscribers. Guard errors propagate for durable endpoint retry; unrelated subscriber error behavior is unchanged. Legacy payloads without identity are accepted only before a modern version has been recorded and while timestamps match.

Named Cloud Tasks reuse a deterministic ID for retries of the same check/heartbeat; AlreadyExists is successful idempotent creation. Deadline failures do not fall back to non-durable local publication for checks/named alerts. Emulator deliveries retry failed HTTP/transport attempts with exponential backoff and a fresh deadline, retaining task identity; the emulator remains explicitly process-local/non-durable.

Equal MongoDB millisecond timestamps select the lowest UUID consistently with GORM First ordering. Last fetches two timestamp-ordered candidates using the existing history index; only ties require another lookup bounded to that exact timestamp. No full-history secondary sort or additional index is introduced.

This linearizes monitor-state transitions, not distributed exactly-once external delivery or physical email/webhook ordering. A new arrival after an accepted offline transition legitimately creates a later online transition. Best-effort urgent FCM behavior remains unchanged.

Clean Code / SOLID

Heartbeat orchestration uses short focused private helpers for distinct responsibilities. The requested general rule is persisted in .github/copilot-instructions.md without changing other instructions or overengineering interfaces.

Validation evidence

Current pushed head: 22adc510821d4317a1e4304486d6548ca265e666 (finishing commits bc73e427 and 22adc510).

• Affected entity/event/listener/service/repository/DI suites and vet passed; DI compiled. gofumpt, whitespace checks and normal hooks passed.
• Incremental golangci-lint v2.12.2 against 9763037e: 0 new issues; full baseline lint is not claimed clean.
• Deterministic regressions exercise returning heartbeat/lease takeover, delayed offline fanout and history/status interleavings, concurrent arrivals, fail-open recording, missing-history restart prevention, finalization/claim-loss recovery, and retry failures.
• GORM regressions verify ownership/version/user predicates, row locking, previous status, bounded context and unchanged check lease timestamps. Real driver/net.Pipe regressions cover reversed insertion order, more than two timestamp ties, older records and missing/single history. Named-task bufconn RPC tests cover retry/name collisions.
• Current-head CI: 10 SUCCESS, including Integration Tests, Codacy, CodeQL/three Analyze jobs, Validate, GitGuardian, Greptile and Copilot; two Deploy SKIPPED. API integration run 41323433874 passed against the test-stack services, including heartbeat storage/indexing, adapter heartbeat wakeup, encrypted SMS and outstanding SMS.
• Codacy check 124898522115: SUCCESS, zero annotations. The only transport exception is a supported line-local Semgrep rule annotation on the bufconn-only test server; both server and client dialer are in-memory, with no network socket. No global security rule/threshold/application transport was weakened.
• Live Cloud Tasks infrastructure and a literal 60-day wall-clock cleanup run are not claimed; deterministic interleavings/protocol tests and the database-atomic operation contracts establish the implemented invariants.

Review outcome

GitHub finishing cycle: 2 of 5 authorized rounds used; no review timeout/pass limit reached. All 20 PR threads are resolved (19 addressed during this run: 15 Copilot, four Greptile). Fresh current-head Greptile check 124898584905 succeeded; summary 6558182897, updated 2026-11-01 15:55:57 UTC, explicitly reviews this SHA at 5/5, no actionable findings. Fresh Copilot review 5728941942, submitted 15:59:28 UTC, reviews the same SHA: No open issues. All inline sources and paginated review threads/comments were checked.

Copilot's general sentence about pending integration used pre-completion review context; the current-head integration result is now verified SUCCESS. No additional concrete code defect was supplied in that review. Actual test/deployment limits are stated above rather than claiming exactly-once or live-infrastructure coverage.

Previous CI investigation (outside heartbeat scope)

On unchanged bc73e427, API run 41320306100 failed OutstandingFlow on attempt 1 and Encrypted on attempt 2, both stuck in sending after SENT. A deterministic diagnostic reproduced the existing lost update: pause HandleMessageSending after its loaded snapshot; complete HandleMessageSent; resume sending; the repository blindly saves the stale sending snapshot. These message service/repository paths have zero diff from main. The diagnostic was removed, and no unrelated SMS behavior/test changes were made. Current-head integration subsequently passed both tests; the pre-existing race is disclosed, not silently rewritten or hidden by repeated reruns.

[codacy-production]

Not up to standards ⛔

🔴 Issues 1 critical

Alerts:
⚠ 1 issue (≤ 0 issues of at least minor severity)

Results:
1 new issue

Category	Results
Security	1 critical

View in Codacy

🟢 Metrics 183 complexity · 152 duplication

Metric	Results
Complexity	183
Duplication	152

View in Codacy

_{NEW Get contextual insights on your PRs based on Codacy's metrics, along with PR and Jira context, without leaving GitHub. Enable AI reviewer
TIP This summary will be updated as you push new changes.}

[greptile-apps]

Confidence Score: 5/5

[High impact] The reviewed changes appear safe to merge; no new blocking issue was found.

Summary

Adds 60-day MongoDB heartbeat retention and keeps monitoring recoverable when history expires.

• MongoDB keeps 60 days of heartbeat history and picks tied latest records consistently.
• Heartbeat checks keep one current owner across retries and restarts.
• Offline alerts reach subscribers only for the heartbeat they checked.
• Heartbeat queue retries keep failed checks and avoid duplicate alert tasks.
• Contributor guidance now asks for short, focused code and small interfaces.

Diagram

%%{init: {'theme': 'neutral'}}%%
flowchart TD
  A[Heartbeat arrives] --> B[Save history and record heartbeat identity]
  B --> C{Monitor needs a check?}
  C -->|Yes| D[Claim and queue a check]
  D --> E{History exists?}
  E -->|No| F[Clear owned marker and recheck history]
  E -->|Yes| G[Check age and queue successor]
  G --> H[Queue offline event when late]
  H --> I[Check current history and heartbeat identity]
  I -->|Still current| J[Mark offline before notifying subscribers]

Loading

_{Reviews (10) · Last reviewed commit: "fix(api): retry emulator checks and stab..." · Reviewed by Greptile}

[Copilot]

🟢 Approval recommended

The MongoDB heartbeat retention change has no active review issues.

No open issues 🎉

What changed in this PR

Adds automatic 60-day retention for MongoDB heartbeat records through a TTL index created during API startup.

Changes:

• Adds the named heartbeats_retention index on timestamp, allowing MongoDB to remove expired records in the background, including existing records.
• Preserves the existing compound index used for heartbeat queries.

File	Description
api/​pkg/​repositories/​mongodb.go	Adds a single-field TTL index with a 60-day expiration period.

_{Review effort: Balanced}

[AchoArnold]

@greptileai please review the latest changes again. Heartbeat history expiration stops checks, new heartbeats restart inactive monitors, restart failures are logged while returning the saved heartbeat, and RequiresCheck includes the empty queue marker.

[greptile-apps]

Want your agent to iterate on Greptile's feedback? Try greploops.

[Copilot]

🟡 Changes recommended

Unresolved heartbeat restart races and enqueue-failure handling can create duplicate monitoring chains or leave monitors without active checks, causing duplicate or missed offline alerts.

3 open issues

• Concurrent stores can enqueue duplicate monitoring chains · New
• Enqueue failures can suppress offline-check retries for two hours · New
• Clearing the marker can leave a disconnected phone unmonitored · New

_{Review effort: Balanced}

[AchoArnold]

@greptile review

[AchoArnold]

@greptile review

[AchoArnold]

@greptile review

[Copilot]

🟡 Changes recommended

Unresolved ownership, crash-recovery, and retry gaps can lose or strand heartbeat checks, leaving phones unmonitored and causing missed offline alerts.

5 open issues

• Retry expired restart-only claims · New
• Propagate enqueue failures from the fallback · New
• Reject legacy tasks after fenced-check migration · New
• Allow redelivery to recover a cleared queue marker · New
• Retry when history-check ownership changes · New

3 resolved since last review

• Concurrent stores can enqueue duplicate monitoring chains
• Clearing the marker can leave a disconnected phone unmonitored
• Enqueue failures can suppress offline-check retries for two hours

_{Review effort: Balanced}

[AchoArnold]

@greptile review

[Copilot]

🟡 Changes recommended

Acknowledging tasks before a pending restart has a durable successor can leave phones unmonitored if enqueueing fails or the process crashes.

1 open issue

• Keep restart delivery retryable until successor enqueue commits · New

5 resolved since last review

• Retry when history-check ownership changes
• Allow redelivery to recover a cleared queue marker
• Reject legacy tasks after fenced-check migration
• Propagate enqueue failures from the fallback
• Retry expired restart-only claims

_{Review effort: Balanced}

[AchoArnold]

@greptile review

[Copilot]

🟡 Changes recommended

Critical marker and lease-finalization races can acknowledge heartbeat checks without a recoverable queued successor, stalling monitoring and preventing later heartbeats from restarting it.

4 open issues

• Do not restore the marker after a failed history check · New
• Return a retryable error when finalization fails or loses ownership · New
• Reject the incoming check's restored marker as a committed successor · New
• Prevent duplicate offline alerts when successor enqueueing fails · New

1 resolved since last review

• Keep restart delivery retryable until successor enqueue commits

_{Review effort: Balanced}

[AchoArnold]

@greptile review

[Copilot]

🔵 Needs a closer look

Human assessment is needed because lease-takeover and duplicate-delivery races can publish stale offline events or leave heartbeats without queued checks.

No open issues 🎉

4 resolved since last review

• Reject the incoming check's restored marker as a committed successor
• Return a retryable error when finalization fails or loses ownership
• Do not restore the marker after a failed history check
• Prevent duplicate offline alerts when successor enqueueing fails

_{Review effort: Balanced}

[AchoArnold]

Authorized additional pass ended on 9763037: all four targeted findings are fixed and resolved, and all 18 review threads are resolved. Fresh Greptile review is 5/5; fresh Copilot review confirms no open issues but retains its human-assessment caveat. The original ten-minute deadline was preserved; no further round or merge was started.

Codacy still requires action on the bufconn-only test server at api/pkg/services/google_cloud_push_queue_service_test.go:77. It is not network-exposed; this needs manual triage or a separately authorized harness adjustment, not an overridden check. Copilot's general stale-alert/lease concern is also preserved in the PR description rather than ignored despite zero inline findings. All affected package tests, vet, formatting, whitespace checks, incremental lint and normal hooks passed; ten CI checks passed and two deployments were skipped. The worktree is clean and pushed; no redundant MongoDB index or Dependabot change was added.

[codacy-production]

Up to standards ✅

🟢 Issues 0 issues

Results:
0 new issues

View in Codacy

🟢 Metrics 357 complexity · 207 duplication

Metric	Results
Complexity	357
Duplication	207

View in Codacy

_{NEW Get contextual insights on your PRs based on Codacy's metrics, along with PR and Jira context, without leaving GitHub. Enable AI reviewer
TIP This summary will be updated as you push new changes.}

[AchoArnold]

Concrete assessment and fixes for the latest Copilot general warning are now in bc73e42 and documented in the PR description: revalidated lease ownership/history, heartbeat-version CAS before offline fanout, and deterministic interleaving/RPC regressions. Online/offline state transitions are linearized; exactly-once external delivery is not claimed. The single bufconn test warning uses a rule-specific line annotation with in-memory-only evidence. Checking fresh current-head CI and reviews before declaring completion.

[AchoArnold]

@greptile review

[Copilot]

🟡 Changes recommended

Emulator delivery failures lack retries and can block recovery until the two-hour stale threshold, while nondeterministic selection of equal-timestamp heartbeats can discard valid offline alerts.

2 open issues

• Retry failed emulator event deliveries with backoff · New
• Break ties deterministically when selecting the latest heartbeat · New

_{Review effort: Balanced}

[AchoArnold]

Both new Copilot threads are fixed, replied to and resolved in 22adc51. Focused and affected package tests, vet, formatting/whitespace checks, incremental lint (0 new issues) and normal hooks passed. Integration was concretely investigated: unchanged bc73e42 rerun shifted the sending-after-SENT failure from OutstandingFlow to Encrypted. A deterministic interleaving reproduces the existing blind-save SMS race in paths unchanged from main; no unrelated SMS changes were made. Verifying current-head integration and fresh bot reviews before completion.

[AchoArnold]

@greptile review

[Copilot]

🔵 Needs a closer look

Current-head integration results are pending, leaving cross-replica lease recovery, MongoDB TTL cleanup, PostgreSQL concurrency, and Cloud Tasks behavior unvalidated beyond simulated backends.

No open issues 🎉

2 resolved since last review

• Break ties deterministically when selecting the latest heartbeat
• Retry failed emulator event deliveries with backoff

_{Review effort: Balanced}

[AchoArnold]

Verified finishing result on 22adc51: all 10 current-head checks SUCCESS (including Integration Tests and Codacy with zero annotations), two Deploy jobs SKIPPED. Greptile 5/5 explicitly reviews this SHA; fresh Copilot review 5728941942 reports No open issues, and all 20 threads are resolved. Its pending-integration sentence is superseded by successful run 41323433874; actual live-infrastructure testing limits and the unchanged pre-existing SMS race investigation are documented, without claiming exactly-once delivery. Worktree and remote head match; no merge or #1059 changes. Completed two of this finishing cycles five authorized rounds, without timeout.

[AchoArnold]

Final verification for current head 22adc51: Integration Tests has now completed successfully, so the pending-integration note in the latest Copilot summary is no longer current. All non-deployment checks pass; the two deployment jobs are intentionally skipped for this PR. Greptile reports 5/5 for this exact head, Copilot has no open issues, and all 20 review threads are resolved. The branch is pushed and the worktree is clean. PR remains unmerged.