docs(adr): propose unified acknowledged operation results (#114)

[tetsuh]

Summary

Closes: N/A — this ADR PR is related to #114, which remains open through ADR-0029 and the coordinated Issue/document updates.

• add and accept ADR-0028 as the first ADR produced by Issue Proposal: unify acknowledged writes and BufferPublisher fences with typed AckResult #114;
• define one strict typed acknowledgement result protocol for acknowledged Put, PutBatch, and Fence;
• freeze UUIDv4 token ownership, attachment/result v1 layouts, node-local bounded token lifecycle, duplicate/collision handling, PutBatch confirmed-prefix behavior, and submit-once/total-deadline polling;
• append Status::OutcomeUnknown = 9 to the accepted contract without changing code yet;
• register the accepted acknowledgement contracts as Normative with implementation still Planned;
• keep Fence marker ordering and consumer-specific lifecycle in a separate ADR under Transport: same-publisher in-band fence ordering contract #106, as required by the repository's one-decision-per-ADR rule.

ADR status and split

ADR-0028 is Accepted — 2026-07-23. Issue #114 covers two related but distinct decisions:

1. the shared acknowledged-operation result protocol — this ADR;
2. same-publisher Fence ordering, BufferPublisher failure aggregation, and [M3] ParamCache: wait for local write delivery #99 waiter lifecycle — the follow-up ADR-0029 owned by Transport: same-publisher in-band fence ordering contract #106.

Both ADRs must be Accepted before #99 or #107 implementation begins. #14 and #17 also remain blocked until their Issue bodies are synchronized with accepted ADR-0028.

Scope checklist

• canonical internally generated UUIDv4 token and strict 17-byte attachment;
• node-local Processing/Completed lifecycle, global token claim across lanes, bounded 4096-entry completion ring, collision handling, eviction boundary, phase-aware exception completion, and Stop quiescence;
• strict little-endian AckResult v1 schema, closed Status allowlist, sentinels, Encoding, and query semantics;
• deterministic non-transactional PutBatch stop-first/confirmed-prefix contract, including whole-message and entry-specific validation failures;
• conservative one-submit behavior for current Transport::Put failures and a total 3-second default deadline with ACK-query-only retry;
• C++/Python ParamStore parity and dedicated OutcomeUnknownError mapping;
• explicit separation from ParamCache write semantics and from the Transport: same-publisher in-band fence ordering contract #106 Fence-ordering ADR;
• Linux/Windows, codec, lifecycle, sanitizer, and integration qualification gates;
• ADR index entry;
• contract-registry Rule 1 transition for the Accepted ADR.

Requirements

Requirement IDs: N/A — pre-implementation architecture decision; no production behavior changes in this PR.

Contract Registry

Contract transitions in docs/08_contract_registry.md:

• meta/ack/<uuid> route behavior / AckResult: Contract Planned → Normative; Implementation Planned;
• generated canonical UUIDv4 correlation ID: Contract Planned → Normative; Implementation Planned;
• Status::OutcomeUnknown = 9: Contract Planned → Normative; Implementation Planned;
• Python OutcomeUnknownError: Contract Planned → Normative; Implementation Planned;
• AckAttachmentV1: registered as Contract Normative; Implementation Planned;
• sitos.v1.ack Encoding: registered as Contract Normative; Implementation Planned.

All six rows use ADR-0028 as their normative specification and have no open design authority. The existing lenient meta/ack/<uuid> parser-grammar row remains Contract Planned / Implementation Implemented because ADR-0028 reuses but does not normatively define that grammar. PR #116 introduced Rule 1 after this PR opened, so the two previously absent surfaces are registered in this still-open ADR-landing PR.

Review resolution

The original FIX BEFORE MERGE findings are resolved in the rebased history:

• top-level Batch validation failures use applied_count = 0 and failed_index = UINT32_MAX;
• entry-specific validation failures and application failures carry an explicit entry index;
• pre-mutation/sync exceptions map to Status::Error;
• exceptions after invoking a mutation/sync operation map to Status::OutcomeUnknown when the effect cannot be proven;
• docs/adr/README.md includes ADR-0028;
• the ADR status and decision date are Accepted — 2026-07-23.

The subsequent CodeRabbit/Opus review is resolved through head a50164f:

• rebased onto main after PR docs(process): add contract registry and milestone design-review gate (#115) #116;
• advanced and registered the affected contract-registry rows;
• fixed the Markdownlint MD018 issue caused by a line-leading #105 reference;
• clarified that the Planned Fence-marker row was registered by Docs: add public contract registry and milestone design-review gate #115 / PR docs(process): add contract registry and milestone design-review gate (#115) #116 and remains pending an Accepted Transport: same-publisher in-band fence ordering contract #106 ADR.

The CodeRabbit requests to revert the owner-approved Accepted status and to add the already-present ADR index entry required no code change. Closes #114 is intentionally not used because it would close the still-active cross-cutting proposal before ADR-0029 and coordinated updates finish.

Acceptance Criteria Verification

git rebase origin/main: pass
git diff --check: pass
secret scan: pass
independent read-only review: registry Rule 1 transitions and Markdown fix have no blocker

RED-phase Confirmation

N/A — documentation-only ADR PR; no production code is included.

ADR Needed?

• No
• Yes — this PR contains Accepted ADR-0028.

Compatibility and implementation note

The ADR defines future source/wire behavior changes but this PR changes documentation only. #14/#17 and the normative architecture/wire/C++/Python documents must be updated together before implementation merges. Synced Fence remains additionally blocked by #105. #99 and #107 implementation remain blocked until follow-up ADR-0029 is Accepted.

[coderabbitai]

📝 Walkthrough

Walkthrough

This PR adds ADR-0028, defining a shared acknowledgement-result protocol for Put, PutBatch, and Fence operations, including UUIDv4 tokens, binary encoding, bounded token state, polling deadlines, outcome semantics, qualification tests, and contract registry updates.

Changes

Acknowledged operation results

Layer / File(s)	Summary
Acknowledgement protocol and wire contract docs/adr/0028-unify-acknowledged-operation-results.md, docs/08_contract_registry.md	Defines generated UUIDv4 tokens, attachment encoding, AckResult v1 validation, query routing, and normative acknowledgement contract entries.
Token lifecycle and operation outcomes docs/adr/0028-unify-acknowledged-operation-results.md	Specifies bounded token registries, collision and duplicate handling, result eviction, status mapping, and non-transactional PutBatch prefix semantics.
Client deadlines, Fence reuse, and qualification docs/adr/0028-unify-acknowledged-operation-results.md, docs/adr/README.md, docs/08_contract_registry.md	Defines submission and polling deadlines, shared Fence protocol reuse, qualification tests, follow-up references, OutcomeUnknown mappings, and the ADR index entry.

Estimated code review effort: 2 (Simple) | ~10 minutes

Possibly related issues

• [M2] Ack protocol #14 — ADR-0028 formalizes the unified acknowledgement protocol, token semantics, and timeout/outcome rules.
• [M2] ParamStore: ack / detailed error mapping #17 — ADR-0028 defines the acknowledgement and polling semantics for acknowledged ParamStore operations.
• Docs: add public contract registry and milestone design-review gate #115 — The contract registry updates record ADR-0028 as the authority for the acknowledgement surfaces.

Possibly related PRs

• tetsuh/sitos#110 — Revises the acknowledgement flow that ADR-0028 consolidates into the final AckResultV1 and token model.
• tetsuh/sitos#116 — Establishes the contract-registry framework updated here for AckResultV1 and OutcomeUnknown.

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Linked Issues check	✅ Passed	The PR satisfies #114's main requirement by adding the accepted ADR and updating the registry/index without implementing code.
Out of Scope Changes check	✅ Passed	The extra docs changes are in scope for the ADR and registry/index updates described in the PR objectives.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Title check	✅ Passed	The title is concise and accurately reflects the ADR about unified acknowledged operation results.
Description check	✅ Passed	The description covers the required template sections, including summary, requirements, contract registry, acceptance logs, RED-phase, and ADR status.

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch feat/114-unified-ack-result-adr

---

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

[tetsuh]

Review — FIX BEFORE MERGE

Findings

1. Major — An AckResult cannot represent an invalid batch as a whole. failed_index defines UINT32_MAX as “none/not applicable” (docs/adr/0028-unify-acknowledged-operation-results.md:94), while batch failure always requires the “first failed index” (:118) and full-validation rejection still produces a typed result (:137-139). Inputs such as a truncated header or zero entry count with trailing bytes have no failed entry, so implementations cannot agree on strict sentinel validation (:101-103). Add an explicit invariant for top-level batch validation failures, e.g. failed_index = UINT32_MAX.

2. Major — The contract that converts every post-claim exception to Status::Error (docs/adr/0028-unify-acknowledged-operation-results.md:75-77) conflicts with the definition of OutcomeUnknown (:150-152,199-201). StorageEngine::Put is not noexcept (include/sitos/storage_engine.hpp:59-60), and the contract does not preclude an implementation from throwing after applying the value. In that case application cannot be proven and OutcomeUnknown is required. Define phase-aware completion: Error before mutation/sync is invoked, and OutcomeUnknown for exceptions after invocation when the outcome cannot be guaranteed.

3. Minor — The ADR index is not updated: docs/adr/README.md:6-34 ends at 0027. Add an entry for ADR-0028.

4. Minor (merge gate) — The ADR remains Proposed (docs/adr/0028-unify-acknowledged-operation-results.md:3-5). After approval, update it to Accepted with the decision date before merge, per docs/10_adr_process.md:113-121.

Validation

• git diff --check passes, as do the required CI checks displayed for the PR.
• This is documentation-only; no implementation tests are needed in this PR.

Notes

Fence ordering, publisher-failure aggregation, and #99 waiter lifecycle remain undecided until ADR-0029 is Accepted.

[tetsuh]

Resolved all four FIX BEFORE MERGE findings in cf2e104:

• defined failed_index = UINT32_MAX for whole-batch validation failures and explicit indices for entry validation/application failures;
• made exception completion phase-aware (Error before mutation/sync invocation, OutcomeUnknown after invocation when the effect cannot be proven), while preserving confirmed batch prefixes;
• added ADR-0028 to docs/adr/README.md;
• accepted ADR-0028 with decision date 2026-07-23.

The PR body now reflects Accepted status and the review resolution. git diff --check and secret scan pass; an independent read-only review found no remaining AckResult invariant conflict.

[coderabbitai]

Actionable comments posted: 3

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@docs/adr/0028-unify-acknowledged-operation-results.md`:
- Around line 3-5: Update the ADR-0028 Status entry from “Accepted — 2026-07-23”
to “Proposed”, removing the decision date until approval.
- Around line 283-292: Add ADR-0028 to the ADR catalog in docs/adr/README.md,
using the document’s existing index format and linking it to
0028-unify-acknowledged-operation-results.md. Leave the ADR content and other
references unchanged.
- Line 237: Update the prose at the affected line so the issue reference does
not begin with an unescaped hash; prefix the reference with “issue” while
preserving the existing StorageEngine durability contract wording.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: c6ebe024-51f3-451e-8ba7-5f3ecc857c36

📥 Commits

Reviewing files that changed from the base of the PR and between 0b61249 and cf2e104.

📒 Files selected for processing (2)

• docs/adr/0028-unify-acknowledged-operation-results.md
• docs/adr/README.md

[tetsuh]

Re-review: ADR-0028 (head cf2e104) — reviewed by Opus

I read the full ADR, confirmed the prior FIX BEFORE MERGE findings are resolved, and did a fresh
pass against Issue #114, my #114 review amendments, the repository's ADRs/Status/StorageEngine
contract, and the contract registry merged in #116.

Prior FIX BEFORE MERGE findings — all resolved

1. Batch whole-message failure — resolved: the schema and prose now define three distinct modes
   (envelope/whole-message → failed_index = UINT32_MAX; entry validation → that entry's index;
   application failure → confirmed prefix + first failed entry).
2. Post-claim exception vs OutcomeUnknown — resolved: phase-aware completion (Error before a
   mutation/sync call is invoked, OutcomeUnknown after invocation when the effect cannot be
   proven), consistent with StorageEngine::Put not being noexcept; confirmed prefix preserved.
3. ADR index — resolved (README updated).
4. Accepted status — resolved (Accepted — 2026-07-23).

ADR content — technically sound

• All five of my Proposal: unify acknowledged writes and BufferPublisher fences with typed AckResult #114 amendments are faithfully incorporated: Python parity + OutcomeUnknownError;
  BufferPublisher: explicit applied and synced fences #107 failure aggregation deferred to ADR-0029 (consistent with one-decision-per-ADR); the
  wire/cache asymmetry note with [M3] ParamCache: disconnect and reconnect recovery #20 as the resync boundary; ParamCache writes unaffected; and
  OutcomeUnknown framed as "no stronger claim available", tightenable by a future typed engine
  result without a wire change.
• Byte layouts check out: AckResultV1 header = 1+1+1+1+4+4+8+8+4 = 32 bytes + message (≤1024);
  AckAttachmentV1 = 1+16 = 17 bytes; offsets contiguous.
• The Status allowlist is correct and matches the enum, and deliberately excludes Timeout = 3
  (a client-side deadline outcome the node never emits) — a sound distinction.
• Lock discipline (no token-registry lock held across engine/callback/sync calls), Stop quiescence
  under ADR-0017, ADR-0020 consolidated-reply reuse (no reply-counting), and the non-LRU 4096-entry
  completion-order ring are all consistent with the existing ADRs and code.

No correctness blocker in the ADR itself.

Findings

1. [should fix — first application of registry Rule 1] The ADR-landing PR does not advance the registry

Rule 1 (merged in #116) says "the PR that lands the owning ADR advances its Contract status."
ADR-0028 is the owning ADR for the acknowledged-result registry rows and is now Accepted, but this
PR touches only the ADR and README — docs/08_contract_registry.md is untouched. Sol's #116 review
explicitly anticipated this ("when PR #117's ADR-0028 is Accepted, update the related registry row's
spec, status, and authority under Rule 1"). Needed:

• Advance existing rows (Contract: Planned → Normative; authority → ADR-0028; Normative spec →
  ADR-0028): the meta/ack route-behavior/AckResult row, the generated-UUIDv4 correlation-id row,
  the Status: OutcomeUnknown row, and the OutcomeUnknownError row. Implementation status stays
  Planned ([M2] Ack protocol #14/[M2] ParamStore: ack / detailed error mapping #17 not implemented) — only Contract status advances.
• Register the new surfaces ADR-0028 introduces that have no row yet: AckAttachmentV1 (the
  17-byte attachment) and the sitos.v1.ack Encoding identifier (Contract Normative by ADR-0028,
  Implementation Planned, authority ADR-0028).
• PR body: fill the new Contract Registry field (advanced rows + transition).

Do not advance the lenient meta/ack parser-grammar row: ADR-0028 only says the route "accepts
the existing safe route grammar" and does not normatively specify the parser grammar, so that row
stays Planned.

Fairness note: #116 merged after this PR was opened, so it is understandable the branch predates
Rule 1. But #117 is still open and is the ADR-landing PR, so advancing the registry here is the
correct precedent for the mechanism's first use. Doing it as an immediate tracked follow-up PR is a
defensible owner choice, but the same-PR advance is cleaner.

2. [minor — implementer note for #14] SHA-256 fingerprint

The ADR asserts the fingerprint introduces "no external dependency"; the implementer must confirm a
SHA-256 source consistent with the dependency policy (docs/09 / ADR-0003). Also worth weighing
whether a full-payload SHA-256 is proportionate for detecting collisions of internally generated
UUIDv4 tokens (a condition "a supported high-level caller cannot create"); a lighter fingerprint may
suffice. Not an ADR blocker — the cost is already noted in Consequences.

Verdict

The ADR is technically excellent and the four prior findings are genuinely resolved. The one
substantive item is finding 1 — advancing the contract registry per Rule 1, which is exactly the
process introduced by #116 working as intended on its first ADR-landing PR. Recommend applying it in
this PR before merge; finding 2 is a #14 implementation note.

[tetsuh]

Resolved the actionable post-cf2e104 review at rebased head 033a19d:

• rebased onto current main after PR docs(process): add contract registry and milestone design-review gate (#115) #116;
• advanced the four existing ADR-0028-owned registry rows from Planned to Normative while keeping Implementation Planned;
• registered AckAttachmentV1 and sitos.v1.ack as Normative / Implementation Planned;
• intentionally left the lenient meta/ack parser-grammar row Planned;
• fixed the line-leading #105 Markdownlint warning;
• updated the PR body with the Contract Registry transitions and an explicit Closes: N/A rationale so Proposal: unify acknowledged writes and BufferPublisher fences with typed AckResult #114 remains open for ADR-0029.

git diff --check and secret scan pass. An independent read-only review found no registry Rule 1, authority, or status-separation blocker. The SHA-256 note remains a #14 implementation/qualification item, not an ADR blocker.

[coderabbitai]

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@docs/08_contract_registry.md`:
- Line 58: Update the “Same-publisher in-band fence marker” row in the contract
registry so its provenance column records the actual earlier issue/proposal
review or milestone-assembly registration point. Keep the status as Planned and
retain the Fence ADR acceptance as the condition for acceptance, but remove the
inaccurate “added on ADR acceptance” note.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: e8b7ad46-ec51-4a48-aacf-6d20ef2a89aa

📥 Commits

Reviewing files that changed from the base of the PR and between cf2e104 and 033a19d.

📒 Files selected for processing (3)

• docs/08_contract_registry.md
• docs/adr/0028-unify-acknowledged-operation-results.md
• docs/adr/README.md

🚧 Files skipped from review as they are similar to previous changes (2)

• docs/adr/README.md
• docs/adr/0028-unify-acknowledged-operation-results.md

[sonarqubecloud]

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud