Skip to content

fix(api): reject nonfinite heartbeat pulse windows - #207

Merged
ErikBjare merged 1 commit into
ActivityWatch:masterfrom
TimeToBuildBob:fix/nonfinite-heartbeat-pulsetime
Nov 7, 2026
Merged

ErikBjare merged 1 commit into
ActivityWatch:masterfrom
TimeToBuildBob:fix/nonfinite-heartbeat-pulsetime

Conversation

@TimeToBuildBob

Copy link
Copy Markdown
Contributor

Summary

Reject non-finite pulsetime at the heartbeat REST boundary, before lock acquisition or heartbeat processing, using the existing JSON BadRequest pattern. No merge-algorithm or negative-pulse policy change.

Reproduction

On fetched master 994ea988, seed a heartbeat and send another with identical data one second later. pulsetime=inf, -inf, nan or 1e999 passes float parsing and returns 500 during timedelta conversion. In an empty bucket the same non-finite values return 200, so validation was path-dependent.

The eight new seeded/empty Flask regressions all failed before the three-line source repair. They now require JSON 400, unchanged stored events and a successful finite follow-up; seeded controls also verify normal same-data merging.

Verification

  • poetry run pytest tests/test_input_validation.py -q: 22 passed.
  • poetry run pytest tests/test_api_heartbeat.py tests/test_input_validation.py -q: 23 passed.
  • Package-native offline poetry run make test: 144 passed. This target does not enumerate test_input_validation.py, so it was explicitly run separately; no live-client integration tests were run.
  • poetry run make typecheck: 31 source files clean.
  • Changed files pass Black and Ruff; git diff --check clean.

Synthetic Flask test client and default MemoryStorage only, no live server or personal data. Existing malformed-string handling remains covered; numeric event timestamps and CSV recovery are outside this change. Independent of #206's query-timeperiod validation. Merge and master symptom replay remain the tracked completion gate.

@greptile-apps

greptile-apps Bot commented Oct 28, 2026 •

Copy link
Copy Markdown

RetriggerView in GreptileConfidence Score: 5/5

[Medium impact] The PR appears safe to merge; no actionable issues were found at the current head.

Summary

Rejects non-finite pulsetime values at the heartbeat REST boundary.

  • Heartbeat requests reject non-finite pulse windows before processing.

Reviews (3) · Last reviewed commit: "fix(api): reject nonfinite heartbeat pul..." · Reviewed by Greptile

@TimeToBuildBob

Copy link
Copy Markdown
Contributor Author

Fixed the Linux CI setup failure in d164a0d. Both jobs failed before reaching project code because ubuntu-latest now selects Ubuntu 26.04, where setup-python has no Python 3.9 build. The Linux test and typecheck jobs now use ubuntu-22.04; Python 3.9, Windows/macOS coverage, and all build/test/typecheck steps are unchanged.

Validated the YAML and confirmed those runner labels are the only semantic workflow changes. Re-ran the heartbeat/input-validation tests locally: 23 passed. Replacement hosted checks are starting, not yet verified green.

@TimeToBuildBob

Copy link
Copy Markdown
Contributor Author

CI-green and mergeable (Greptile 5/5) — waiting only on a maintainer click.

This PR is ready to merge, but the bot has pull-only access to this repo and can't self-merge — surfacing it here so it isn't lost. The monitoring loop will stop re-flagging it now that this note is posted.

@TimeToBuildBob

TimeToBuildBob commented Oct 28, 2026 •

Copy link
Copy Markdown
Contributor Author

🤖 AI code review

aw_server/rest.py imports math and rejects non-finite pulsetime values with BadRequest before acquiring the heartbeat lock or calling ServerAPI.heartbeat. tests/test_input_validation.py adds eight parameterized cases covering non-finite values in empty and seeded buckets, checking rejection, unchanged stored events, and finite follow-up requests.

Safe to merge — no P0/P1 findings

Confidence 5/5

✅ No findings. The diff looks correct to me on this pass.

Files changed (2) — the diff as I read it
  • aw_server/rest.py — Adds a math.isfinite check for parsed heartbeat pulsetime values and raises InvalidParameter when the value is non-finite.
  • tests/test_input_validation.py — Adds test_heartbeat_nonfinite_pulsetime for inf, -inf, nan, and 1e999 with empty and seeded buckets, including storage comparisons and finite follow-ups.
Previous review passes
commit score findings engine when
d164a0d1c892 5/5 0 subscription 2026-10-28 16:42 UTC

Reviewed e2c9f47c9480 · panel: codex:gpt-6.1-sol + claude-code:sonnet · subscription engine · 19s · about this reviewer

Maintainer commands

@TimeToBuildBob review (own line) — fresh review · @TimeToBuildBob fix — a worker acts on the findings. Once per comment; 👀 = received.

Git-Session-Id: 18fece4b-f2e9-4ea7-93f5-5eee59badef2
@TimeToBuildBob
TimeToBuildBob force-pushed the fix/nonfinite-heartbeat-pulsetime branch from d164a0d to e2c9f47 Compare November 6, 2026 15:34
@TimeToBuildBob

Copy link
Copy Markdown
Contributor Author

Rebased onto master and pushed e2c9f47 with an explicit lease. The conflict was only the old CI-runner commit: dropped it in favor of master's ubuntu-24.04 pin, retaining master's checkout upgrade too. The heartbeat validation and all eight non-finite pulse-window regression cases are unchanged (git range-diff confirms the feature commit is patch-identical).

Verification after rebase: heartbeat/input-validation tests 23 passed; package-native offline suite 144 passed; mypy 31 source files clean; git diff --check clean. GitHub now reports the PR mergeable; replacement CI is running, not yet verified green.

@TimeToBuildBob

Copy link
Copy Markdown
Contributor Author

@greptileai review

@ErikBjare
ErikBjare merged commit 7af1920 into ActivityWatch:master Nov 7, 2026
9 checks passed
TimeToBuildBob added a commit to TimeToBuildBob/aw-server that referenced this pull request Nov 11, 2026
Git-Session-Id: 18fece4b-f2e9-4ea7-93f5-5eee59badef2
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.

2 participants