Skip to content

fix(cp): plan, re-read and verify before an IX-F merge deletes a row - #303

Draft
netravnen wants to merge 1 commit into
dev-nextfrom
fix/cp-ixf-merge-gates
Draft

fix(cp): plan, re-read and verify before an IX-F merge deletes a row#303
netravnen wants to merge 1 commit into
dev-nextfrom
fix/cp-ixf-merge-gates

Conversation

@netravnen

Copy link
Copy Markdown
Contributor

applyIxfMerges wrote from the snapshot the table was rendered from and
PUT only ipaddr4/ipaddr6 onto the keeper, then DELETEd the sibling. Two
consequences, both silent. Any preserved field carried solely by the
doomed row -- speed, is_rs_peer, bfd_support, operational, notes -- was
destroyed by that DELETE with nothing recorded, the exact data-loss class
buildMergePlan was written for and which applyIxfMerges never called. And
because nothing was re-read, a row edited between render and Apply was
merged on premises that no longer held.

The conflict resolver already did all of this correctly. This ports its
discipline -- live re-read, re-run the same predicate on the live rows,
absorb before destroying, verify the write landed before deleting the
only other copy -- rather than introducing a second mechanism.

Split v4-only and v6-only rows are created independently and commonly
disagree on speed or is_rs_peer, so hard-failing every disagreement the
way the conflict resolver's gate 8 does would block most real merges.
Instead the audit table now shows, per candidate, what the keeper
absorbs and which value the DELETE discards, and a row with a
disagreement cannot be applied until the admin ticks an acknowledgement
for that specific disagreement.

Changes:

  • buildMergePlan accepts family: "both", skipping ipaddr4 and ipaddr6
    together for the IX-F path, which sets both from the IX-F-confirmed
    pair and so owns neither field's merge decision.
  • Add summarizeIxfMergeEffects: what a merge absorbs, what it discards,
    and a stable key identifying the disagreement set.
  • applyIxfMerges takes ixfMap, re-reads both rows, re-runs
    findIxfMergeCandidates on the live pair and aborts if it no longer
    merges, aborts when the acknowledgement does not match the live
    disagreement set, absorbs the doomed row's gaps into the keeper PUT,
    and re-reads the keeper to confirm every field before the DELETE.
  • Audit-log outcomes gain absorbed and acknowledgedMismatches.
  • The audit modal gains a "Merge effect" column with the per-row
    acknowledgement, holds ixfMap at modal scope, blocks Apply on
    unreviewed rows, reports the counts in the confirm dialog, and
    refreshes after any aborted row.
  • Test shim: fetchMap entries accept { __sequence: [...] }, serving
    responses in order and repeating the last. A read-modify-verify flow
    cannot be tested without a URL that answers differently after a write.

Security:

  • An IX-F merge can no longer delete the last carrier of a value: the
    keeper must be re-read and confirmed to hold it first.
  • A merge can no longer proceed on a stale premise; if the live pair
    no longer satisfies findIxfMergeCandidates, the row aborts unwritten.
  • With no IX-F map the loop aborts rather than merging unconfirmed, the
    same "200% sure over 90%" stance as conflict-resolver gates 5 and 6.

Testing:

  • node --test: 532 tests, 531 pass, 1 skipped (live tests are opt-in).
  • New tests/cp-ixf-merge-apply.test.js drives the real write path through
    the shim, asserting on recorded request methods -- including that no
    DELETE is issued when the read-back disagrees, when the live re-check
    fails, when a mismatch is unacknowledged, and when the IX-F map is
    absent. Candidates come from the real findIxfMergeCandidates, so a
    fixture that stops being a valid merge fails loudly.
  • A companion case proves an acknowledged mismatch still completes, so
    the gate is not vacuously closed.
  • Two self-tests for the new shim capability in browser-shim.test.js.
  • Each of the four safeguards reverted individually and confirmed red.
  • DOM wiring for the new column is covered by the AGENTS.md smoke test,
    per this repo's testing split.

Backwards Compatibility:

  • applyIxfMerges gains an ixfMap parameter in second position; its only
    caller is updated here. It is newly exported on the test hooks.
  • buildMergePlan's existing 4 and 6 callers are unaffected.
  • Audit-log entries gain fields; readers tolerate unknown keys.

Assisted-by: Claude:claude-opus-5


Stack created with GitHub Stacks CLIGive Feedback 💬

