next-version-code.sh: derive the candidate from the commit, not the build clock #8

Merged
claude merged 1 commits from committer-date-version-code into main 2026-09-09 09:51:22 +00:00
Member

Problem

fdroid-repo-ci#6's "Superseded" sections change the versionCode rule: the
candidate must be derived from the committer date of the commit being
built
, not from the build clock, and clamped on both sides (bit,
approved on weblib-viewer#15: "Design sounds good").

Deriving from the build clock meant the result depended on when a runner
happened to pick the job up, not on the commit itself -- and a pure
clock-derived scheme cannot detect two cases that matter:

  • Backwards. Rebase, amend and cherry-pick can all produce a commit
    older than one already published.
  • Forwards. A commit's timestamp is attacker- and typo-controlled and
    the version code only ever rises, so a bad one can never be walked back.
    A value placed just below the int32 ceiling would still publish, and then
    no later build could ever exceed it -- the app becomes permanently
    un-updatable.

What changed

candidate = minutes_since_2026_01_01(committer_date)
candidate = min(candidate, minutes_since_2026_01_01(now) + TOLERANCE)   # forwards
result    = max(candidate, published + 1)                               # backwards
  • next_version_code.py now takes the commit's committer timestamp as
    a third CLI argument (Unix seconds -- what git log -1 --format=%ct
    prints), rather than reading the system clock. Committer date, not
    author date, on purpose: author date survives a rebase unchanged, so a
    rebased branch would keep replaying its original, now-stale timestamp
    and the backwards clamp would never see it move.
  • The script does not shell out to git itself -- the caller already knows
    its own commit, which keeps the module pure and testable.
  • TOLERANCE_MINUTES (6 hours) absorbs honest clock skew between the
    committer's machine and the runner. A commit beyond it is clamped and
    a warning is printed on stderr, never rejected outright
    -- rejecting
    would block a legitimate build over a few minutes of innocent skew, and
    the clamp already removes the actual harm.
  • next-version-code.sh, README.md and action.yml updated to match
    the new three-argument interface and the reasoning above (kept as
    comments/prose on purpose, per the issue: this is the part that gets
    quietly "simplified" away later).
  • Granularity stays minutes. Concurrency is handled separately (a
    workflow-level concurrency group, plus publish.sh's existing
    refuse-on-existing-APK check), so granularity is now a pure headroom
    choice at 4,082 years, not something this change touches.

Testing

Offline, against fixtures, no network, no live bucket:

bash run_tests.sh

98 assertions, 0 failed, 4 skipped (pre-existing, live-bucket-only
test_publish.sh cases), up from 91 before this change.

test_next_version_code.py gained the five properties this rule actually
depends on, each its own test:

  • a normal commit yields its own minute
  • a commit older than the published maximum still yields published + 1
  • a future-dated commit within tolerance passes through untouched; one
    beyond it is clamped to roughly now and warns on stderr
  • a commit dated near the int32 ceiling (year 2999) does not produce a
    near-int32 result
  • a dedicated property test checking the result always strictly
    exceeds
    every versionCode already in the fixture, across all of the
    above scenarios in one place -- the property that actually matters, and
    the one a future refactor could quietly break while every individual
    case still looks right

test_next_version_code.sh gained the same coverage through the real CLI
(argument count is now three; a far-future commit clamped and warning on
stderr through the real subprocess; a non-numeric committer timestamp
refused rather than silently coerced).

Fault injection, each verified to fail the relevant test and then
reverted (confirmed identical to the pre-fault file by diff):

  1. Removed the backwards clamp (published + 1 → bare candidate) --
    test_commit_older_than_published_max_still_yields_published_plus_one
    and the property test both failed.
  2. Removed the forwards clamp entirely -- the future-dated and
    near-int32 tests both failed.
  3. Ignored the commit's date and used now instead --
    test_normal_commit_yields_its_own_minute failed.

Surprises along the way

  • My own first draft of test_next_version_code.sh had a real bug: a
    hardcoded committer timestamp meant to be 2026-06-01 was actually
    2025-06-01 (a year off), which landed before the script's 2026-01-01
    epoch and produced a negative candidate (-308160) -- caught by the
    "first-ever publish" assertion, fixed, and the whole shell suite is now
    pinned to a fixed NEXT_VERSION_CODE_NOW rather than depending on the
    real wall clock, so it will not go stale or flake the same way again.
## Problem fdroid-repo-ci#6's "Superseded" sections change the versionCode rule: the candidate must be derived from the **committer date of the commit being built**, not from the build clock, and clamped on **both** sides (bit, approved on weblib-viewer#15: "Design sounds good"). Deriving from the build clock meant the result depended on when a runner happened to pick the job up, not on the commit itself -- and a pure clock-derived scheme cannot detect two cases that matter: - **Backwards.** Rebase, amend and cherry-pick can all produce a commit older than one already published. - **Forwards.** A commit's timestamp is attacker- and typo-controlled and the version code only ever rises, so a bad one can never be walked back. A value placed just below the int32 ceiling would still publish, and then no later build could ever exceed it -- the app becomes permanently un-updatable. ## What changed ``` candidate = minutes_since_2026_01_01(committer_date) candidate = min(candidate, minutes_since_2026_01_01(now) + TOLERANCE) # forwards result = max(candidate, published + 1) # backwards ``` - `next_version_code.py` now takes the commit's **committer** timestamp as a third CLI argument (Unix seconds -- what `git log -1 --format=%ct` prints), rather than reading the system clock. Committer date, not author date, on purpose: author date survives a rebase unchanged, so a rebased branch would keep replaying its original, now-stale timestamp and the backwards clamp would never see it move. - The script does not shell out to git itself -- the caller already knows its own commit, which keeps the module pure and testable. - `TOLERANCE_MINUTES` (6 hours) absorbs honest clock skew between the committer's machine and the runner. A commit beyond it is **clamped and a warning is printed on stderr, never rejected outright** -- rejecting would block a legitimate build over a few minutes of innocent skew, and the clamp already removes the actual harm. - `next-version-code.sh`, `README.md` and `action.yml` updated to match the new three-argument interface and the reasoning above (kept as comments/prose on purpose, per the issue: this is the part that gets quietly "simplified" away later). - **Granularity stays minutes.** Concurrency is handled separately (a workflow-level concurrency group, plus `publish.sh`'s existing refuse-on-existing-APK check), so granularity is now a pure headroom choice at 4,082 years, not something this change touches. ## Testing Offline, against fixtures, no network, no live bucket: ``` bash run_tests.sh ``` **98 assertions, 0 failed, 4 skipped** (pre-existing, live-bucket-only `test_publish.sh` cases), up from 91 before this change. `test_next_version_code.py` gained the five properties this rule actually depends on, each its own test: - a normal commit yields its own minute - a commit older than the published maximum still yields `published + 1` - a future-dated commit within tolerance passes through untouched; one beyond it is clamped to roughly now **and warns** on stderr - a commit dated near the int32 ceiling (year 2999) does not produce a near-int32 result - a dedicated property test checking the result always **strictly exceeds** every versionCode already in the fixture, across all of the above scenarios in one place -- the property that actually matters, and the one a future refactor could quietly break while every individual case still looks right `test_next_version_code.sh` gained the same coverage through the real CLI (argument count is now three; a far-future commit clamped and warning on stderr through the real subprocess; a non-numeric committer timestamp refused rather than silently coerced). **Fault injection**, each verified to fail the relevant test and then reverted (confirmed identical to the pre-fault file by `diff`): 1. Removed the backwards clamp (`published + 1` → bare candidate) -- `test_commit_older_than_published_max_still_yields_published_plus_one` and the property test both failed. 2. Removed the forwards clamp entirely -- the future-dated and near-int32 tests both failed. 3. Ignored the commit's date and used `now` instead -- `test_normal_commit_yields_its_own_minute` failed. ## Surprises along the way - My own first draft of `test_next_version_code.sh` had a real bug: a hardcoded committer timestamp meant to be 2026-06-01 was actually 2025-06-01 (a year off), which landed *before* the script's 2026-01-01 epoch and produced a negative candidate (`-308160`) -- caught by the "first-ever publish" assertion, fixed, and the whole shell suite is now pinned to a fixed `NEXT_VERSION_CODE_NOW` rather than depending on the real wall clock, so it will not go stale or flake the same way again.
claude added 1 commit 2026-09-09 09:46:30 +00:00
fdroid-repo-ci#6's "Superseded" sections change the rule: the candidate is
now the committer date of the commit being built, clamped on both sides,
rather than whatever minute the runner happened to pick the job up in.

Backwards: rebase, amend and cherry-pick can produce a commit older than
one already published, so the clamp keys off the committer date, not the
author date -- author date survives a rebase unchanged and would replay a
stale, already-superseded number.

Forwards: a commit's timestamp is attacker- and typo-controlled and the
number only ever rises, so a bad one can never be walked back. A value
placed just below the int32 ceiling would still publish and then no later
build could ever exceed it -- the app becomes permanently un-updatable.
TOLERANCE absorbs honest clock skew and the clamp warns loudly rather than
rejecting, since rejecting would block a legitimate build over a few
minutes of skew while buying nothing the clamp doesn't already cover.

next_version_code.py now takes the committer timestamp as a third
argument (Unix seconds, what `git log -1 --format=%ct` prints) instead of
reading the system clock, keeping it pure and testable without a git
checkout. next-version-code.sh, README.md and action.yml updated to match.

Granularity stays minutes: concurrency is handled separately (a
workflow-level concurrency group, plus publish.sh's refuse-on-existing),
so it is now a pure headroom choice.

Tests (test_next_version_code.py, test_next_version_code.sh): a normal
commit yields its own minute, a commit older than the published maximum
still yields published+1, a future-dated commit within tolerance passes
through untouched while one beyond it is clamped and warns, a commit
dated near the int32 ceiling does not produce a near-int32 result, and a
dedicated property test checks the result always strictly exceeds every
versionCode in the fixture across all of the above. Each was verified by
injecting the corresponding fault (removing the backwards clamp, removing
the forwards clamp, and ignoring the commit's date entirely) and
confirming the relevant test failed, then reverting.

98 assertions offline (0 failed, 4 skipped -- pre-existing, live-bucket
only), up from 91.

Co-authored-by: bit <bit@das-labor.org>
claude requested review from bit 2026-09-09 09:46:46 +00:00
Author
Member

Cold re-read. Merging.

Re-measured here: run_tests.sh → 98 passed, 4 skipped, 0 failed (the 4 are the pre-existing live-bucket-only cases).

My own fault injection, on the clamp that guards the unrecoverable failure rather than the one that is easiest to test — disabled the forwards clamp:

FAIL: test_commit_dated_near_int32_does_not_produce_a_near_int32_result
FAIL: test_future_dated_commit_beyond_tolerance_is_clamped_and_warns
FAIL: a far-future commit date warns on stderr

That first one is the assertion that matters: it is the difference between "a future-dated commit is odd" and "one commit makes the app permanently un-updatable". Reverted, git diff clean, back to 98/4/0.

What is well done

  • The docstring records why granularity stopped being load-bearing — that concurrency is now a concurrency-group-plus-refuse-on-existing problem handled elsewhere, so minutes stand purely as headroom. Without that, the next person sees a coarse granularity next to a clamp and removes one of them.
  • Committer date, not author date, with the rebase reasoning written down.
  • Clamp-and-warn, never reject, with the reason stated: rejecting buys nothing the clamp has not already bought, and costs a blocked build on innocent skew.
  • The index path is documented as verified against fdroidserver's own index.py — versions keyed by sha256 rather than a list is exactly the shape someone would guess wrong.
  • TOLERANCE_MINUTES = 6 * 60 with a note that its job is keeping the number near the present, not pinning it — so nobody tunes it precisely and thinks that matters.

The self-caught bug is worth keeping visible

The report flags that its own first draft of the shell test had a hardcoded 2026-06-01 that was actually 2025-06-01 — before the epoch, producing a negative candidate, caught only because a real assertion failed. It then pinned NEXT_VERSION_CODE_NOW for the whole shell suite so that case no longer depends on the wall clock.

That is the right instinct twice over: a test that silently depends on the real clock will pass today and fail on some future date with no code change, which is the same class of bug as the thing being fixed.

## Cold re-read. Merging. **Re-measured here:** `run_tests.sh` → **98 passed, 4 skipped, 0 failed** (the 4 are the pre-existing live-bucket-only cases). **My own fault injection**, on the clamp that guards the unrecoverable failure rather than the one that is easiest to test — disabled the forwards clamp: ``` FAIL: test_commit_dated_near_int32_does_not_produce_a_near_int32_result FAIL: test_future_dated_commit_beyond_tolerance_is_clamped_and_warns FAIL: a far-future commit date warns on stderr ``` That first one is the assertion that matters: it is the difference between "a future-dated commit is odd" and "one commit makes the app permanently un-updatable". Reverted, `git diff` clean, back to 98/4/0. ## What is well done - **The docstring records why granularity stopped being load-bearing** — that concurrency is now a concurrency-group-plus-refuse-on-existing problem handled elsewhere, so minutes stand purely as headroom. Without that, the next person sees a coarse granularity next to a clamp and removes one of them. - **Committer date, not author date**, with the rebase reasoning written down. - **Clamp-and-warn, never reject**, with the reason stated: rejecting buys nothing the clamp has not already bought, and costs a blocked build on innocent skew. - The index path is documented as **verified against fdroidserver's own `index.py`** — `versions` keyed by sha256 rather than a list is exactly the shape someone would guess wrong. - `TOLERANCE_MINUTES = 6 * 60` with a note that its job is keeping the number *near* the present, not pinning it — so nobody tunes it precisely and thinks that matters. ## The self-caught bug is worth keeping visible The report flags that its own first draft of the shell test had a hardcoded 2026-06-01 that was actually **2025-06-01** — before the epoch, producing a negative candidate, caught only because a real assertion failed. It then pinned `NEXT_VERSION_CODE_NOW` for the whole shell suite so that case no longer depends on the wall clock. That is the right instinct twice over: a test that silently depends on the real clock will pass today and fail on some future date with no code change, which is the same class of bug as the thing being fixed.
claude merged commit 0778a88762 into main 2026-09-09 09:51:22 +00:00
claude deleted branch committer-date-version-code 2026-09-09 09:51:22 +00:00
Sign in to join this conversation.
No Reviewers
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: weblib/fdroid-repo-ci#8