Skip to content

docs: define the opaque acquisition identifier contract - #124

Merged
flyingrobots merged 3 commits into
mainfrom
docs/acquisition-contract
Oct 5, 2026
Merged

docs: define the opaque acquisition identifier contract#124
flyingrobots merged 3 commits into
mainfrom
docs/acquisition-contract

Conversation

@flyingrobots

@flyingrobots flyingrobots commented Oct 5, 2026 •
edited
Loading

Copy link
Copy Markdown
Member

Propose an explicit acquisition-identifier contract: identifiers remain opaque nonempty UTF-8 lines, and callers compare the complete value without normalization. The current generator pattern does not become a stored-data restriction. This preserves existing reader and migration behavior while defining malformed, absent, stale, expired, and matching guard outcomes.

Add synthetic compatibility checks for path and semaphore identities through offline migration, record rewrites, and guarded release. Unicode cases include a decomposed character and require a distinct NFC-normalized guard to remain stale. These tests exercise existing behavior; production code does not change.

Part of GL-006 and #74. This does not close #74: schema, event-table, and output-contract implementation remains GL-007. The decision remains proposed until review and acceptance. Ready for review against main. Recorded acceptance and the task-card/evidence update remain follow-up work after roadmap #121 lands.

Validation:

  • Full guarded Docker lint and test suite passed at 4bce816f19a3ee836b89bc34726cc4b6d6c20334: four opaque-identity cases including normalized-guard refusal, 1,102 shell checks, 31 release-guard cases, 760 capacity checks, and the remaining suites.
  • Local source hashes and resource receipts are retained in .test-results/acquisition-contract-evidence.json. Hosted CI run 37297214775 passed on the same head.
  • The reused worker stopped; resource monitoring reported no error.
  • These are synthetic stores, not a survey of real historical installations or proof of collision-free identifiers.

@coderabbitai

coderabbitai Bot commented Oct 5, 2026 •
edited
Loading

Copy link
Copy Markdown

Warning

Review limit reached

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

Next included review available in 32 minutes.

Check out review usage here.

View limit details

Limit details: You’ve used the included review currently available.

Learn how review limits work.

Review configuration:

⚙️ Run configuration
  • Configuration used: Organization UI
  • Review profile: ASSERTIVE
  • Plan: Advanced
  • Run ID: d13006ad-1f7d-43d2-9d72-df675f06875e
📥 Commits

Reviewing files that changed from the base of the PR and between 6735e52 and 4bce816.

📒 Files selected for processing (3)
  • Makefile
  • docs/decisions/acquisition-identifiers.md
  • test/acquisition-identity.py
  • Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@flyingrobots

flyingrobots commented Oct 5, 2026

Copy link
Copy Markdown
Member Author

Parent triage and current validation

I read the full independent report and accept its technical APPROVE for exact head 4bce816f19a3ee836b89bc34726cc4b6d6c20334. The review supervisor exited 0 and released its reservation. Required hosted CI run37297214775 passed on this head, as did the full local guarded suite.

Corrections and limits:

  • Migration calls doctor to validate legacy records before preserving their existing object IDs. It does inspect validity; it does not regenerate acquisition identifiers.
  • NUL rejection belongs to the byte-capture boundary. A Bash string/argument cannot carry embedded NUL for a later string validator to inspect.
  • runtime_log_budget_bytes is an accounting budget, not measured generated log bytes. The measured stdout length is93,805bytes; the other resource fields retain their recorded meaning.
  • The NFC test uses a conditional mismatch list, not the literal assertion quoted by the reviewer. The concrete decomposed Unicode fixture yields a different NFC value, which is tested as a stale guard.
  • The760capacity count is present in the raw full log; the review's line512 coordinate is incorrect.
  • The new sequential fixtures verify identity preservation and guard behavior. They do not independently prove concurrent CAS schedules or crash durability. The review itself inspected evidence; it did not rerun the suite.
  • The contract remains a proposal pending recorded acceptance and roadmap task-card integration. This approval does not close issue74, GL-006, or GL-007. Formal STE compliance is not claimed.

Full review follows with normalized links. Ignored receipt paths refer to retained local evidence.

Independent Adversarial Review: PR #124 (git-stunts/locks)

1. Executive Summary & Scope

PR #124 defines the contractual specification for acquisition identifiers in git-locks under task GL-006 / issue #74. It establishes that acquisition identifiers are treated as opaque, non-empty, single-line UTF-8 strings compared by exact byte equality without normalization, rather than being restricted to the internal numeric generator format (<timestamp>-<pid>-<random>).

