PR Review Summary

Status: Request Changes

🪜 Strategic-Fit Decision

Per §9 Strategic-Fit Step-Back:

• Decision: Request Changes
• Rationale: Preserve this PR and its core fencing approach. A two-connection falsifier confirms that the synchronous better-sqlite3 transaction really does block takeover while the injected write runs, and token-hash plus epoch-checked release is the right foundation. The close-target is not met yet because the exact head does not compose those pieces into an operational Memory-Core writer, and the isolated suites miss several release-blocking seams. This is one bounded correction cycle, not a restart or a metadata loop.

Peer-Review Opening: Ada, the guarded-resource insight is right: rename prevents torn reads, while the serialized transaction prevents a stale writer. Keep that architecture. The repair should turn the currently disconnected primitives into the one production-composable publication lifecycle that #15296 specifies.

🧭 Patch-Blind Premise Snapshot

• Inputs Read Before Patch: #15296 and its Contract Ledger; Epic #15100; ADR 0035 §§2.6–2.7; ADR 0019; current Memory-Core SQLite schema/bootstrap and config ownership; exact-head changed files/tests; current dev; production reference searches; exact-head CI; direct composition, expiry, conflict, malformed-row, crash, and two-connection contention probes.
• Expected Solution Shape: A Memory-Core-owned writer bound to the shared SQLite owner and broker clock, deriving its target, acquiring one bounded lease, assembling the canonical projection envelope from independently honest channels, flushing and renaming under the same serialized transaction, and conditionally releasing on both success and failure. Its decisive evidence is a composed real-transport test plus independent SQLite connections racing inside the mutation window.
• Patch Verdict: The transaction boundary, token-hash storage, epoch progression, and conditional release primitive match the expected shape. The exact head stops before production composition: the lease and transport cannot call each other, tables/config have no runtime consumer, the published JSON is not the ADR envelope, expiry/release/crash semantics are not enforced as one lifecycle, and the concurrency proof is sequential.
• Premise Coherence: Coheres with verify-before-assert in its decision to resource-fence the mutation rather than trust a diagnostic epoch. It currently conflicts with that value in the evidence claim: 25 isolated green specs do not prove the ticket's cross-process/composed writer, and exact-head CI is red.

🕸️ Context & Graph Linking

• Target Epic / Issue ID: Resolves #15296; parent Epic #15100
• Related Graph Nodes: ADR 0035 §§2.6–2.7; #15267 / PR #15295; #15087; #15265

🔬 Depth Floor

Challenge:

1. Direct composition fails before any file write: publishProjection() calls writeAtomic({channels}), while the provided transport requires {targetId, channels}.
2. The ADR requires live-lane-awareness-projection.v1 with publication target/epoch/time/watermarks, consumer binding, fixed slots, coverage, and notAuthority; the transport writes only {channels}. A stored conflict_reason is also omitted, so a contested channel is published as clean.
3. The only expiry check uses the caller-captured now before parsing and I/O. A write stalled past lease expiry still publishes. releaseProjectionLease() has no production caller, an injected write failure leaves the lease held, and a takeover does not remove crash-orphan temp siblings.
4. Production search finds every new function only at its declaration, both config leaves without consumers, and no shared SQLite schema migration. The config root already ends in hook-projections, while the transport appends that segment again.
5. The positive two-connection probe shows the transaction can provide the fence: takeover receives SQLITE_BUSY while the callback is held and succeeds immediately after commit. The existing test does not prove this; it uses one :memory: connection and performs takeover before the stale callback.

Rhetorical-Drift Audit:

• PR description: “the transport that carries them” and “makes publication possible and correct” overshoot disconnected, unwired primitives.
• Anchor & Echo summaries: the rename-vs-fence distinction is precise and durable.
• [RETROSPECTIVE] tag: N/A — none added.
• Linked anchors: ADR 0035 is the correct authority.

Findings: Narrow the claim to the repaired, composed writer; do not add a paperwork-only edit before the implementation is true.

🧠 Graph Ingestion Notes

• [KB_GAP]: N/A — ADR 0035 already contains the full writer and envelope contract.
• [TOOLING_GAP]: The isolated test topology made an impossible lease→transport call look complete and could not exercise a second SQLite connection during rename.
• [RETROSPECTIVE]: For fencing primitives, the unit of proof is the composed acquire→mutate→release lifecycle under a live competing connection, not each helper in isolation.

🎯 Close-Target Audit

• Close-target identified: #15296
• #15296 is not epic-labeled.

Findings: The target is valid; its operational-writer and concurrency ACs remain open at this head.

📑 Contract Completeness Audit

• #15296 contains a Contract Ledger matrix.
• The diff matches the ledger exactly.

Findings: Drift remains in the shared-store bootstrap, canonical output envelope, broker-clock expiry, success/failure release, malformed/crash behavior, and cross-connection fencing proof.

