fix: normalize semaphore capacity as decimal

CHANGELOG.md

@@ -6,6 +6,8 @@ All notable changes to this project are recorded here. The format follows Keep a
 
 ### Fixed
 
+- Semaphore capacity is validated as a bounded positive decimal and normalized before storage, arithmetic, and JSON serialization. Leading-zero values such as `01`, `08`, and `010` keep their decimal meaning, including when reading metadata written by older versions. Invalid stored capacities fail with `store-read` (#35).
+- `doctor` reads a stored semaphore capacity with the same decimal rule. Before, a legacy `08` printed a bash arithmetic error, `010` was compared as octal eight (so nine live slots were a false `sem-capacity` finding), and a capacity past 2^64 wrapped around to a small number instead of being a `sem-record` finding.
 - Validate authoritative lock and semaphore records before normal reads or planning (#33). Corrupt records now produce a structured `store-read` error with exit 2 before any success output or mutation. Doctor shares the decoder and safely reports malformed numeric fields; generation tokens remain opaque. Stored decimal fields normalize leading zeros and reject values outside the nonnegative signed 64-bit range. Child admission refuses a parent whose family generation cannot advance without overflow.
 - `sem list` no longer reads the slot of a job named `meta` as a second semaphore. It matched any ref ending in `/meta`, so `refs/locks/sem/gpu/slots/meta` printed a `gpu/slots` line with an empty capacity, which is not valid JSON. Snapshot validation splits semaphore refs the same way, so such a slot is validated as a slot rather than as metadata.
 - Path normalisation preserves literal `*`, `?` and bracket characters instead of expanding them against files in the working tree.

Makefile

@@ -14,12 +14,13 @@ lint:
 
 test:
 	bash test/test.sh
+	python3 test/capacity.py
 	bash test/literal-paths.sh
 	bash test/unicode-locale.sh
 	bash test/unicode-locale-calibration.sh
 
 test-docker: # the same suite inside the official bash image, for a wall between the tests and your machine
-	docker run --rm -v "$(CURDIR)":/src -w /src bash:5.2 bash -c 'apk add --no-cache git python3 py3-jsonschema >/dev/null && git config --global user.email t@example.invalid && git config --global user.name t && bash test/test.sh && bash test/literal-paths.sh && bash test/unicode-locale.sh && bash test/unicode-locale-calibration.sh'
+	docker run --rm -v "$(CURDIR)":/src -w /src bash:5.2 bash -c 'apk add --no-cache git python3 py3-jsonschema >/dev/null && git config --global user.email t@example.invalid && git config --global user.name t && bash test/test.sh && python3 test/capacity.py && bash test/literal-paths.sh && bash test/unicode-locale.sh && bash test/unicode-locale-calibration.sh'
 
 install:
 	mkdir -p $(PREFIX)/bin

README.md

@@ -261,6 +261,8 @@ In summary, "all or nothing" is never a loop with a rollback. It is one stanza,
 
 A path lock says one holder. Some resources are better described by a number: two GPUs, five build agents. This section shows how git-locks gives a named resource a capacity while keeping the same atomicity, and it needs one new idea, a generation token, that the reader has all the pieces for.
 
+Capacity is a positive decimal integer from `1` through `9223372036854775807`. Leading zeros are accepted and normalized: `--capacity 010` means ten and emits JSON `10`. Reads also normalize leading-zero capacities stored by older versions without rewriting their metadata. Invalid or out-of-range input is refused before semaphore refs are created; invalid stored capacity is a `store-read` error, and `doctor` reports it as a `sem-record` finding.
+
 A semaphore is three kinds of ref under `refs/locks/sem/<name>/`. `meta` points at a blob holding the capacity. `slots/<job>` is one ref per holder, pointing at a slot record with the holder and an expiry, exactly like a lock record. And `gen` points at a blob whose only purpose is to change: every transaction on the semaphore writes a fresh generation blob and `update`s `gen` from the generation it read. Two acquirers that both read "2 of 3 live" both try to move `gen` from the same old value; git lets exactly one through, and the other re-reads and finds the semaphore full. Here is the example's semaphore, capacity 2, filled by alice and bob, then refused to carol:
 
 ```text

bin/git-locks

@@ -1908,6 +1908,10 @@ with_release_sem() { # record -> releases this invocation's slot, if it is still
 # acquirers who both counted "n of N live" contend on one compare-and-swap and
 # exactly one commits. The other re-reads.
 
+valid_capacity() { # VAR value: record_uint (leading zeros are decimal, never octal; signed 64-bit range), and positive
+  record_uint "$1" "$2" && [[ "${!1}" != 0 ]]
+}
+
 sem_meta_ref() { printf '%s/sem/%s/meta' "${NS}" "$1"; }
 sem_gen_ref() { printf '%s/sem/%s/gen' "${NS}" "$1"; }
 sem_slot_ref() { printf '%s/sem/%s/slots/%s' "${NS}" "$1" "$2"; }
@@ -1946,6 +1950,7 @@ sem_read() {      # name -> 0, or 1 when the semaphore does not exist
   SEM_META_OID="$(ref_oid "${mref}")"
   [[ -n "${SEM_META_OID}" ]] || return 1
   SEM_CAP="$(field "${SEM_META_OID}" capacity)"
+  valid_capacity SEM_CAP "${SEM_CAP}" || store_error "semaphore ${name} has an invalid capacity"
   gref="$(sem_gen_ref "${name}")"
   SEM_GEN_OID="$(ref_oid "${gref}")"
   SLOT_JOBS=()
@@ -2183,7 +2188,7 @@ cmd_sem() {
   done
   case "${verb}" in
     create)
-      [[ "${capacity}" =~ ^[0-9]+$ && "${capacity}" -gt 0 ]] || fail '--capacity is a positive number' 2
+      valid_capacity capacity "${capacity}" || fail '--capacity is a decimal integer from 1 through 9223372036854775807' 2
       if sem_read "${name}"; then
         sem_refusal "${name}" exists
         exit 1

lib/170-semaphores.sh

@@ -5,6 +5,10 @@
 # acquirers who both counted "n of N live" contend on one compare-and-swap and
 # exactly one commits. The other re-reads.
 
+valid_capacity() { # VAR value: record_uint (leading zeros are decimal, never octal; signed 64-bit range), and positive
+  record_uint "$1" "$2" && [[ "${!1}" != 0 ]]
+}
+
 sem_meta_ref() { printf '%s/sem/%s/meta' "${NS}" "$1"; }
 sem_gen_ref() { printf '%s/sem/%s/gen' "${NS}" "$1"; }
 sem_slot_ref() { printf '%s/sem/%s/slots/%s' "${NS}" "$1" "$2"; }
@@ -43,6 +47,7 @@ sem_read() {      # name -> 0, or 1 when the semaphore does not exist
   SEM_META_OID="$(ref_oid "${mref}")"
   [[ -n "${SEM_META_OID}" ]] || return 1
   SEM_CAP="$(field "${SEM_META_OID}" capacity)"
+  valid_capacity SEM_CAP "${SEM_CAP}" || store_error "semaphore ${name} has an invalid capacity"
   gref="$(sem_gen_ref "${name}")"
   SEM_GEN_OID="$(ref_oid "${gref}")"
   SLOT_JOBS=()
@@ -280,7 +285,7 @@ cmd_sem() {
   done
   case "${verb}" in
     create)
-      [[ "${capacity}" =~ ^[0-9]+$ && "${capacity}" -gt 0 ]] || fail '--capacity is a positive number' 2
+      valid_capacity capacity "${capacity}" || fail '--capacity is a decimal integer from 1 through 9223372036854775807' 2
       if sem_read "${name}"; then
         sem_refusal "${name}" exists
         exit 1

test/capacity.py

@@ -0,0 +1,127 @@
+#!/usr/bin/env python3
+"""Exercise decimal capacity through the CLI with an independent integer oracle."""
+import concurrent.futures
+import json
+import os
+from pathlib import Path
+import random
+import subprocess
+import tempfile
+
+import jsonschema
+
+ROOT = Path(__file__).resolve().parents[1]
+CLI = ROOT / 'bin/git-locks'
+SCHEMA = json.loads((ROOT / 'schema/git-locks.schema.json').read_text())
+VALIDATOR = jsonschema.Draft202012Validator(SCHEMA)
+SEED = 3507
+LIMIT = 9223372036854775807
+checks = 0
+
+
+def check(condition, message):
+    global checks
+    assert condition, message
+    checks += 1
+
+
+with tempfile.TemporaryDirectory(prefix='git-locks-capacity-') as tmp:
+    base = Path(tmp)
+    env = {key: value for key, value in os.environ.items()
+           if not key.startswith('GIT_')}
+    env.update(HOME=tmp, GIT_LOCKS_STORE=str(base / 'store.git'),
+               GIT_LOCKS_NOW='1000000', LC_ALL='C')
+
+    def run(*args, expected=0):
+        result = subprocess.run([str(CLI), *args], cwd=tmp, env=env,
+                                text=True, capture_output=True, timeout=45)
+        check(result.returncode == expected,
+              f'{args}: exit {result.returncode}, expected {expected}: {result.stderr}')
+        data = []
+        for output in (result.stdout, result.stderr):
+            for line in output.splitlines():
+                value = json.loads(line)
+                VALIDATOR.validate(value)
+                data.append(value)
+        check(bool(data), f'{args}: missing structured output')
+        return data
+
+    def git(*args, input=None):
+        return subprocess.run(['git', '--git-dir=' + env['GIT_LOCKS_STORE'], *args],
+                              cwd=tmp, env=env, text=True, input=input,
+                              capture_output=True, check=True).stdout.strip()
+
+    rng = random.Random(SEED)
+    values = ['1', '01', '08', '010', str(LIMIT), '000' + str(LIMIT), '0' * 256 + '8']
+    values += ['0' * rng.randrange(1, 25) + str(rng.randrange(1, 1000000)) for _ in range(32)]
+    for index, value in enumerate(values):
+        name = f'valid-{index}'
+        want = int(value, 10)
+        check(run('sem', 'create', name, '--capacity', value)[0]['capacity'] == want,
+              f'create did not normalize {value!r}')
+        check(run('sem', 'show', name)[0]['capacity'] == want, 'show capacity mismatch')
+        meta = git('show', f'refs/locks/sem/{name}/meta')
+        check(f'capacity: {want}' in meta.splitlines(), 'stored capacity is not canonical')
+        check(run('sem', 'acquire', name, '--job', 'a', '--holder', 'alice')[0]['capacity'] == want,
+              'acquire capacity mismatch')
+        check(run('sem', 'release', name, '--job', 'a')[0]['capacity'] == want,
+              'release capacity mismatch')
+        run('sem', 'delete', name)
+
+    invalid = ['', '0', '00', '-1', '+1', '1.0', '1e2', ' 1', '1 ', '08x', '１２',
+               str(LIMIT + 1), str(2**64 + 1), '9' * 100, '0' * 100 + str(LIMIT + 1)]
+    invalid += [str(rng.randrange(LIMIT + 1, 2**100)) for _ in range(16)]
+    before = git('for-each-ref', '--format=%(refname) %(objectname)', 'refs/locks/')
+    for index, value in enumerate(invalid):
+        result = run('sem', 'create', f'invalid-{index}', '--capacity', value, expected=2)
+        check(result[0]['reason'] == 'usage', 'invalid capacity is not a usage error')
+        check(git('for-each-ref', '--format=%(refname) %(objectname)', 'refs/locks/') == before,
+              'invalid input changed authoritative refs')
+
+    # Stores written by 0.7.0 may have leading zeros; reads normalize without rewriting.
+    run('sem', 'create', 'legacy', '--capacity', '1')
+    meta = git('show', 'refs/locks/sem/legacy/meta')
+    oid = git('hash-object', '-w', '--stdin', input=meta.replace('capacity: 1', 'capacity: 01') + '\n')
+    git('update-ref', 'refs/locks/sem/legacy/meta', oid)
+    check(run('sem', 'show', 'legacy')[0]['capacity'] == 1, 'legacy show is not normalized')
+    check(run('sem', 'list')[0]['capacity'] == 1, 'legacy list is not normalized')
+    check(git('rev-parse', 'refs/locks/sem/legacy/meta') == oid, 'read rewrote legacy metadata')
+    run('sem', 'acquire', 'legacy', '--job', 'first', '--holder', 'alice')
+    refused = run('sem', 'acquire', 'legacy', '--job', 'second', '--holder', 'bob', expected=1)[0]
+    check(refused['reason'] == 'capacity' and refused['capacity'] == 1, 'legacy capacity not enforced')
+    run('sem', 'release', 'legacy', '--job', 'first')
+    run('sem', 'delete', 'legacy')
+
+    # A fresh metadata record can be corrupt independently of its JSON serialization.
+    for value in ('0', '08x', str(LIMIT + 1)):
+        run('sem', 'create', 'bad-meta', '--capacity', '1')
+        meta = git('show', 'refs/locks/sem/bad-meta/meta')
+        original = git('rev-parse', 'refs/locks/sem/bad-meta/meta')
+        oid = git('hash-object', '-w', '--stdin', input=meta.replace('capacity: 1', 'capacity: ' + value) + '\n')
+        git('update-ref', 'refs/locks/sem/bad-meta/meta', oid)
+        result = run('sem', 'show', 'bad-meta', expected=2)
+        check(result[0]['reason'] == 'store-read', 'bad stored capacity did not fail closed')
+        check(git('rev-parse', 'refs/locks/sem/bad-meta/meta') == oid, 'bad read changed metadata')
+        git('update-ref', 'refs/locks/sem/bad-meta/meta', original)
+        run('sem', 'delete', 'bad-meta')
+
+    run('sem', 'create', 'race', '--capacity', '03')
+
+    def contender(index):
+        result = subprocess.run([str(CLI), 'sem', 'acquire', 'race', '--job', f'r{index}',
+                                 '--holder', f'h{index}'], cwd=tmp, env=env, text=True,
+                                capture_output=True, timeout=45)
+        assert result.returncode in (0, 1), result
+        for line in (result.stdout + result.stderr).splitlines():
+            VALIDATOR.validate(json.loads(line))
+        return result.returncode
+
+    with concurrent.futures.ThreadPoolExecutor(max_workers=12) as pool:
+        statuses = list(pool.map(contender, range(12)))
+    check(statuses.count(0) == 3, f'12 racers on capacity 03 had {statuses.count(0)} winners')
+    observed = run('sem', 'show', 'race')[0]
+    check(observed['capacity'] == 3 and observed['live'] == 3 and len(observed['slots']) == 3,
+          'post-race slot state violates capacity')
+    check(run('doctor')[0]['healthy'], 'post-race doctor unhealthy')
+
+print(f'capacity: {checks} checks passed; seed {SEED}; {len(values)} valid and {len(invalid)} invalid inputs; 12 racers / 3 winners')

test/test.sh

@@ -727,6 +727,20 @@ valid "sem delete line" "${out}"
 git-locks sem show batch >/dev/null 2>&1
 check "a deleted semaphore is gone" "$?" "1"
 
+# Decimal capacity must preserve its numeric value in storage and every JSON line.
+for capacity_case in '01:1' '08:8' '010:10'; do
+  capacity_input="${capacity_case%:*}"
+  capacity_want="${capacity_case#*:}"
+  out="$(git-locks sem create "decimal-${capacity_want}" --capacity "${capacity_input}" 2>&1)"
+  check "capacity ${capacity_input} creates a semaphore" "$?" "0"
+  jfields "capacity ${capacity_input} emits decimal ${capacity_want}" "${out}" "capacity=${capacity_want}"
+  valid "capacity ${capacity_input} create line" "${out}"
+  out="$(git-locks sem show "decimal-${capacity_want}" 2>&1)"
+  check "capacity ${capacity_input} can be read" "$?" "0"
+  jfields "capacity ${capacity_input} reads decimal ${capacity_want}" "${out}" "capacity=${capacity_want}"
+  valid "capacity ${capacity_input} show line" "${out}"
+done
+
 # exactly K winners under contention
 git-locks sem create race --capacity 3 >/dev/null 2>&1
 wins=0
@@ -1233,6 +1247,33 @@ findings n "${out}" sem-gen
 check "a semaphore without its gen ref is one sem-gen finding" "${n}" "1"
 valid "semaphore finding lines" "${out}"
 
+# Doctor reads a stored capacity with the same decimal rule as sem_read: leading zeros are decimal, out of range is a sem-record finding.
+git-locks sem create legacy --capacity 1 >/dev/null 2>&1
+legacy_meta="$(gs rev-parse refs/locks/sem/legacy/meta)"
+for capacity_case in '08:0' '010:0' '9223372036854775808:1' '18446744073709551617:1'; do
+  capacity_stored="${capacity_case%:*}"
+  capacity_findings="${capacity_case#*:}"
+  rewritten="$(gs cat-file -p "${legacy_meta}" | sed "s/^capacity: 1\$/capacity: ${capacity_stored}/" | blob)"
+  gs update-ref refs/locks/sem/legacy/meta "${rewritten}"
+  out="$(git-locks doctor 2>&1)"
+  findings n "${out}" sem-record
+  check "doctor on stored capacity ${capacity_stored} has ${capacity_findings} sem-record findings" "${n}" "${capacity_findings}"
+  valid "doctor lines on stored capacity ${capacity_stored}" "${out}"
+done
+gs update-ref refs/locks/sem/legacy/meta "${legacy_meta}"
+for i in $(seq 1 9); do
+  slot="$(printf 'schema: git-locks-slot/1\nsemaphore: legacy\njob: l%s\nholder: bob\nclaimed: 1000000\nexpires: 2000000\nacquisition: t-l%s' "${i}" "${i}" | blob)"
+  gs update-ref "refs/locks/sem/legacy/slots/l${i}" "${slot}"
+done
+rewritten="$(gs cat-file -p "${legacy_meta}" | sed 's/^capacity: 1$/capacity: 010/' | blob)"
+gs update-ref refs/locks/sem/legacy/meta "${rewritten}"
+out="$(git-locks doctor 2>&1)"
+findings n "${out}" sem-capacity
+check "nine live slots under stored capacity 010 (ten, not octal eight) are not a sem-capacity finding" "${n}" "0"
+for i in $(seq 1 9); do gs update-ref -d "refs/locks/sem/legacy/slots/l${i}"; done
+gs update-ref refs/locks/sem/legacy/meta "${legacy_meta}"
+git-locks sem delete legacy >/dev/null 2>&1
+
 # An unreadable store is an error, never healthy.
 err="$(PATH="${BROKEN}:${PATH}" git-locks doctor 2>&1 >/dev/null)"
 rc="$?"