Skip to content

docs: ADR-0033 Slice 2 acceptance contract — transactional apply (no implementation) - #184

Merged
PhysShell merged 8 commits into
mainfrom
claude/adr-0033-slice-2-apply-contract
Aug 20, 2026
Merged

docs: ADR-0033 Slice 2 acceptance contract — transactional apply (no implementation)#184
PhysShell merged 8 commits into
mainfrom
claude/adr-0033-slice-2-apply-contract

Conversation

@PhysShell

@PhysShell PhysShell commented Aug 17, 2026

Copy link
Copy Markdown
Owner

This PR specifies ADR-0033 Slice 2 only. It does not implement transactional Apply. Implementation remains prohibited until independent acceptance is formally recorded against the exact head SHA.

Status: CONTENT REVIEW PASS at 47e734cfbf1a6bd90c1bd2a035cdc68692378e96 (see the final hostile re-review comment in this PR's thread, 2026-08-19) — six hostile review rounds, no remaining contract-level blockers, no r8 requested. The content verdict is deliberately not the acceptance record: §16 requires independent acceptance by someone other than the PR author, and the reviewing GitHub identity here is the PR author. Implementation stays blocked until that acceptance is recorded against this exact SHA; any head movement that changes the contract reopens review.

Base / head

  • Base: main (branch carries a clean merge of main @ d7012f5; the contract was authored against 6188cc2)
  • Head: 47e734cfbf1a6bd90c1bd2a035cdc68692378e96 (r7 — the content-pass SHA)
  • Changed files vs main: docs/proposals/song-curation-slice-2-transactional-apply.md (new), docs/proposals/README.md (index entry only)

Revision history

  • r1 (473377a) — initial contract.
  • r2 (f9e8a67) — six blockers from review round 1 (r2 audit): filesystem names decoupled from plan content; report after the single staged preflight; single-writer index lock; four-way report partition; honest duplicate-key guard; supersession-evidence consistency law. (Reviews arrive as PR comments: the reviewing identity is the PR author, so GitHub rejects a formal REQUEST_CHANGES with a 422.)
  • r3 (975550f) — five blockers + one minor from round 2 (r3 audit): §6 invariant narrowed, lock I/O typed; canonical resolved index identity (F8); step-12 failure state completed, R3 restricted; staged tree-agreement re-check (K11); Slice-1 surface split 11 reachable + 3 unreachable (A8).
  • r4 (dca507f) — four blockers from round 3 (r4 audit): missing-index refusal reachable at step 1; one observable result shape (F9); reserved-area carry-forward law; coordination-path collisions refused pre-lock (F10).
  • r5 (49a7750) — four blockers from round 4 (r5 audit): hardlinked index refused (F11); pre-existing temp refused, never silently unlinked (C13); step-11 publication-rename failure enumerated; reserved-area shape law wired through step 3 / taxonomy / K12. Mergeability restored by merge commit d24cc1b (contract untouched).
  • r6 (d441331) — four blockers + two corrections from round 5 (r6 audit): temp inspection under the held lock (C14); lock ownership marker + ApplicationIndexLockPathOccupied (C15); step-12 temp write create_new no-clobber (F12); reserved .apply-staging namespace (OutputNameReserved, F13) + honestly documented empty-directory rename residual; staging-may-remain wording; flock claim corrected.
  • r7 (47e734c) — the one remaining round-6 blocker (r7 audit): the lock-marker state machine closed over all byte-prefixes — contention classification (any marker prefix refuses ApplicationIndexLocked, so live ownership is never misclassified) separated from recovery authority (only the exact complete marker auto-deletes; ambiguous prefixes take an operator-proven path; non-prefix occupants are never deleted or relocated by mandate); hard_link alternative recorded as considered; case C16. The crash-between-publish-and-commit model survived all six review rounds unchanged.

Authority

Normative: ADR-0033 (Decisions 5–10), ADR-0031, ADR-0032, the accepted frozen Slice 1 (song-curation/), core contracts (song_holdout_preflight, corpus schema), the migrate-v9 preflight precedent. The historical proposal is historical context only — every detailed rule not fixed by ADR-0033 is explicitly adopted, modified, or rejected in §13; nothing is inherited by silence.

Decisions resolved

  • Inputs / authority (§4): plan + corpus snapshot + application index + fresh output; one authority per fact; index must exist (typed refusal at step 1); reserved song-curation/ area with recursive shape law and supersession semantics; tree agreement; no-root-songs law.
  • Application index v1 (§5.1): strict schema, ordered records, uniqueness, internal chain validation, no paths, deterministic rendering.
  • Application report v1 (§5.2, §5.4): chain self-description, total-order refusal sort, four-way partition, no wall-clock, exact report_digest coverage.
  • Curated manifest (§5.3): Slice 2 owns generation; fixed distinct path; hard refusal as defense in depth.
  • Verification order (§6): 12-step total order; step 1 = pure path preflight (index existence, nlink == 1, coordination collisions, reserved namespace) → lock acquisition with prefix-closed content classification → under-lock temp inspection; nothing else touches the filesystem before step 9; step 10 re-runs tree agreement on the staged tree, runs the single preflight, then writes the report; verify_plan literally reused; already-applied precedes chain equations.
  • Chain law (§7.1–7.2): three exact equalities / one null-relation.
  • Already-applied (§7.3): registry-only; fingerprint-neutral batches provably applied; independent lineages allowed.
  • Replacement authority (§7.4): supersession-evidence consistency law, then the exhaustive case table.
  • Transaction semantics (§8): fixed-name staging (atomic create_dir; .apply-staging reserved) → one publication rename (honestly non-no-clobber; accepted empty-directory residual documented) → create_new temp + rename as the single commit point, under a marker-bearing single-writer lock bound to the canonical resolved index (symlink aliases converge; hardlinks refused; marker state machine closed over all prefixes; unproven occupants of coordination paths never deleted — by Apply or by recovery); one observable result shape; complete enumerated failure model; durability caveat; no ACID vocabulary.
  • Preservation law (§10): raw byte copy outside the reserved area; semantic identity + canonical rendering for touched files; two-check guard; reserved artifacts superseded.
  • Holdout readiness (§11): the real core preflight, once, over the staged curated view; partial curation is a valid holdout_ready: false snapshot.
  • Refusal taxonomy (§12): closed — 11 Apply-reachable Slice-1 refusals (+3 documented unreachable) + 24 new typed refusals, each with condition, evidence, ordering, may-output-exist; every refusal proves pre-commit.
  • RED→GREEN matrix (§14): 63 preregistered cases; RED-eligibility marked; fault-injection and raw-bytes fixtures identified; every Apply-reachable refusal covered.
  • Prior art (§15): Slice-2 deltas only; no new dependency required or permitted.

Unresolved blockers

None at content level (six-round review record above). Outstanding: the formal independent-acceptance record itself.

Non-goals

No Apply implementation; no corpus writes or labels; no suggestions/normalization; no SongId issuance; no Slice 3 / pilot / full-corpus labeling; no changes to ADR-0031/0032/0033, S7/S8/S9/S15/S16, or Swang; no CLI/cockpit integration; no griff manifest changes; no similarity/embeddings/MIR; song-curation/ stays a non-workspace-member; no production dependency.

Validation performed

  • Diff vs main contains no executable-code changes: two Markdown files only; the main merge commit (d24cc1b) leaves docs/proposals/ untouched; each review round's semantic delta is one dedicated docs commit.
  • All relative links resolve (scripted check, re-run each revision); 80-column prose convention held (tables excepted).
  • Frozen/accepted material untouched; no acceptance entry written anywhere before independent acceptance.

Implementation acceptance criteria

After independent acceptance is recorded against 47e734c, the implementation is acceptable only if: strict RED→GREEN per the §14 matrix (tests-only RED commits first, per commit); frozen Slice-1 API, semantics, and tests untouched and green; the §12 taxonomy implemented closed; the §6 ordering and §8 publication semantics observable in the enumerated failure states; any needed deviation reopens this contract for re-acceptance.

🤖 Generated with Claude Code

https://claude.ai/code/session_01Kc6r6PhKQCNqArwkijUnzn

…onal apply

Open Slice 2 as a docs-only, separately reviewable acceptance contract
(ADR-0033 Decision 10): exact Apply inputs/outputs and per-fact authority,
application-index v1 and apply-report v1 schemas, the fail-closed
verification order, chain equations, already-applied semantics, the
replacement-authority law, staging/commit publication semantics with an
enumerated failure model, a bounded evidence-backed preservation law
(the historical byte-for-byte claim audited against migrate's re-rendering
and core's tolerant parsing), holdout-readiness law, a closed refusal
taxonomy, explicit dispositions of historical-proposal rules, prior-art
deltas, and the preregistered RED→GREEN acceptance matrix.

No implementation: Slice 2 stays prohibited until this contract receives
independent acceptance. No corpus files, labels, runtime behaviour,
dependencies, or Swang files change.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Kc6r6PhKQCNqArwkijUnzn
@coderabbitai

coderabbitai Bot commented Aug 17, 2026

Copy link
Copy Markdown

Important

  • 🔍 Trigger review

This repository does not receive automatic reviews because it has fewer than 10 stars.

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 68c2c4d1-27d5-465d-be99-f7d327c1d006


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.

…s addressed

1. Filesystem names decoupled from plan content: fixed staging name
   (<output_parent>/.<output_name>.apply-staging) and index temp/lock names
   derived only from the index path, so step 1 needs nothing parsed and an
   unrestricted batch_id String can never influence a path (adversarial
   matrix case F7).
2. Step 9/10 ordering fixed: stage corpus + curated manifest first, then the
   single song_holdout_preflight and recomputation self-check, and only then
   build, digest, and write the report — its fields now exist before it is
   written.
3. Single-writer index lock: exclusive create of <index_dir>/.<index_name>.lock
   held from before the first index read to after the commit rename;
   pre-existing lock refuses ApplicationIndexLocked (no waiting, no
   lock-breaking); stale-lock crash state and recovery added to the §8.2
   table; matrix cases C11/C12. Closes the read-to-commit lost-update window
   that could silently drop a committed record.
4. Report partition corrected to four ways: applied / unchanged /
   reviewed-unassigned / untouched, so a reject_suggestion source (referenced
   yet unassigned) is representable; sum equals coverage.unique_sources.
5. Preservation guard made honest about duplicate keys: a distinct
   duplicate-rejecting JSON pass is required (Value comparison cannot prove
   duplicates absent — serde_json keeps the last one silently); matrix case
   K10.
6. Supersession-evidence consistency law: redundant supersedes_song_ids on
   accept/merge/split must agree with the action's authority set
   (empty / from_song_ids / [from_song_id]); violation is the new typed
   SupersessionEvidenceContradiction, checked per event before any
   per-assignment authority decision; matrix cases L8/L9.

Docs-only; still awaiting independent acceptance; implementation remains
prohibited.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Kc6r6PhKQCNqArwkijUnzn

Copy link
Copy Markdown
Owner Author

Revision r2 (f9e8a6724f419b9b5f411f866180b97142c16f7d) — six hostile-review findings addressed.

An independent hostile review of 473377a (delivered off-GitHub: the reviewer's REQUEST_CHANGES was rejected by GitHub with a 422 because the reviewing identity is the PR author) found six contract blockers. Acceptance was not granted at 473377a. All six are now resolved in the contract text; this comment is the audit record of finding → resolution. The crash-between-publish-and-commit model was attacked and survived unchanged.

  1. Step 1 was unexecutable as ordered + batch_id path injection. The staging name depended on batch_id, which is only known after step-2 parsing, and batch_id is an unrestricted Slice-1 String. → No filesystem name is derived from plan content anymore: staging is the fixed <output_parent>/.<output_name>.apply-staging, index temp/lock names derive only from the index path. Step 1 now needs nothing parsed. Adversarial matrix case F7 (path-like batch_id) added.

  2. Report was written before its fields existed. Step 9 wrote apply-report.json, but holdout_ready / holdout_refusals come from the single song_holdout_preflight at step 10. → Reordered: step 9 stages corpus + curated manifest only; step 10 runs the recomputation self-check and the single preflight, then builds, digests, and writes the report into staging. §8.1 updated to match.

  3. No single-writer protection of the application index. Two concurrent appliers could read one head, both pass, and the second commit rename would silently drop the first record after its applier reported success — breaking "applied iff record is in index". → Exclusive lock (create_new of <index_dir>/.<index_name>.lock) held from before the first index read to after the commit rename; pre-existing lock refuses the new typed ApplicationIndexLocked (fail-closed, no waiting, no lock-breaking); stale-lock crash state and mandated recovery added to the §8.2 table; matrix cases C11/C12.

  4. Three-way report partition could not place reject_suggestion sources (referenced, yet no assignment). → Four-way partition: assignments_applied / assignments_unchanged / sources_reviewed_unassigned / sources_untouched, summing exactly to coverage.unique_sources; cross-referenced from the C6 rejection-only case.

  5. The duplicate-key claim of the preservation guard was false. serde_json::Value is already a map that silently keeps the last duplicate, so Value comparison can never prove duplicates absent. → §10.3 now mandates a distinct duplicate-rejecting JSON pass (map visitor refusing a repeated key at any depth, native, no new dependency) alongside the round-trip equality check; matrix case K10 (raw-bytes fixture).

  6. Redundant supersedes_song_ids could contradict the authority law and be silently ignored. → New per-event consistency law in §7.4, checked before any per-assignment authority decision: accept_suggestion ↔ empty, merge ↔ its from_song_ids, split ↔ exactly [from_song_id]; violation is the new typed SupersessionEvidenceContradiction. Recorded as an explicit disposition of the historical proposal's §8.2 in §13. Matrix cases L8/L9.

Totals: refusal taxonomy now 14 reused + 19 new typed refusals; preregistered matrix now 50 cases. Still docs-only; Slice 2 remains PROPOSED / IMPLEMENTATION BLOCKED, awaiting a repeat independent hostile review at f9e8a67.


Generated by Claude Code

Copy link
Copy Markdown
Owner Author

r2 hostile re-review on exact head f9e8a6724f419b9b5f411f866180b97142c16f7d: the original six findings are closed, but I am not accepting r2 yet. Five contract-level blockers remain.

  1. The new lock contradicts the stated verification-order invariant and has an untyped I/O failure path. §6 still says “No step may create, modify, or delete anything on the filesystem before step 9” and calls step 1 “pure”, but step 1 now create_news the index lockfile and later deletes it. That is an intentional coordination mutation, so the invariant must be narrowed explicitly (e.g. no corpus/output/index mutation before step 9; transient lock is the sole exception). More importantly, lock creation can fail for reasons other than AlreadyExists (permissions, read-only FS, etc.); the closed taxonomy maps only pre-existence to ApplicationIndexLocked, while ApplyIoError is currently steps 9–12. Define the truthful typed result for lock acquire/release I/O and its cleanup semantics. A failed best-effort release also needs an explicit externally observable outcome.

  2. The single-writer proof is not yet bound to the canonical physical index path. §8.3 says the index location is canonicalized for comparisons, but §8.1 describes lock/temp/read/rename in terms of <index_dir>/<index_name> without saying that all subsequent index operations use the resolved canonical target. If the supplied index path is a symlink alias, two aliases can otherwise derive different locks; rename over the alias path can also replace the symlink rather than the file that was read. Either reject symlink index inputs or state that the canonical resolved index file path is the one and only path used for lock, read, temp, fsync/rename, and release. Add an alias-path concurrency/update test.

  3. Step-12 failure recovery is incomplete, and R3 contradicts it. Step 12 runs only after step 11 published the output tree. Therefore a temp-write / fsync / commit-rename failure at step 12 leaves output already present with its report, plus the old index and possibly the temp file. The §8.2 row “failure during step 12's temp write” currently lists only temp index file present, index unchanged and tells the operator to delete only the temp file; that leaves an orphan output, so retry then hits OutputAlreadyExists. Enumerate the whole pre-commit step-12 state and recover both temp + orphan output. Also R3 currently says “after any refusal ... neither ... a published tree exists”, which is false for precisely these post-publication ApplyIoError states and conflicts with F4/R4 and §8.2. Restrict R3 to pre-publication refusals and pin the post-publication error state separately.

  4. The staged self-check still does not prove output tree agreement. Step 3 proves input root-manifest ↔ *.chunk.json agreement. Step 9 then rewrites both representations. Step 10 re-reads only staged root manifest.json + curated manifest and runs song_holdout_preflight over the manifest view. A buggy write that updates the root/curated manifests but misses one affected *.chunk.json can therefore pass step 10 and publish a snapshot whose storage files disagree with its authority manifest. Re-run the same tree-agreement check over the staged tree before publication (this is not a second song_holdout_preflight), and make K1/K5 or a new case prove a deliberately stale staged chunk file aborts.

  5. The claimed Slice-2 refusal surface includes three ledger-only Slice-1 errors that verify_plan cannot emit. Apply consumes a DryRunPlan, not a DecisionsLedger. Frozen verify_plan validates the embedded standalone batch and can return batch-order/duplicate-event, digest/fingerprint/projection, inventory and replay failures; UnsupportedDecisionsLedgerSchema, DuplicateDecisionBatchId, and BatchNotInLedger arise only in validate_ledger / build_plan. Yet §12 says all 14 Slice-1 refusals are “reused verbatim through step 5”, and §14 claims every refusal in §12 has a case. That is mechanically false. Distinguish crate-wide CurationError members from the reachable Apply refusal surface, or otherwise explain why those three are intentionally unreachable; the acceptance matrix must match the reachable surface exactly.

Minor but worth fixing while there: §8.1 says a second applier after the first completes is refused because its input fingerprint no longer matches the head. That is not universally true for the contract's own fingerprint-neutral/reject-only batch (C6); refusal may instead come from duplicate batch or predecessor-report/chain mismatch. Phrase it in terms of the actual chain law, not fingerprint change.

No implementation/RED work should start. After these docs-only corrections, I will re-review the next exact head.

1. The §6 no-mutation invariant is narrowed honestly: the transient index
   lockfile is its one named exception; lock create/release failures other
   than pre-existence are typed ApplyIoError (extended to step-1 lock I/O),
   and a failed best-effort release is surfaced explicitly without changing
   the run's primary outcome.
2. The canonical resolved index file is declared the one and only index
   identity: lock name, read, temp, fsync, commit rename, and release all
   act on the canonicalized path, so symlink aliases contend on one lock and
   the rename replaces the real file, never an alias (matrix case F8).
3. The step-12 failure row now includes the already-published output tree;
   recovery removes both the temp file and the orphan. R3 is restricted to
   pre-publication refusals, deferring post-publication states to F4/R4 and
   the §8.2 table.
4. Step 10 re-runs the §4.2 tree-agreement law over the staged tree, so a
   write that updates the manifests but misses a chunk file (or vice versa)
   aborts as OutputPreflightInconsistent before publication (fault-injected
   matrix case K11).
5. The Slice-1 refusal surface is split honestly: eleven members reachable
   through verify_plan, three ledger-side members
   (UnsupportedDecisionsLedgerSchema, DuplicateDecisionBatchId,
   BatchNotInLedger) declared intentionally unreachable in Apply and
   excluded from the coverage claim; new case A8 exercises the reachable
   inventory/replay refusals at the Apply boundary.
6. Minor: the post-completion second-applier refusal is phrased via the
   actual chain law (already-applied or a broken §7.2 relation), not a
   fingerprint change, which a fingerprint-neutral batch would falsify.

Docs-only; still awaiting independent acceptance; implementation remains
prohibited.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Kc6r6PhKQCNqArwkijUnzn

Copy link
Copy Markdown
Owner Author

Revision r3 (975550f27143d6fe5b6f1f0b675d3ec6de89fd9c) — the five r2 re-review blockers and the minor are addressed. Finding → resolution:

  1. Lock vs. the "nothing before step 9" invariant; untyped lock I/O. The §6 preamble invariant is narrowed to what is actually true: no mutation of the corpus trees, the output path, or the index file before step 9, with the transient lockfile named as the single deliberate exception (coordination state, never output). Step 1's heading now separates the pure path checks from the lock acquisition. ApplyIoError is extended to step-1 lock I/O: any create/release failure other than pre-existence (permissions, read-only FS, …) is typed there; only pre-existence is ApplicationIndexLocked. A failed best-effort release never changes the run's primary outcome (a committed apply stays applied, a refusal stays that refusal) but must be surfaced explicitly, naming the leftover lockfile and the §8.2 recovery.

  2. Lock not bound to the canonical physical index. §8.1 now declares the canonical resolved index file the one and only index identity: the supplied path is canonicalized at step 1, and that resolved path alone is used for the lock name, the read, the temp file, the fsync, the commit rename, and the release. Two aliases of one index therefore contend on one lock, and the commit rename replaces the real file, never a symlink alias. New matrix case F8 (alias-path lock contention and post-commit alias resolution).

  3. Step-12 failure state incomplete; R3 contradicted it. The §8.2 row for a failure inside step 12 (temp write, fsync, or commit rename) now states the whole state — output tree already published with its report, old index unchanged, temp file possibly present — and mandates recovery of both the temp file and the orphaned output tree, with retry refusing OutputAlreadyExists until the orphan is removed. R3 is restricted to pre-publication refusals (steps 1–10); the post-publication commit-failure states are explicitly deferred to F4/R4 and the §8.2 table.

  4. Staged output tree agreement unproven. Step 10 now re-runs the §4.2 tree-agreement law over the staged tree (staged root manifest ↔ staged *.chunk.json, the same multiset check as step 3 — not a second preflight) before fingerprinting, the preflight, and the report. A write that updates the manifests but misses an affected chunk file — or the reverse — aborts as OutputPreflightInconsistent and never publishes. New fault-injected matrix case K11 (deliberately stale staged chunk file).

  5. Three ledger-only Slice-1 errors claimed as reused. §12 now splits the shared crate error type honestly: eleven members reachable through step 5's verify_plan (reused verbatim), and three ledger-side members — UnsupportedDecisionsLedgerSchema, DuplicateDecisionBatchId, BatchNotInLedger — declared intentionally unreachable in Apply (it consumes a plan, never a ledger) and excluded from the Apply refusal surface and the §14 coverage claim. New matrix case A8 exercises the reachable inventory/replay refusals at the Apply boundary (UnidentifiedSource, ConflictingExistingSongIds, UnknownDecisionSource, SourceAssignedToMultipleSongs, DuplicateDecisionEventId), so the "every reachable refusal has a case" claim is now mechanically true.

  6. (minor) The §8.1 post-completion second-applier sentence is rephrased via the actual chain law: the same batch refuses DecisionBatchAlreadyApplied; a batch planned against the stale head refuses on a broken §7.2 relation — for a fingerprint-neutral head that is the previous-report-digest relation (1), not any fingerprint change.

Totals: matrix now 53 preregistered cases; refusal surface = 11 reachable Slice-1 refusals + 19 new typed refusals (+ 3 ledger-side members documented as unreachable). Still docs-only; Slice 2 remains PROPOSED / IMPLEMENTATION BLOCKED, awaiting re-review at exact head 975550f.


Generated by Claude Code

Copy link
Copy Markdown
Owner Author

r3 hostile re-review on exact head 975550f27143d6fe5b6f1f0b675d3ec6de89fd9c: the five r2 blockers + minor are closed as claimed. I am not accepting r3 yet. Four contract-level blockers remain.

  1. Missing-index refusal is unreachable under the new canonical-index order. §4.1 and §12 promise a missing index → MalformedApplicationIndex at step 2. But §8.1 now requires the supplied index path itself to be canonicalized at step 1 and explicitly says the file must already exist; canonicalize on a missing target fails before step 2. Preserve the existing refusal by making step 1 detect missing/broken-target index input and return MalformedApplicationIndex before canonicalization/lock, or deliberately reclassify it and update C7/taxonomy. The current text cannot be implemented literally.

  2. Lock-release failure has two incompatible public outcomes. §6/§12 says any non-preexistence lock create/release failure is typed ApplyIoError, and §12 defines it inside the closed refusal surface. §8.1 simultaneously says a failed best-effort release “never changes the run's primary outcome — a committed apply stays applied, a refusal stays that refusal” and is merely surfaced alongside it. In index rename succeeds → lock unlink fails, returning Err(ApplyIoError) makes an applied batch look failed; returning success+warning means this ApplyIoError is not a refusal. Specify one observable result shape unambiguously, including primary-refusal + release-failure, and preregister fault-injection tests for both. The operator must never infer whether ApplyIoError happened before or after the commit point.

  3. Incremental Apply contradicts the preservation law for the reserved song-curation/ area. A successful Slice-2 output contains song-curation/manifest.json and song-curation/apply-report.json; that output is the natural input for C2/the next chained batch. §4.2 excludes the reserved directory from corpus-content enumeration, but §10.1 says every input file not touched is raw-copied, while §10.2 defines touched only for assigned chunk files and root manifest.json. The prior curated manifest/report are therefore “untouched” by the literal law, yet steps 9–10 must generate new bytes at those same paths. Define carry-forward semantics explicitly: the known tool-owned proof artifacts should be excluded from raw-copy preservation and regenerated, with any other reserved-area files preserved or rejected by a stated rule. Make C2 use the first Apply's real output as the second input and assert the reserved-area result.

  4. User-chosen output can collide with the index's internal .lock / .tmp paths, and step 1 does not reject it. The preflight checks output/staging against corpus and canonical index file, but not against <canonical_index_dir>/.<canonical_index_name>.lock or .tmp. If output equals the lock path, step 1 itself creates the lock at the declared output path, violating the no-output-path-mutation-before-step-9 invariant. If output equals the temp path, step 12 is guaranteed to collide. Once the canonical index path is known, preflight must prove output/staging do not equal or overlap internal coordination paths before acquiring the lock. Add an adversarial path case and a deterministic typed refusal rather than degrading into late ApplyIoError.

Everything else from r3's intended repairs checks out: staged tree agreement is real, post-publication step-12 recovery is complete, symlink aliases converge on the canonical index path, reachable Slice-1 refusals are separated correctly, and the fingerprint-neutral post-completion wording is fixed.

No RED/implementation work should start. After these docs-only corrections, re-review the next exact head.

1. The missing-index refusal is made reachable again: step 1 first proves
   the supplied index path resolves to an existing regular file
   (MalformedApplicationIndex raised there, before canonicalization and the
   lock, because canonicalize needs an existing target); C7 and the
   taxonomy row now name both raise points.
2. One unambiguous observable result shape: Apply returns a single primary
   outcome (applied, or one typed refusal) plus an optional orthogonal
   lock-release warning. A release failure is never a refusal and never
   changes the primary outcome; every refusal, ApplyIoError included, is
   returned only by a run that did not reach the commit point, so a refusal
   always proves pre-commit. Fault-injection case F9 preregisters both
   shapes (success+warning, refusal+warning).
3. Reserved-area carry-forward law: the input's song-curation/ proof
   artifacts describe the input's own application and are superseded, never
   raw-copied; a §4.2 shape law admits only the two tool-owned artifacts in
   the reserved area (anything else is CorpusTreeDisagreement); §10.1
   excludes the area from the raw-copy law; C2 now chains from the first
   Apply's real published output and asserts the second reserved area.
4. Coordination-path collisions refused deterministically: new typed
   OutputCollidesWithIndexArtifacts when output or staging equals the
   canonical index file, its lockfile, or its temp path — checked at step 1
   before lock acquisition, so the lock can never be created at a declared
   output path; adversarial case F10. ApplicationIndexInsideTree tightened
   to equals-or-inside.

Docs-only; still awaiting independent acceptance; implementation remains
prohibited.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Kc6r6PhKQCNqArwkijUnzn

Copy link
Copy Markdown
Owner Author

Revision r4 (dca507f9a2e75b57e99ba3422bbd9c1bd26261c5) — the four r3 re-review blockers are addressed. Finding → resolution:

  1. Missing-index refusal was unreachable under the canonical-index order. Step 1 now begins by proving the supplied index path resolves to an existing regular file — a missing path or a dangling symlink chain refuses MalformedApplicationIndex at step 1, before canonicalization and the lock (stated rationale: canonicalize requires an existing target). Only then is the canonical index identity resolved and the coordination names derived. §4.1, the §12 row (now naming both raise points: step 1 missing/unresolvable, step 2 parse/strict-type), and C7 (split into its step-1 and step-2 halves, asserting no lockfile is created) are aligned. The refusal classification is preserved, not reclassified.

  2. Lock-release failure had two incompatible outcomes. §8.2 now defines the single observable result shape: exactly one primary outcome (applied, or one typed §12 refusal) plus an optional orthogonal lock-release warning (leftover lockfile path + detail). A release failure is never a refusal and never changes the primary outcome: a committed apply stays success (the index record exists) with the warning attached; a refusal stays that refusal with the warning attached. ApplyIoError is correspondingly narrowed to lock acquisition + staging/publication/commit, and the invariant is stated explicitly: every refusal, ApplyIoError included, is returned only by a run that did not reach the commit point — a refusal always proves pre-commit, so the operator never has to infer which side of the commit an ApplyIoError happened on. Fault-injection case F9 preregisters both shapes.

  3. Incremental Apply contradicted the preservation law for the reserved area. Explicit carry-forward semantics: the input snapshot's song-curation/ proof artifacts describe the input's own application and are superseded, never raw-copied — the output's reserved area contains exactly this application's newly generated curated manifest and report. A §4.2 shape law admits only the two tool-owned artifacts in an input's reserved area; any other file there refuses CorpusTreeDisagreement (so nothing can be silently dropped or blindly carried). §10.1's raw-copy law is scoped to files outside the reserved area. C2 now chains from the first Apply's real published output tree (reserved area included) and asserts the second output's reserved area contains exactly the second application's artifacts.

  4. Output/staging could collide with the index's .lock / .tmp paths. New typed refusal OutputCollidesWithIndexArtifacts { path, artifact }: after the canonical index identity is resolved, step 1 proves the resolved output and staging paths equal neither the canonical index file, nor its lockfile path, nor its temp path — before lock acquisition, so the lock can never be created at a declared output path and step 12 can never collide. ApplicationIndexInsideTree is tightened to equals-or-inside. Adversarial case F10.

Totals: matrix now 55 preregistered cases; refusal surface = 11 reachable Slice-1 refusals + 20 new typed refusals (+ 3 ledger-side members documented as unreachable). Still docs-only; Slice 2 remains PROPOSED / IMPLEMENTATION BLOCKED, awaiting re-review at exact head dca507f.


Generated by Claude Code

Copy link
Copy Markdown
Owner Author

r4 hostile re-review on exact head dca507f9a2e75b57e99ba3422bbd9c1bd26261c5: all four r3 blockers are closed as claimed. I am not accepting r4 yet. Four contract-level blockers remain.

  1. Canonical-path locking still does not serialize hardlink aliases of the same index inode. canonicalize collapses symlink aliases, not hardlinks. Two directory entries hardlinked to the same initial index can therefore derive different <canonical_index_name>.lock files, both acquire them, both read the same old inode, and both pass validation. Commit-by-rename then makes it worse: the first rename replaces only its directory entry with a new inode while the second hardlink still names the old inode; the second applier can then rename its own updated index and both runs can return success with divergent registries. This is the original lost-update bug in a form F8 does not cover, and it falsifies the unqualified §8.1 claim that two aliases of one index always contend on one lock. Close it explicitly: e.g. reject an index target with multiple hardlinks at step 1 (typed, preregistered), or use a locking primitive bound to the opened file identity rather than a path-derived sidecar. A path-canonical sidecar alone is insufficient.

  2. A pre-existing fixed .tmp is treated as owned crash debris and deleted without proof of ownership. §8.1 says that after acquiring the lock, a leftover <canonical_index_dir>/.<canonical_index_name>.tmp is deleted before writing. That name can already be an unrelated file — or even another valid application index whose filename happens to equal this index's temp name. Example: indexes /d/foo and /d/.foo.tmp; applying foo can delete the second index at step 12. The lock proves only that no writer holds foo's lock; it does not prove ownership of the pre-existing temp pathname. Fail closed instead: a pre-existing temp artifact must be a typed refusal (or require an ownership marker that is actually verified); crash recovery can mandate explicit operator deletion. Do not silently unlink an unproven file. Add the two-index filename-collision case.

  3. The supposedly exhaustive failure model has no state for failure of the step-11 publication rename itself. ApplyIoError covers publication, and §8.2 says errors from steps 11–12 must name an enumerated filesystem state, but the table covers: steps 9–10 failure, successful step 11 followed by crash, successful step 12, and failures inside step 12. It does not cover rename(staging → output) returning an error. The observable state is normally staging still present + output absent + index unchanged (or, under an external race, a newly occupied output path), and recovery/cleanup semantics need to be explicit. F4 currently covers a staging write failure and a failure between publication and commit, not a failed publication operation. Add the missing row and fault-injection case; otherwise the claimed enumeration is false at the exact visibility boundary.

  4. The new reserved-area shape law is normative but is not wired through the executable order/taxonomy/matrix. §4.2 says any file other than song-curation/manifest.json and song-curation/apply-report.json is CorpusTreeDisagreement. But §6 step 3 says it enforces only tree agreement and the no-root-songs law; the §12 CorpusTreeDisagreement row still defines only missing root manifest / manifest-vs-chunk disagreement; and §14 has no case for a foreign reserved-area entry. That leaves the new rule in prose while the contract simultaneously claims every executable refusal condition is pinned. Add reserved-area shape validation explicitly to step 3, extend the refusal row's condition/evidence, and preregister a case (including a nested foreign entry, so “contains only two files” has an unambiguous recursive meaning).

Separate integration note, not a Slice-2 semantic blocker: GitHub currently reports PR #184 as mergeable: false on this head. The contract can still be reviewed at this exact SHA, but any conflict resolution that changes the accepted contract file would necessarily require re-review of the new head; do not smuggle resolution edits into an accepted SHA.

Everything r4 intended to repair checks out: missing index now has a reachable pre-lock outcome; release failure is orthogonal to the single primary outcome and no longer masquerades as ApplyIoError; chained snapshots explicitly supersede prior proof artifacts; and output/staging collisions with this index's canonical coordination paths are refused before lock acquisition.

No RED or implementation work should start. Next revision should remain docs-only.

1 similar comment

Copy link
Copy Markdown
Owner Author

r4 hostile re-review on exact head dca507f9a2e75b57e99ba3422bbd9c1bd26261c5: all four r3 blockers are closed as claimed. I am not accepting r4 yet. Four contract-level blockers remain.

  1. Canonical-path locking still does not serialize hardlink aliases of the same index inode. canonicalize collapses symlink aliases, not hardlinks. Two directory entries hardlinked to the same initial index can therefore derive different <canonical_index_name>.lock files, both acquire them, both read the same old inode, and both pass validation. Commit-by-rename then makes it worse: the first rename replaces only its directory entry with a new inode while the second hardlink still names the old inode; the second applier can then rename its own updated index and both runs can return success with divergent registries. This is the original lost-update bug in a form F8 does not cover, and it falsifies the unqualified §8.1 claim that two aliases of one index always contend on one lock. Close it explicitly: e.g. reject an index target with multiple hardlinks at step 1 (typed, preregistered), or use a locking primitive bound to the opened file identity rather than a path-derived sidecar. A path-canonical sidecar alone is insufficient.

  2. A pre-existing fixed .tmp is treated as owned crash debris and deleted without proof of ownership. §8.1 says that after acquiring the lock, a leftover <canonical_index_dir>/.<canonical_index_name>.tmp is deleted before writing. That name can already be an unrelated file — or even another valid application index whose filename happens to equal this index's temp name. Example: indexes /d/foo and /d/.foo.tmp; applying foo can delete the second index at step 12. The lock proves only that no writer holds foo's lock; it does not prove ownership of the pre-existing temp pathname. Fail closed instead: a pre-existing temp artifact must be a typed refusal (or require an ownership marker that is actually verified); crash recovery can mandate explicit operator deletion. Do not silently unlink an unproven file. Add the two-index filename-collision case.

  3. The supposedly exhaustive failure model has no state for failure of the step-11 publication rename itself. ApplyIoError covers publication, and §8.2 says errors from steps 11–12 must name an enumerated filesystem state, but the table covers: steps 9–10 failure, successful step 11 followed by crash, successful step 12, and failures inside step 12. It does not cover rename(staging → output) returning an error. The observable state is normally staging still present + output absent + index unchanged (or, under an external race, a newly occupied output path), and recovery/cleanup semantics need to be explicit. F4 currently covers a staging write failure and a failure between publication and commit, not a failed publication operation. Add the missing row and fault-injection case; otherwise the claimed enumeration is false at the exact visibility boundary.

  4. The new reserved-area shape law is normative but is not wired through the executable order/taxonomy/matrix. §4.2 says any file other than song-curation/manifest.json and song-curation/apply-report.json is CorpusTreeDisagreement. But §6 step 3 says it enforces only tree agreement and the no-root-songs law; the §12 CorpusTreeDisagreement row still defines only missing root manifest / manifest-vs-chunk disagreement; and §14 has no case for a foreign reserved-area entry. That leaves the new rule in prose while the contract simultaneously claims every executable refusal condition is pinned. Add reserved-area shape validation explicitly to step 3, extend the refusal row's condition/evidence, and preregister a case (including a nested foreign entry, so “contains only two files” has an unambiguous recursive meaning).

Separate integration note, not a Slice-2 semantic blocker: GitHub currently reports PR #184 as mergeable: false on this head. The contract can still be reviewed at this exact SHA, but any conflict resolution that changes the accepted contract file would necessarily require re-review of the new head; do not smuggle resolution edits into an accepted SHA.

Everything r4 intended to repair checks out: missing index now has a reachable pre-lock outcome; release failure is orthogonal to the single primary outcome and no longer masquerades as ApplyIoError; chained snapshots explicitly supersede prior proof artifacts; and output/staging collisions with this index's canonical coordination paths are refused before lock acquisition.

No RED or implementation work should start. Next revision should remain docs-only.

claude added 2 commits August 18, 2026 16:43
Base moved by Swang merges (#185#187). No contract-file changes in this
merge.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Kc6r6PhKQCNqArwkijUnzn
1. Hardlink aliases of the index are refused outright: step 1 requires the
   canonical index file's link count to be exactly one, else the new typed
   ApplicationIndexHardLinked. canonicalize collapses only symlinks; two
   hardlinked entries would derive distinct sidecar locks (reviving the
   lost update) and commit-by-rename would split the aliases into divergent
   registries. The unqualified aliases-contend-on-one-lock claim in §8.1 is
   corrected to its symlink-only truth, with the hardlink mechanism spelled
   out and the flock alternative rejected for needing a non-std dependency
   (§15). Matrix case F11 (both appliers through different hardlinks
   refuse).
2. A pre-existing file at the temp coordination path is never silently
   unlinked: the lock proves no live writer, not ownership of bytes already
   at that name (which could be an unrelated file or another index whose
   name equals this index's temp name). Step 1 refuses the new typed
   ApplicationIndexTempExists; crash debris is removed only by the explicit
   §8.2 operator recovery. Matrix case C13 (two-index filename collision:
   the second index is never unlinked).
3. The failure model now enumerates failure of the step-11 publication
   rename itself: staging present, output absent, index unchanged — or,
   under an external race, an output path occupied by an entity this run
   did not create, which is never deleted. F4 extended with the injected
   publication-rename failure.
4. The reserved-area shape law is wired through the executable surface:
   step 3 enforces it explicitly with an unambiguous recursive meaning
   (exactly at most the two tool-owned proof artifacts as regular files at
   the reserved root; any nested entry or subdirectory is foreign), the
   CorpusTreeDisagreement taxonomy row names the condition and carries the
   foreign path, and matrix case K12 exercises both a root-level and a
   nested foreign entry.

Also merges origin/main (Swang #185#187) in the preceding merge commit to
restore mergeability; the merge itself does not touch the contract.

Docs-only; still awaiting independent acceptance; implementation remains
prohibited.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Kc6r6PhKQCNqArwkijUnzn

Copy link
Copy Markdown
Owner Author

Revision r5 (49a775001cae460c56c7ec1a3b4f06a07e811e15) — the four r4 re-review blockers are addressed; mergeability restored. Finding → resolution:

  1. Hardlink aliases defeated the canonical-path lock. Refused outright: step 1 now requires the canonical index file's link count to be exactly one; nlink > 1 refuses the new typed ApplicationIndexHardLinked { path, nlink }, before the lock. §8.1's unqualified claim is corrected to its symlink-only truth, with the hardlink mechanism spelled out (distinct sidecar locks → revived lost update; commit-by-rename splitting the aliases into divergent registries). The file-identity-lock alternative (flock) is explicitly rejected because it needs a non-std dependency, which §15 permits none of. The check runs at step 1 of every applier, so two appliers entering through different hardlinks both refuse — preregistered as F11.

  2. Pre-existing .tmp was deleted without proof of ownership. No silent unlink remains anywhere in the contract: step 1 refuses a pre-existing file at the temp coordination path with the new typed ApplicationIndexTempExists { path } (the lock proves only that no writer holds this index's lock — never that bytes already at the temp name belong to this Apply; that pathname can be an unrelated file or a second real index whose own name equals this index's temp name). Crash debris at the temp path is removed only by the explicit §8.2 operator recovery. The two-index filename-collision case is preregistered as C13, asserting the second index is never unlinked or overwritten.

  3. No failure-model state for the step-11 publication rename itself. Added as its own §8.2 row: staging still present, output absent, index unchanged — or, under an external race, an output path newly occupied by an entity this run did not create, which is never deleted (retry refuses OutputAlreadyExists until the operator resolves it). F4 is extended with the injected publication-rename failure, pinning that state.

  4. Reserved-area shape law was prose-only. Wired through the executable surface: §6 step 3 enforces it explicitly with an unambiguous recursive meaning (exactly at most the two tool-owned proof artifacts as regular files at the reserved root; any other entry — nested files and subdirectories included — is foreign); the CorpusTreeDisagreement taxonomy row names the condition and carries the foreign path as evidence; §4.2 states the recursion; matrix case K12 exercises both a root-level and a nested foreign entry.

Mergeability: origin/main (Swang #185#187) is merged in a separate merge commit (d24cc1b) that does not touch the contract file (git diff dca507f..d24cc1b -- docs/proposals/ is empty); all r5 semantic changes live in one dedicated docs commit on top, so the reviewed artifact delta is exactly that commit.

Totals: matrix now 58 preregistered cases; refusal surface = 11 reachable Slice-1 refusals + 22 new typed refusals (+ 3 ledger-side members documented as unreachable). Still docs-only; Slice 2 remains PROPOSED / IMPLEMENTATION BLOCKED, awaiting re-review at exact head 49a7750.


Generated by Claude Code

Copy link
Copy Markdown
Owner Author

r5 hostile re-review on exact head 49a775001cae460c56c7ec1a3b4f06a07e811e15: the four r4 blockers are closed as claimed, and the merge-main commit is clean with respect to docs/proposals/. I am not accepting r5 yet. Four filesystem-protocol blockers remain.

  1. The temp-artifact check happens before the index lock, so a second compliant applier can misclassify a live writer's temp as stale/foreign debris. Step 1 currently checks .<index>.tmp and returns ApplicationIndexTempExists before attempting .<index>.lock. During the first applier's step 12, that temp legitimately exists while the lock is legitimately held. A second applier arriving in that window therefore returns ApplicationIndexTempExists, never observes ApplicationIndexLocked, and has no proof that the temp is stale. This defeats the ownership argument r5 was trying to establish and makes operator recovery dangerous. Derive paths before the lock, acquire the lock first, then inspect the temp path under the held lock. A live writer must always lose at the lock boundary; only a holder of the lock may classify a pre-existing temp as non-live debris/foreign occupancy. Add a concurrency case specifically during step-12 temp lifetime.

  2. The lock sidecar still has the exact ownership-collision problem r5 fixed for .tmp. A real second index can be named d/.foo.lock while the first is d/foo. Applying foo sees the second index at its lock path and returns ApplicationIndexLocked; §8.2 stale-lock recovery then instructs the operator to verify no apply is running and delete that path. That can delete the second real index. An empty sidecar carries no identity proving it is Griff's lock. Either give the lock an ownership format that can be validated before recovery, introduce a distinct "lock coordination path occupied by an unowned entity" refusal, or otherwise make recovery incapable of deleting an unproven path. Add the symmetric two-index case to C13/F*; fixing .tmp but not .lock leaves the same destructive namespace collision one suffix away.

  3. Step-12 temp creation is still a TOCTOU write. §8.1 says the temp "was proven absent at step 1, so the write never overwrites a pre-existing file". Absence at step 1 cannot prove absence at step 12. Even after moving the check under the lock, a non-Apply actor can create that pathname later. The write itself must use atomic no-clobber creation (File::create_new / OpenOptions::create_new(true)), and AlreadyExists at that point must become a pre-commit ApplyIoError (or another accepted typed refusal) without touching the entity. Rust's std API explicitly provides create_new for exactly this TOCTOU case, so this requires no dependency. Preregister the late-appearance case.

  4. rename(staging, output) is not a no-clobber publication primitive, so the step-11 external-race model is false and two compliant applies with different indexes can interfere. std::fs::rename is documented as replacing to when it already exists; on Unix, a directory source may replace an existing empty directory. The contract currently assumes an output that appears after step-1 preflight either makes publication fail or remains untouched. Counterexample without a malicious actor: Apply A chooses an output path equal to Apply B's derived staging path. They use different indexes, so their index locks do not serialize them. B creates its staging directory at step 9; while it is still empty, A's step-11 rename can replace that live staging directory on POSIX. The result violates both runs' path ownership and the claim that foreign output is never deleted. This needs an atomic no-replace publication primitive, or an explicitly accepted serialization/threat-model redesign that makes such overlap impossible. Plain std::fs::rename plus a preflight existence check cannot prove it.

Two non-blocking corrections while touching the text:

  • F4 / the failure table should say staging may remain after a step-11 rename failure: §8.2 also mandates best-effort cleanup, so a successful cleanup leaves no staging and retry need not hit OutputAlreadyExists. The current F4 wording makes residue/retry refusal unconditional.
  • The §8.1 rationale that a file-identity flock alternative "would need a non-std dependency" is stale for this repository's Rust 1.92 baseline. std::fs::File::{lock,try_lock,unlock} has been stable since Rust 1.89. This does not by itself solve commit-by-rename hardlink alias splitting, so the nlink == 1 refusal can remain a valid design choice; only the dependency claim is false.

Everything r5 intended to close otherwise checks out: initial hardlink aliases refuse before side effects; a pre-existing temp is no longer silently unlinked; the publication-rename error state is represented; reserved-area shape is now executable through step 3/taxonomy/K12; PR #184 is mergeable again; and d24cc1b -> 49a7750 changes only the contract file.

No RED or implementation work should start. Next revision should remain contract/docs-only.

1. Temp inspection moved under the held lock: step 1 now acquires the lock
   first, so a live writer's step-12 temp is unreachable for a non-holder —
   a second applier always loses at the lock boundary
   (ApplicationIndexLocked) and can never misclassify a live commit's temp
   as debris. Concurrency case C14 preregisters exactly the step-12 temp
   lifetime window.
2. The lockfile gains a validated ownership marker (one line,
   song-curation.lock.v1). On contention the occupant is classified by
   content: marker or empty file -> ApplicationIndexLocked; anything else
   -> the new typed ApplicationIndexLockPathOccupied, and the §8.2
   stale-lock recovery validates content before any deletion, so a real
   second index named .foo.lock can never be deleted by recovery (case
   C15, symmetric to C13).
3. The step-12 temp write is an atomic no-clobber creation
   (OpenOptions::create_new — std's answer to exactly this TOCTOU); the
   step-1 absence check is a fail-fast courtesy, not the safety argument;
   AlreadyExists at creation is a pre-commit ApplyIoError with the late
   occupant untouched (case F12).
4. Publication is stated honestly as non-no-clobber: compliant-applier
   overlap is made impossible by construction — the .name.apply-staging
   pattern is a reserved namespace refused for outputs (OutputNameReserved,
   case F13) and staging creation is an atomic create_dir — while the
   residual (an external actor's empty directory at the output path being
   replaced by POSIX rename) is documented and accepted with rationale
   (std has no RENAME_NOREPLACE); a foreign file or non-empty tree makes
   the rename fail with the occupant untouched.

Non-blocking corrections: the step-11 failure row and F4 now say staging
MAY remain (best-effort cleanup can succeed); the stale flock claim is
replaced — std::fs::File::{lock,try_lock} is stable since Rust 1.89, and
nlink == 1 stays the chosen design because file-identity locking would not
repair commit-by-rename alias splitting.

Docs-only; still awaiting independent acceptance; implementation remains
prohibited.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Kc6r6PhKQCNqArwkijUnzn

Copy link
Copy Markdown
Owner Author

Revision r6 (d4413315e88b56e5843bef23687be75c5dfaf2c3) — the four r5 re-review blockers and both non-blocking corrections are addressed. Finding → resolution:

  1. Temp check before the lock let a second applier misclassify a live writer's temp. Reordered: step 1 derives all coordination paths (pure), acquires the lock, and only under the held lock inspects the temp path. The ordering is stated as load-bearing in §6: during a live writer's step 12 the temp legitimately exists while the lock is held, so a non-holder always loses at the lock boundary (ApplicationIndexLocked) and ApplicationIndexTempExists is unreachable for it — only a lock holder may classify a pre-existing temp, and even then nothing is unlinked. The §12 rows pin the relative ordering. Concurrency case C14 preregisters exactly the step-12 temp-lifetime window.

  2. The lock sidecar had the same ownership-collision hazard as .tmp. The lockfile now carries a validated ownership marker (one fixed line, song-curation.lock.v1). On contention the occupant is classified by content: the marker, or a zero-byte file (acquisition interrupted between creation and marker write — deleting an empty file destroys no data), is a Griff lock → ApplicationIndexLocked; any other content → the new typed ApplicationIndexLockPathOccupied, and the §8.2 stale-lock recovery now validates content before any deletion — an unproven occupant (e.g. a real second index named d/.foo.lock) can never be deleted by recovery. Symmetric case C15 added next to C13; the leftover-temp recovery is likewise restricted to operator inspection, never blind deletion.

  3. Step-12 temp creation was a TOCTOU write. The temp write is now an atomic no-clobber creation (OpenOptions::create_new(true)std's own answer to exactly this race). The under-lock step-1 absence check is explicitly demoted to a fail-fast courtesy, not the safety argument; AlreadyExists at creation refuses with a pre-commit ApplyIoError and the late-appearing occupant is left untouched. Case F12 (late appearance between step 1 and step 12) preregistered.

  4. rename(staging, output) is not no-clobber, and two compliant applies could overlap. Both halves addressed. Compliant overlap is now impossible by construction: .<name>.apply-staging is a reserved namespace — step 1 refuses any output whose final component matches it (new typed OutputNameReserved, case F13) — so no compliant output can equal any apply's staging path; and staging creation is an atomic no-clobber create_dir, so two runs deriving one staging path serialize with a loser refusal. The residual is stated honestly instead of papered over: §8.1 now says outright that POSIX rename replaces an existing empty directory at the destination (non-empty fails ENOTEMPTY; a file destination fails, occupant untouched → ApplyIoError), and that an external actor's empty directory appearing at the output mid-run would be silently replaced — an accepted, documented residual, with the rationale that closing it needs platform-specific renameat2(RENAME_NOREPLACE) outside std and that a mid-run mutator of declared paths is already outside every other guarantee.

Non-blocking corrections: the §8.2 step-11 failure row and F4 now say staging may remain (best-effort cleanup can succeed), with retry refusal conditional on actual residue; the stale flock dependency claim is replaced — std::fs::File::{lock, try_lock} is stable since Rust 1.89 (within this repo's MSRV), and nlink == 1 remains the chosen design on its merits: a file-identity lock would serialize appliers but not repair commit-by-rename alias splitting.

Totals: matrix now 62 preregistered cases; refusal surface = 11 reachable Slice-1 refusals + 24 new typed refusals (+ 3 ledger-side members documented as unreachable). Still docs-only (the r6 delta is one commit touching only the contract file); Slice 2 remains PROPOSED / IMPLEMENTATION BLOCKED, awaiting re-review at exact head d441331.


Generated by Claude Code

Copy link
Copy Markdown
Owner Author

r6 hostile re-review on exact head d4413315e88b56e5843bef23687be75c5dfaf2c3: all four r5 blockers and both non-blocking corrections are closed as claimed. The r5→r6 delta is exactly one docs commit (+116/-50), PR is mergeable, and I found one remaining contract blocker.

Blocker — the lock ownership marker is itself published non-atomically

The protocol is now create_new(lock_path) followed by writing the fixed marker {"schema":"song-curation.lock.v1"}. Creation is atomic; publication of the marker bytes is not. The contract currently classifies only three observed contents: exact marker → ApplicationIndexLocked; zero bytes → also ApplicationIndexLocked; anything else → ApplicationIndexLockPathOccupied, and recovery may delete marker/empty but must never delete other content.

That leaves two states the contract itself can produce:

  1. Live contention during marker write. A second compliant applier can observe the lock path after create_new but while the first writer has only written a non-empty prefix of the marker. A short/partial write is legal; write_all may require multiple writes. The second applier then sees "anything else" and returns ApplicationIndexLockPathOccupied even though a live compliant writer owns the lock. So the new claim that a live writer is classified at the lock boundary as ApplicationIndexLocked is not actually guaranteed by the mechanism.

  2. Crash during marker write. The same non-empty prefix can be left permanently after a crash. On the next run it is ApplicationIndexLockPathOccupied; §8.2 says any non-marker/non-empty occupant is unproven and must not be deleted. But in this case it is Griff's own crash debris. The supposedly complete crash-recovery model has therefore created a self-produced state from which its own mandated recovery forbids recovery.

The zero-byte special case does not close this. It also weakens the ownership claim in the opposite direction: a pre-existing unrelated empty file is not actually proven to be Griff-owned merely because deleting it loses no payload bytes; pathname existence/metadata can itself be meaningful. This directly conflicts with the PR body's stronger statement that unproven coordination-path occupants are never deleted.

Please make lock acquisition/recovery semantics closed over all prefixes / partial-marker states, not just {empty, exact}. I would not prescribe the mechanism from review, but the contract needs a mechanically testable rule that satisfies both properties:

  • live compliant ownership can never be misclassified as foreign occupancy; and
  • recovery never deletes an occupant whose Griff provenance has not actually been established.

One possible design direction is to separate contention classification from recovery authority: conservative prefix/empty states may safely block as ApplicationIndexLocked while live, but only an exact committed marker is automatically deletable; ambiguous interrupted-acquisition states require an explicit operator-proven recovery path. Another is to change the acquisition primitive so the ownership marker becomes visible atomically. Whichever design is chosen, preregister a pause/fault case between lock creation and completed marker publication, including a non-empty partial marker.

Everything else in r6 held under adversarial review:

  • temp inspection is now correctly under the held lock; C14 pins the live step-12 window;
  • step-12 temp creation is genuinely no-clobber via create_new; F12 closes the late-occupant TOCTOU;
  • OutputNameReserved + atomic staging create_dir closes overlap among contract-following appliers; the remaining external empty-directory rename race is explicitly scoped out rather than falsely denied;
  • hardlink refusal rationale is now correct on Rust 1.92;
  • step-11 cleanup wording is consistent (staging may remain);
  • taxonomy counts and the 62-case matrix are internally consistent.

No RED or implementation work should start yet. If the lock-marker publication/recovery state machine is closed without changing other accepted laws, I currently see no further contract-level blocker.

…e machine

The r6 ownership marker was published non-atomically (create_new is
atomic, write_all is not), so a compliant applier could produce a
non-empty marker prefix at the lock path — live during a concurrent
observation, or permanently after a crash — and the {empty, exact}
classification would misread it as foreign occupancy, while recovery
forbade deleting the contract's own debris.

The state machine is now closed over all prefixes, with contention
classification and recovery authority deliberately separated:

- Contention classification (live-safe): any byte-prefix of the canonical
  marker — empty, partial, or complete — classifies ApplicationIndexLocked,
  so live compliant ownership can never be misclassified as foreign; only
  non-prefix content is ApplicationIndexLockPathOccupied.
- Recovery authority (provenance-gated): only the exact complete marker is
  automatically deletable by stale-lock recovery; an empty or partial
  prefix is ambiguous (an unrelated file can resemble one, and pathname
  existence/metadata can themselves be meaningful) and takes the explicit
  operator-proven path — provenance-established deletion or
  non-destructive relocation out of the coordination namespace; non-prefix
  occupants are never deleted or relocated by any mandated recovery.

The atomic-visibility alternative (private file + std::fs::hard_link
publication, the mail-spool lockfile protocol) is recorded as considered
and not chosen: it adds a second transient coordination namespace with its
own debris states, while prefix-closed classification needs no new names
and is mechanically testable.

Matrix: new fault-injection case C16 (pause between lock creation and
completed marker publication — live observation and permanent crash
variants, both with a non-empty partial marker); C12/C15 aligned with the
exact-marker auto-delete rule.

Docs-only; still awaiting independent acceptance; implementation remains
prohibited.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Kc6r6PhKQCNqArwkijUnzn

Copy link
Copy Markdown
Owner Author

Revision r7 (47e734cfbf1a6bd90c1bd2a035cdc68692378e96) — the one remaining r6 blocker is addressed: the lock-marker state machine is closed over all prefixes.

The chosen design is the review's first direction — contention classification separated from recovery authority — because it needs no new coordination names and every rule is mechanically testable:

  • Contention classification (live-safe, conservative). §8.1 now states outright that create_new is atomic but marker publication is not, and that a compliant applier can itself produce a non-empty prefix of the marker at the lock path (short/partial write, or a crash mid-write). Classification is closed over exactly that: an occupant whose content is a byte-prefix of the canonical marker — empty, partial, or complete — classifies ApplicationIndexLocked. A live compliant writer can only ever be observed in a prefix state, so live ownership can never be misclassified as foreign occupancy. Only non-prefix content is ApplicationIndexLockPathOccupied.
  • Recovery authority (provenance-gated). Only the exact, complete marker is automatically deletable by stale-lock recovery. An empty or partial-prefix occupant is ambiguous — interrupted Griff acquisition, or an unrelated pre-existing file that merely resembles one; the contract now says explicitly that "deleting it loses no payload bytes" is not a provenance proof, since pathname existence and metadata can themselves be meaningful (this also retires r6's zero-byte special case, which weakened the ownership claim in the opposite direction). Ambiguous states take the explicit operator-proven path: after confirming no apply is running, delete only with independently established Griff provenance, otherwise relocate the occupant out of the coordination namespace — non-destructive, unblocks the lock path, preserves the evidence. Non-prefix occupants are never deleted or relocated by any mandated recovery.
  • The atomic-visibility alternative (write the marker to a private name, publish via std::fs::hard_link, which fails on an existing destination — the classic mail-spool lockfile protocol) is recorded in §8.1 as considered and not chosen: it introduces a second transient coordination namespace with its own debris states, while prefix-closed classification needs no new names.

Both required properties now hold mechanically: (1) live compliant ownership is never misclassified as foreign — every state a live writer can present is a prefix, and every prefix classifies ApplicationIndexLocked; (2) recovery never deletes an occupant whose Griff provenance has not actually been established — auto-deletion requires the exact marker, everything else is operator-proven or untouchable.

Preregistered: new fault-injection case C16 — a pause/fault between lock creation and completed marker publication with a non-empty partial marker, in both variants: (a) a concurrent applier observing it live refuses ApplicationIndexLocked, never ApplicationIndexLockPathOccupied; (b) a crash leaves it permanently, subsequent applies refuse ApplicationIndexLocked, recovery does not auto-delete, and the operator-proven path unblocks. C12 and C15 are aligned with the exact-marker auto-delete rule (an index document is not a byte-prefix of the marker, so C15's second index remains untouchable).

Totals: matrix now 63 preregistered cases; refusal surface unchanged (11 reachable Slice-1 + 24 new typed + 3 documented unreachable). The r6→r7 delta is one docs commit touching only the contract file (+43/−16). Slice 2 remains PROPOSED / IMPLEMENTATION BLOCKED, awaiting re-review at exact head 47e734c.


Generated by Claude Code

Copy link
Copy Markdown
Owner Author

Final hostile re-review on exact head 47e734cfbf1a6bd90c1bd2a035cdc68692378e96: CONTENT REVIEW PASS — no remaining contract-level blockers found.

I re-checked the r6→r7 delta rather than accepting the summary. It is exactly one docs commit (+43/-16) touching only docs/proposals/song-curation-slice-2-transactional-apply.md; PR #184 remains mergeable.

The last blocker is genuinely closed. The lock-marker protocol now distinguishes two different questions that r6 had conflated:

  • contention classification is prefix-closed: empty, partial, and complete marker contents all refuse ApplicationIndexLocked, so every byte state a live compliant writer can expose is conservatively recognized as lock contention;
  • recovery authority is provenance-gated: only the exact complete marker is automatically deletable; empty/partial prefixes are explicitly ambiguous and require independently established Griff provenance for deletion, otherwise non-destructive relocation; non-prefix occupants are never deleted or relocated by mandated recovery.

That closes both failure modes from round 6: a live partial marker cannot be misclassified as foreign occupancy, and a crash-created partial marker no longer produces an unrecoverable self-state. The zero-byte weakness is also fixed: emptiness is no longer treated as ownership proof.

C16 mechanically pins both sides of the state machine: live observation of a non-empty partial marker must yield ApplicationIndexLocked, while a permanent crash-prefix must block until the provenance-gated recovery path is used. C12 and C15 are consistent with exact-marker-only automatic deletion and foreign-occupant preservation.

I also checked the secondary edges introduced across r5/r6: temp inspection remains under the held lock; late temp creation remains atomic no-clobber; hardlink aliases remain fail-closed; reserved staging namespace still closes compliant-applier publication overlap; the external empty-directory rename residual is explicitly scoped rather than denied; reserved-area carry-forward/shape, staged self-check, primary-outcome semantics, and the 24-new-refusal surface remain internally consistent. The ApplyIoError taxonomy covers lock-acquisition I/O, so a returned marker-write I/O failure does not require a new refusal member.

Verdict at this exact SHA: the Slice-2 transactional Apply contract is content-acceptable and has passed hostile falsification review. I found no reason to request an r8.

One governance boundary remains separate from the content verdict: §16 requires independent acceptance by someone other than the PR author. The GitHub connector identity here is PhysShell, which is also the PR author, so this comment is intentionally not represented as a formal GitHub APPROVE or the repository's independent-acceptance record. Implementation should remain blocked until that independent acceptance is recorded against this exact SHA. Any head movement after that, if it changes the contract, reopens review.

@PhysShell
PhysShell merged commit 31e5939 into main Aug 20, 2026
28 of 29 checks passed
PhysShell pushed a commit that referenced this pull request Aug 20, 2026
…ntract

ADR-0033 Slice 2 transactional-Apply acceptance contract is independently
ACCEPTED at exact reviewed head 47e734c
after six hostile review rounds (PR #184; 24 contract blockers found and
resolved across r2–r7; final re-review found none; CI 14/14 green).
Acceptance is recorded here per repository governance (AGENTS.md routing;
S7 Slice C precedent at 7d0c0cb) — the PR merge itself was not the
acceptance act. Authorizes only Slice 2 implementation under the frozen
§14 RED→GREEN matrix and closed §12 refusal surface; Slice 3, the
controlled pilot, and real-/full-corpus labeling remain separately gated.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Kc6r6PhKQCNqArwkijUnzn
PhysShell pushed a commit that referenced this pull request Aug 20, 2026
…only

Update the stale Status lines in the Slice 2 contract and the proposals
index to reflect the acceptance recorded in docs/decisions.log.md @
bad7b44. Lifecycle metadata only: the normative reviewed contract remains
the exact artifact at 47e734c (PR #184),
no contract law changes, and acceptance is not reopened by this edit.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Kc6r6PhKQCNqArwkijUnzn
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