Importantly:

  1. Zero production runtime changes: No files in bin/ or lib/ are altered.
  2. Backwards compatibility: Existing validator logic in production already implements the proposed opaque contract. PR docs: define the opaque acquisition identifier contract #124 documents this contract and provides explicit regression/compatibility test coverage.
  3. Clear implementation boundaries: Output schema adjustments and event-table alignment remain explicitly owned by task GL-007; PR docs: define the opaque acquisition identifier contract #124 makes no premature modifications to runtime schema files.
  4. Synthetic verification: A new test suite test/acquisition-identity.py exercises offline migration, renewal/refresh, stale guard refusal, NFC-normalized alias refusal, and matching release for both path locks and semaphore slots.

2. Code Paths Analysis & Parallel Path Verification

We traced every code path handling acquisition identifiers across both reservation mechanisms (path locks and semaphores) to ensure uniform enforcement.

Operation / Invariant Path Lock Production Path Semaphore Slot Production Path Analysis & Parity Verification
CLI Validation lib/110-release.sh#L21-L26
lib/140-extend.sh#L20-L25		 lib/170-semaphores.sh#L289-L294 Identical: Both call valid_holder "$2" || fail '--acquisition must be a nonempty UTF-8 line' 2. Rejects empty input, CR, LF, and invalid UTF-8 without store mutations.
Stored Blob Validation lib/055-record-validation.sh#L68-L71 lib/055-record-validation.sh#L68-L71 Shared: Role lock and role slot both execute valid_holder "${R_FIELD["${oid} acquisition"]:-}". Any syntax violation marks the record invalid.
NUL Byte Detection lib/050-the-snapshot.sh#L31-L37
lib/005-utf8.sh#L4-L6		 lib/050-the-snapshot.sh#L31-L37
lib/005-utf8.sh#L4-L6		 Shared: capture_text uses read -r -d '' to abort on embedded NULs from git cat-file --batch. valid_utf8 pattern range [\001-\177] strictly excludes byte 0x00.
Minting New Identifier lib/090-claim-planning.sh#L141-L142 lib/170-semaphores.sh#L160 Identical: Both invoke new_acquisition (lib/080-families.sh#L80-L84), generating printf '%s-%05d-%05d%05d' "${at}" "$$" "${RANDOM}" "${RANDOM}".
Renewal / Refresh Preservation lib/140-extend.sh#L38-L48 lib/170-semaphores.sh#L154-L165 Consistent semantics: In extend, acq is read from poid and written unchanged to the new record blob. In sem acquire, an active holder refresh keeps acq from own_oid (L157), whereas an expired slot triggers new_acquisition (L160).
Family Hierarchy Rewrites lib/080-families.sh#L102,L111 N/A (Semaphores do not participate in family trees) Verified: bump_parent increments family generation and creates a new blob OID, but carries acq="$(field "${poid}" acquisition)" forward unchanged.
Guard Matching & Stale Handling lib/110-release.sh#L56-L62 lib/170-semaphores.sh#L216-L221 Exact Parity: Release checks [[ "${have_acq}" != "${acqs[${i}]}" ]] and [[ "${want_acquisition}" != "${own_acq}" ]]. Mismatches emit {"event":"nothing",...,"reason":"superseded"} with exit code 0.
Multi-Guard Orthogonality lib/110-release.sh#L52-L61 lib/170-semaphores.sh#L218 Independent: Both compare --record and --acquisition independently. Evaluated as (want_record != own_oid) || (want_acquisition != own_acq). Neither overrides the other regardless of CLI flag order (honoring PR #123 invariants).
Offline Migration lib/185-migrate.sh#L16-L19 lib/185-migrate.sh#L16-L19 Shared: cmd_migrate indexes existing blob OIDs directly into the state tree without inspecting or regenerating acquisition values.
Doctor / Integrity Auditing lib/175-doctor.sh#L25-L29 lib/175-doctor.sh#L31-L48 Consistent: In case of invalid stored acquisition text, validate_record fails and doctor emits structured findings record-decodes (locks) or sem-record (semaphores) rather than crashing.
Wrapper (with) Verification lib/160-with.sh#L374-L378 lib/160-with.sh#L374-L378 Shared: with_reason performs direct string comparison [[ "${acquired}" != "${W_ACQUISITION}" ]] during both admission verification and exit cleanup.

3. Merges and History Audit

Inspection of git log from base 6735e52007ab86a37feda8b43b1d43742fdb5f94 to head 4bce816f19a3ee836b89bc34726cc4b6d6c20334:

* 4bce816 test: reject normalized aliases of acquisition guards
* 512fd4a test: verify opaque identities through migration and renewal
* d665170 docs: propose opaque acquisition identifier contract

4. No Trusted Claims: Evidence & Behavioral Verification

Every factual, behavioral, and compatibility assertion in the PR description, commit messages, and decision document was audited against raw code and recorded receipts:

  1. Synthetic Opaque Identities Through Migration and Renewal:

    • Doc Claim: Stored records with arbitrary nonnumeric ASCII and Unicode identifiers survive offline migration with identical root tree hash, survive renewal with new record OID, reject stale record guards, and reject NFC-normalized variants of decomposed characters.
    • Code Audit: test/acquisition-identity.py synthesizes blobs with opaque-legacy-owner and owner-e\u0301-雪 (decomposed e + U+0301). It verifies:
      • assert root() == synthetic after git locks migrate --offline (line 63).
      • assert current['acquisition'] == identity and assert current['record'] != record after extension/refresh (lines 68-69).
      • assert stale['reason'] == 'superseded' and root() == renewed when providing old record OID (line 72).
      • assert unicodedata.normalize('NFC', identity) != identity generates owner-é-雪, which is rejected with reason: superseded without altering root state (lines 74-80).
      • assert locks(*release, '--acquisition', identity)[0]['event'] == 'released' succeeds and modifies root (lines 81-82).
    • Verdict: Fully verified against test implementation.

  2. PR fix: enforce independent semaphore release guards #123 Regression Evidence:

    • Doc Claim: "PR #123 corrected semaphore guard aliasing at this baseline. Its full guarded suite passed, including 31 release-guard cases. The four RED failures represented three distinct failure scenarios."
    • Raw Evidence Check: Inspected .test-results/semaphore-guards-evidence.json and .test-results/semaphore-guards-red-latest.log. Lines 26, 29, 30, and 31 show the 4 RED failures:
      • Failure 1 (stale-record False): old record passed before matching acquisition released anyway.
      • Failure 2 (stale-acquisition True): stale acquisition passed before matching record released anyway.
      • Failures 3 & 4 (record-as-acquisition False & True): record OID passed to --acquisition released anyway due to argument aliasing.
        This precisely constitutes 3 distinct failure scenarios across 4 test cases.
    • Verdict: Fully verified against historical receipts.

  3. Absence of Broken Links:

5. Constants & Resource Bounds Audit

Audited against .test-results/acquisition-contract-evidence.json and .test-results/acquisition-contract-full-launch.json:

Metric / Parameter Enforced Binding Limit Measured Value in Evidence Margin / Compliance
Subprocess Timeout 10 seconds 10s per command in test/acquisition-identity.py Compliant; guards against deadlock
Suite Execution Timeout 1800 seconds (30m) Completed within single bounded run Compliant
Host Disk Free Space $\ge$ 50 GiB floor 717,054,595,072 bytes (667.8 GiB) Well above floor ($\sim 13\times$)
Docker VM Free Space $\ge$ 50 GiB floor 681,437,782,016 bytes (634.6 GiB) Well above floor ($\sim 12.6\times$)
Generated Build Cache $\le$ 20 GiB 0 bytes generated Well within budget
Peak Runtime Non-Object Storage $\le$ 4 GiB 6,070,272 bytes (5.79 MiB) Well within budget
Runtime Log Budget $\le$ 128 MiB (134,217,728 bytes) 71,269,792 bytes generated / 93,805 bytes stdout Well within budget
Tmpfs Mount: /work 512 MiB quota 3,235,840 bytes (3.09 MiB) Peak $\approx 0.6%$ of quota
Tmpfs Mount: /tmp 512 MiB quota 21,561,344 bytes (20.56 MiB) Peak $\approx 4.0%$ of quota
Tmpfs Mount: /evidence 16 MiB quota 3,448,832 bytes (3.29 MiB) Peak $\approx 20.6%$ of quota
Container Constraints 2 CPUs, 2 GiB RAM, 256 PIDs Configured via host_config in isolation manifest Verified

Worker state: Reused container git-locks-tests stopped cleanly; native host lock /var/folders/g7/h_9lmyb14gs68tfzv88dklt00000gn/T/git-locks-docker-tests.lock and workstation leases released.

6. Numeric Claims Audit

Cross-verified all numbers stated in PR description, documentation, and receipts:

  1. Source SHA-256 Hashes:
    • Makefile: d0b1d1f64f68fc8d48ec0016f75c64d96d778e4b72fbd6363fe0c7e917ba8014 (Verified byte-identical)
    • docs/decisions/acquisition-identifiers.md: 1347ff8e693a4c3892f5be68d4237b70556fc4c614d23b14c37912a256e427b4 (Verified byte-identical)
    • test/acquisition-identity.py: b561c60993b3ab1130fae38124e886fa1c1ab0b3dd22b6b507d4077be731c501 (Verified byte-identical)
  2. Test Checks:
    • 4 synthetic identity cases: Checked in test/acquisition-identity.py (path + semaphore $\times$ ASCII + Unicode) and confirmed in .test-results/acquisition-contract-full-latest.log#L494-L499.
    • 1,102 shell checks: Verified in log line: 1102 passed, 0 failed from test/test.sh.
    • 31 release-guard checks: Verified in log line 493: 31 passed; 0 failed from test/release-guards.py.
    • 760 capacity checks: Verified in log line 512: capacity: 760 checks passed; seed 3507; 39 valid and 31 invalid inputs; 12 racers / 3 winners.
    • 240 literal path checks: Verified by counting 240 ok lines in test/literal-paths.sh run (log lines 1615–1857).