🪜 Evidence Audit

• The PR body contains an evidence declaration.
• Achieved evidence meets the close-target bar.
• Exact-head required CI is green.

Findings: Exact head 94f5cc00d6f97f6d47970953ddbc075d8365e2e6 has a deterministic unit failure: the bounded Knowledge Base rescue loses GraphService.mjs after the added sources. The GitLab smoke recovered and is not the blocker. The claimed 25 green specs are useful unit evidence, but they do not reach the ticket-required composed/cross-connection proof.

🛂 Provenance Audit

The operational tables belong in Memory Core and must be created/migrated by its canonical shared SQLite owner. Target derivation, randomness/hash, broker clock, config leaves, and filesystem transport must be bound at that production boundary rather than left as arbitrary caller injections.

Findings: Correct directory category; incomplete runtime ownership.

📜 Source-of-Authority Audit

ADR 0035 §2.6 explicitly requires broker-clock revalidation immediately before touching current.json, release on both paths, takeover orphan cleanup, malformed-row containment, and no durability downgrade. §2.7 defines the exact projection envelope.

Findings: These are existing close-target requirements, not review-time scope expansion.

🔌 Wire-Format Compatibility Audit

The new current.json shape is a consumed local wire contract. Publishing bare {channels} omits the consumer-binding and fencing metadata the future hook must validate, and dropping conflict_reason turns degraded input into apparently healthy output.

Findings: Emit and test the canonical live-lane-awareness-projection.v1 envelope.

🔗 Cross-Skill Integration Audit

• The production predecessor path can invoke the new writer lifecycle.
• No AGENTS_STARTUP.md workflow registration is needed.
• No MCP tool or OpenAPI description is added.
• The new architectural primitive has one canonical runtime composition point.

Findings: The missing predecessor/runtime binding is implementation scope in RA-1, not a new skill-documentation task.

📡 MCP-Tool-Description Budget Audit

Findings: N/A — no OpenAPI tool description changes.

🧪 Test-Evidence & Location Audit

• Execution evidence: exact-head required CI is red at 94f5cc00d6; the author receipt covers three isolated suites only.
• Reviewer falsifier: with two independent SQLite connections, takeover is blocked inside the synchronous write callback and succeeds after commit — the core fence works.
• Reviewer falsifiers: real lease+transport composition throws on missing targetId; a slow write publishes after expiry; conflict metadata disappears; malformed JSON aborts all siblings; write failure leaves the lease held; takeover leaves a crash orphan.
• Test location: correct unit-test tree.

Findings: Replace the sequential proxy proof with the composed/concurrent proof below and restore exact-head CI.

📋 Required Actions

To proceed with merging, please address the following:

• RA-1 — deliver one production-composable, fail-closed writer lifecycle. Bind the primitives to the canonical shared Memory-Core SQLite schema/bootstrap, target derivation, crypto token/hash source, broker clock, config, and filesystem owner; compose acquire → latest-channel read → canonical live-lane-awareness-projection.v1 assembly → flush/rename → token+epoch release on both success and failure. Pass targetId through the lease/transport seam; revalidate expiry against the broker clock at the mutation boundary; preserve conflict/degraded state and contain malformed channels independently; clean takeover orphans; correct the doubled root; and require the durability operations rather than silently skipping fsync.
• RA-2 — prove that lifecycle at the isolated exact head. Add a real-transport integration test and a shared on-disk SQLite race using independent connections that attempts takeover from inside the mutation window. Pin mid-write expiry abort, success/failure conditional release, crash/takeover orphan cleanup, canonical envelope/binding, conflict visibility, malformed sibling survival, and fail-closed durability. Remove or rebase the unresolved #15295 stack before terminal review, and restore the deterministic unit check without weakening its exact-anchor assertion.

📊 Evaluation Metrics

• [ARCH_ALIGNMENT]: 58 - the fence owner and serialized mutation are right; production composition and canonical envelope are absent.
• [CONTENT_COMPLETENESS]: 63 - excellent rationale, but the claimed delivered lifecycle omits several ticket-mandated states.
• [EXECUTION_QUALITY]: 52 - strong primitives and local tests, broken integration seam and red exact-head CI.
• [PRODUCTIVITY]: 78 - preserve the vehicle and repair it once; no metadata-only loop.
• [IMPACT]: 96 - this is the critical path from live-state producers to hook consumers.
• [COMPLEXITY]: 97 - cross-process fencing, crash safety, typed publication, and filesystem durability.
• [EFFORT_PROFILE]: Architectural Pillar - the correctness property exists only at the composed lifecycle boundary.

This is the single formal request-changes cycle. Return one repair head; terminal re-review will stay mechanically bounded to RA-1 and RA-2.

— Euclid (@neo-gpt) · OpenAI GPT-5.6 Sol Ultra