@netravnen
netravnen force-pushed the fix/cp-ixf-merge-gates branch from d1ade79 to 69dd537 Compare August 19, 2026 22:27
@netravnen
netravnen force-pushed the fix/cp-ixf-merge-gates branch from 69dd537 to c393bdb Compare August 19, 2026 22:28
@netravnen
netravnen force-pushed the fix/cp-ixf-merge-gates branch from c393bdb to 67b3ef9 Compare August 19, 2026 22:29
@netravnen
netravnen force-pushed the fix/cp-ixf-merge-gates branch from 67b3ef9 to fd8361b Compare August 19, 2026 22:31
@netravnen
netravnen force-pushed the fix/cp-ixf-merge-gates branch from fd8361b to 39851b2 Compare August 19, 2026 23:05
@netravnen
netravnen force-pushed the fix/cp-ixf-merge-gates branch from 39851b2 to 580639b Compare August 19, 2026 23:06
@netravnen
netravnen force-pushed the fix/cp-ixf-merge-gates branch from 580639b to 0b52565 Compare August 19, 2026 23:07
Base automatically changed from fix/cp-modal-cancellation to dev-next August 19, 2026 23:09
applyIxfMerges wrote from the snapshot the table was rendered from and
PUT only ipaddr4/ipaddr6 onto the keeper, then DELETEd the sibling. Two
consequences, both silent. Any preserved field carried solely by the
doomed row -- speed, is_rs_peer, bfd_support, operational, notes -- was
destroyed by that DELETE with nothing recorded, the exact data-loss class
buildMergePlan was written for and which applyIxfMerges never called. And
because nothing was re-read, a row edited between render and Apply was
merged on premises that no longer held.

The conflict resolver already did all of this correctly. This ports its
discipline -- live re-read, re-run the same predicate on the live rows,
absorb before destroying, verify the write landed before deleting the
only other copy -- rather than introducing a second mechanism.

Split v4-only and v6-only rows are created independently and commonly
disagree on speed or is_rs_peer, so hard-failing every disagreement the
way the conflict resolver's gate 8 does would block most real merges.
Instead the audit table now shows, per candidate, what the keeper
absorbs and which value the DELETE discards, and a row with a
disagreement cannot be applied until the admin ticks an acknowledgement
for that specific disagreement.

Changes:
- buildMergePlan accepts family: "both", skipping ipaddr4 and ipaddr6
  together for the IX-F path, which sets both from the IX-F-confirmed
  pair and so owns neither field's merge decision.
- Add summarizeIxfMergeEffects: what a merge absorbs, what it discards,
  and a stable key identifying the disagreement set.
- applyIxfMerges takes ixfMap, re-reads both rows, re-runs
  findIxfMergeCandidates on the live pair and aborts if it no longer
  merges, aborts when the acknowledgement does not match the live
  disagreement set, absorbs the doomed row's gaps into the keeper PUT,
  and re-reads the keeper to confirm every field before the DELETE.
- Audit-log outcomes gain `absorbed` and `acknowledgedMismatches`.
- The audit modal gains a "Merge effect" column with the per-row
  acknowledgement, holds ixfMap at modal scope, blocks Apply on
  unreviewed rows, reports the counts in the confirm dialog, and
  refreshes after any aborted row.
- Test shim: fetchMap entries accept { __sequence: [...] }, serving
  responses in order and repeating the last. A read-modify-verify flow
  cannot be tested without a URL that answers differently after a write.

Security:
- An IX-F merge can no longer delete the last carrier of a value: the
  keeper must be re-read and confirmed to hold it first.
- A merge can no longer proceed on a stale premise; if the live pair
  no longer satisfies findIxfMergeCandidates, the row aborts unwritten.
- With no IX-F map the loop aborts rather than merging unconfirmed, the
  same "200% sure over 90%" stance as conflict-resolver gates 5 and 6.

Testing:
- node --test: 532 tests, 531 pass, 1 skipped (live tests are opt-in).
- New tests/cp-ixf-merge-apply.test.js drives the real write path through
  the shim, asserting on recorded request methods -- including that no
  DELETE is issued when the read-back disagrees, when the live re-check
  fails, when a mismatch is unacknowledged, and when the IX-F map is
  absent. Candidates come from the real findIxfMergeCandidates, so a
  fixture that stops being a valid merge fails loudly.
- A companion case proves an acknowledged mismatch still completes, so
  the gate is not vacuously closed.
- Two self-tests for the new shim capability in browser-shim.test.js.
- Each of the four safeguards reverted individually and confirmed red.
- DOM wiring for the new column is covered by the AGENTS.md smoke test,
  per this repo's testing split.

Backwards Compatibility:
- applyIxfMerges gains an ixfMap parameter in second position; its only
  caller is updated here. It is newly exported on the test hooks.
- buildMergePlan's existing 4 and 6 callers are unaffected.
- Audit-log entries gain fields; readers tolerate unknown keys.

Assisted-by: Claude:claude-opus-5
@netravnen
netravnen force-pushed the fix/cp-ixf-merge-gates branch from 0b52565 to ef9d440 Compare August 19, 2026 23:09
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.

1 participant