7. State Machine Transitions, Errors, and Guard Outcomes

The Required Behavior matrix in docs/decisions/acquisition-identifiers.md#L39-L49 was systematically tested against the production state transitions:

stateDiagram-v2
    [*] --> InputValidation
    InputValidation --> UsageError : Empty / CR / LF / Invalid UTF-8
    UsageError --> [*] : Exit 2 (No Mutation)

    InputValidation --> Snapshot : Well-formed UTF-8 Line
    Snapshot --> SnapshotRefusal : Stored NUL / Corrupt Stored Record
    SnapshotRefusal --> [*] : Store Error Exit 2 / Doctor Finding

    Snapshot --> StateEvaluation : Valid Snapshot
    StateEvaluation --> AbsenceResult : Job / Slot Missing
    AbsenceResult --> [*] : Exit 0 / Exit 1 (Documented Absence)

    StateEvaluation --> GuardCheck : Job / Slot Exists
    GuardCheck --> SupersededResult : Mismatched Acquisition / Stale Record
    SupersededResult --> [*] : Release exits 0 ("nothing") / Renewal exits 1 ("refused")

    GuardCheck --> ActionExecution : Matching Guards
    ActionExecution --> ExpiredRenewalRefusal : Extend on Expired Lease
    ActionExecution --> MutationCommit : Valid State & Clock
    MutationCommit --> [*] : CAS Write Root Tree
Loading
  • Usage Refusal (Exit 2): Tested in test/release-guards.py. CLI rejects bad UTF-8 or newlines before snapshot or transaction planning.
  • Fail Closed on Stored Corruption: Tested via test/doctor-findings.py and test/store-integrity.py. Bad records in state trigger snapshot read errors or doctor findings.
  • Absence Handling: Absent jobs produce documented absence objects without assuming whether the guard was previously valid.
  • Exact Byte Identity: Prohibits Unicode canonical equivalence aliasing (e.g. NFD vs NFC), ensuring deterministic POSIX-level string comparisons.

8. Repository Standards & Decision Document Review

  1. Drafting Style:
    • Adheres to requested Simplified Technical English (ASD-STE100) principles: short, unambiguous, declarative sentences with active verbs.
    • Paragraphs are concise and focused on single requirements or rationale points.
  2. Scope Isolation:
    • The document explicitly acknowledges that GL-007 retains ownership of schema definitions and event table updates.
    • Frontmatter correctly pins baseline_commit: "6735e52007ab86a37feda8b43b1d43742fdb5f94" and conforms to schema: "git-locks-decision/1".
  3. Makefile Integration:
    • Makefile#L44 inserts python3 test/acquisition-identity.py into the test-container target cleanly.
    • Shell scripts list SCRIPTS is unchanged because the added test is Python, leaving shellcheck and shfmt targets clean.

9. Review Coverage Limitations & Operational Gaps

To maintain strict truth in review, the following limitations are recorded:

  1. Synthetic vs. Historical Store Evidence: As explicitly noted in the decision document and PR description, the tests synthesize records using git hash-object and git update-index. While this proves that production decoders, migration scripts, and release guards accept opaque values, it does not constitute an empirical survey of real-world historical repository stores.
  2. Hosted CI Execution: Hosted CI run 37297214775 was reported as live during supervisor inspection and recorded as successful in the evidence file. In accordance with read-only/no-network instructions, no live network query was initiated; terminal verification will be enforced by the parent before merging.
  3. Concurrency and Power Failure: Synthetic test execution operates sequentially in temporary repositories. It validates deterministic logic and CAS semantics, but does not simulate kernel crashes or physical host power interruption.
  4. Schema Alignment Deferred: schema/git-locks.schema.json currently specifies acquisition with "minLength": 1 and does not yet declare regex constraints excluding CR/LF. This is not a defect of PR docs: define the opaque acquisition identifier contract #124, as GL-007 explicitly owns schema and event-contract alignment.

10. Mandatory Verification Checklist

11. Final Verdict

APPROVE

@flyingrobots
flyingrobots marked this pull request as ready for review October 5, 2026 10:47
@flyingrobots
flyingrobots merged commit 3d004d5 into main Oct 5, 2026
4 checks passed
This was referenced Oct 5, 2026
Open
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

1 participant

@flyingrobots