[Bug] A failed native merge wipes every file in the deck folder except deck.md

brenno commented

2026-07-22 18:42:17 +00:00

Owner

What happens

In tabs_provider_git_native.dart, the resolve callback of mirror.mergeRemote falls back to keeping our own side when it cannot merge:

// Komen we er niet uit, dan blijft ónze kant staan zoals hij was: een
// lege set zou de deckmap wissen, en dat is precies wat nooit mag.
final fallback = <String, Uint8List>{deckFile: ?ourBytes};

The comment is right about the danger and wrong about the remedy. native_git_mirror_io.dart does deckAbs.delete(recursive: true) and then writes exactly the files it was handed, so a fallback of {deck.md: ourBytes} does not 'keep our side as it was' — it keeps one file and removes the rest of the deck folder.

What is lost

data/*.json and data/*.csv: the linked chart data. The deck reopens with the reference intact and the numbers gone.
Since #541, deck.user-notes.json.
Anything else that lands in a deck folder later, automatically, without anyone noticing.

How to reach it

The fallback fires whenever any of base / ours / theirs fails to parse or is refused by the import gate (gated() returns null). A deck that trips the safety scanner therefore costs you your chart data on top of the refusal — two unrelated penalties for one event.

Why this is filed separately

It predates the notes work: the chart-data half has been reachable since linked chart data landed. #541 widens the blast radius but did not create it, and fixing it properly means changing what mergeRemote/_writeAll promise about files the resolver did not mention — a contract question, not a one-line patch.

Suggested direction

Either have the resolver receive (and be able to pass back) the other files in the deck folder, or make _writeAll additive with an explicit delete list instead of clearing the folder first. The second matches how the REST plane already works (commitFiles(upserts:, deletes:)), which is an argument for making the two planes agree rather than inventing a third rule.

Regression guard

A native-mirror test that puts a chart data file and a notes file in the deck folder, forces the fallback (unparseable theirBytes), and asserts both survive.

**What happens** In `tabs_provider_git_native.dart`, the `resolve` callback of `mirror.mergeRemote` falls back to keeping our own side when it cannot merge: ```dart // Komen we er niet uit, dan blijft ónze kant staan zoals hij was: een // lege set zou de deckmap wissen, en dat is precies wat nooit mag. final fallback = <String, Uint8List>{deckFile: ?ourBytes}; ``` The comment is right about the danger and wrong about the remedy. `native_git_mirror_io.dart` does `deckAbs.delete(recursive: true)` and then writes exactly the files it was handed, so a fallback of `{deck.md: ourBytes}` does not 'keep our side as it was' — it keeps *one file* and removes the rest of the deck folder. **What is lost** - `data/*.json` and `data/*.csv`: the linked chart data. The deck reopens with the reference intact and the numbers gone. - Since #541, `deck.user-notes.json`. - Anything else that lands in a deck folder later, automatically, without anyone noticing. **How to reach it** The fallback fires whenever any of base / ours / theirs fails to parse or is refused by the import gate (`gated()` returns null). A deck that trips the safety scanner therefore costs you your chart data on top of the refusal — two unrelated penalties for one event. **Why this is filed separately** It predates the notes work: the chart-data half has been reachable since linked chart data landed. #541 widens the blast radius but did not create it, and fixing it properly means changing what `mergeRemote`/`_writeAll` promise about files the resolver did not mention — a contract question, not a one-line patch. **Suggested direction** Either have the resolver receive (and be able to pass back) the other files in the deck folder, or make `_writeAll` additive with an explicit delete list instead of clearing the folder first. The second matches how the REST plane already works (`commitFiles(upserts:, deletes:)`), which is an argument for making the two planes agree rather than inventing a third rule. **Regression guard** A native-mirror test that puts a chart data file and a notes file in the deck folder, forces the fallback (unparseable `theirBytes`), and asserts both survive.

brenno added the

triage

bug

labels

2026-07-22 18:42:17 +00:00

brenno commented

2026-07-22 19:11:53 +00:00

Author

Owner

Aanvulling na een tweede lezing — de titel is te smal en het risico groter dan hierboven staat.

Het treft niet alleen de faaltak. Ook een geslaagde native merge verwijdert <deckDir>/data/*.json. De resolver bouwt merge.merged uit de drie deck.md's en hydrateert alleen de notities, dus chartDataFilesOf levert niets, dus die bestanden staan niet in wat er teruggegeven wordt — en deckAbs.delete(recursive: true) ruimt ze op. Elke merge tussen twee auteurs kost dus de cijfers van elke gekoppelde grafiek, zonder melding.

En het is sinds vandaag goedkoper te repareren dan hierboven staat. withRepoChartData neemt dezelfde RepoFileReader als withRepoUserNotes, en resolveRepoDeckMerge (PR voor #541) heeft nu per kant een lezer. De hydratatie van de grafiekdata is daarmee vrijwel dezelfde regel als die van de notities; wat nog een plek moet krijgen is waar de melding over ontbrekende bronnen heen gaat op dit pad.

Dat neemt de contractvraag over mergeRemote/_writeAll niet weg — bestanden die de resolver niet noemt, verdwijnen, en dat blijft de eigenlijke fout. Maar het maakt de tussenstap klein genoeg om niet op die contractwijziging te hoeven wachten.

De regressietest moet dus twee gevallen dekken, niet één: de gelukte merge én de fallback.

Aanvulling na een tweede lezing — de titel is te smal en het risico groter dan hierboven staat. **Het treft niet alleen de faaltak.** Ook een *geslaagde* native merge verwijdert `<deckDir>/data/*.json`. De resolver bouwt `merge.merged` uit de drie `deck.md`'s en hydrateert alleen de notities, dus `chartDataFilesOf` levert niets, dus die bestanden staan niet in wat er teruggegeven wordt — en `deckAbs.delete(recursive: true)` ruimt ze op. Elke merge tussen twee auteurs kost dus de cijfers van elke gekoppelde grafiek, zonder melding. **En het is sinds vandaag goedkoper te repareren dan hierboven staat.** `withRepoChartData` neemt dezelfde `RepoFileReader` als `withRepoUserNotes`, en `resolveRepoDeckMerge` (PR voor #541) heeft nu per kant een lezer. De hydratatie van de grafiekdata is daarmee vrijwel dezelfde regel als die van de notities; wat nog een plek moet krijgen is waar de melding over ontbrekende bronnen heen gaat op dit pad. Dat neemt de contractvraag over `mergeRemote`/`_writeAll` niet weg — bestanden die de resolver niet noemt, verdwijnen, en dat blijft de eigenlijke fout. Maar het maakt de tussenstap klein genoeg om niet op die contractwijziging te hoeven wachten. De regressietest moet dus twee gevallen dekken, niet één: de gelukte merge én de fallback.

brenno referenced this issue

2026-07-22 19:23:31 +00:00

feat(git): laat de notities meereizen naar een repo, en houd ze heel (#541) #673

brenno referenced this issue

2026-07-22 19:24:14 +00:00

[Feature] Sidecars travel to git too: notes, ink, seal #541

brenno added

in-progress

and removed

triage

labels

2026-07-22 19:32:23 +00:00

brenno commented

2026-07-22 19:32:24 +00:00

Author

Owner

Opgepakt. Tak: fix/native-merge-deckmap-670. Reikwijdte: lib/services/git/deck_repo_serializer.dart, lib/services/git/native_git_mirror_io.dart, lib/services/git/native_git_mirror_api.dart, lib/state/tabs_provider_git_native.dart, plus een regressietest voor beide gevallen (gelukte merge én fallback).

Opgepakt. Tak: `fix/native-merge-deckmap-670`. Reikwijdte: `lib/services/git/deck_repo_serializer.dart`, `lib/services/git/native_git_mirror_io.dart`, `lib/services/git/native_git_mirror_api.dart`, `lib/state/tabs_provider_git_native.dart`, plus een regressietest voor beide gevallen (gelukte merge én fallback).

brenno referenced this issue from a commit

2026-07-22 19:50:38 +00:00

fix(git): laat de deckmap heel bij openen, opslaan en samenvoegen (#670)

brenno referenced this issue from a commit

2026-07-22 19:50:38 +00:00

docs(git): schrijf op dat de deckmap wordt bijgewerkt, niet vervangen (#670)

brenno referenced this issue from a commit

2026-07-22 19:50:38 +00:00

refactor(git): één plek voor de bytes van een afbeelding, één voor de melding

brenno referenced this issue

2026-07-22 19:51:09 +00:00

fix(git): laat de deckmap heel bij openen, opslaan en samenvoegen (#670) #676

brenno referenced this issue from a commit

2026-07-22 19:51:17 +00:00

Merge pull request 'fix(git): laat de deckmap heel bij openen, opslaan en samenvoegen (#670)' (#676) from fix/native-merge-deckmap-670 into main

brenno removed the

in-progress

label

2026-07-22 19:51:36 +00:00

brenno commented

2026-07-22 19:51:37 +00:00

Author

Owner

Opgelost in 48349fa7 + 93fa64b6 (PR #676), op main sinds b6f95825.

De oorzaak lag een stap eerder dan we allebei dachten. Het native openpad riep withRepoSidecars helemaal niet aan — dat deed alleen het REST-pad. Er stond dus een deck in de editor dat zijn eigen lagen niet kende, en omdat elke schrijfweg de deckmap vervangt, ruimde de eerstvolgende gewone opslag ze al op. Geen merge nodig, geen botsing, geen melding. Jouw aanvulling zag terecht dat ook een geslaagde merge de cijfers kostte; het was nog een slag breder dan dat.

Drie plekken gerepareerd: openen hydrateert nu net als het REST-pad, samenvoegen doet dat per kant, en de terugvalweg houdt onze hele kant vast in plaats van alleen deck.md — git's tekst-merge is daar al over de map gegaan, dus die sidecars kunnen conflictmarkeringen dragen.

De contractvraag ging mee, langs jouw tweede voorstel. mergeRemote beloofde dat wat de resolver niet noemt weg mag; die volledigheid kón hij niet waarmaken. Nu bijwerken: files schrijven, deletes verwijderen, de rest laten staan — hetzelfde als commitFiles(upserts:, deletes:) op het REST-vlak. commitDeck blijft wél vervangen, want daar kent de app de hele set écht; die asymmetrie staat uitgeschreven in GIT_STORAGE §9.7.

Regressietest: de twee gevallen die je vroeg, plus de gewone opslag zonder merge. Eén opmerking daarbij, want hij is leerzaam: mijn eerste versie van de terugval-test toetste tegen de origin en stond daardoor onterecht groen — bij clean: false wordt er niets gepusht, dus de origin bewijst daar niets. Hij toetst nu de clone, en dat is ook de plek die ertoe doet: dát is de werkkopie waar de editor uit leest.

Wat er níét in zat: de ink-sidecar en het zegel reizen nog steeds niet mee naar git (§9.1); daar verandert dit niets aan. En een grafiekbestand van een dia die de merge weghaalde blijft nu als wees achter in plaats van opgeruimd te worden — een achtergebleven bestand in ruil voor geen dataverlies, dezelfde afweging als op het REST-vlak.

Opgelost in `48349fa7` + `93fa64b6` (PR #676), op main sinds `b6f95825`. **De oorzaak lag een stap eerder dan we allebei dachten.** Het native *openpad* riep `withRepoSidecars` helemaal niet aan — dat deed alleen het REST-pad. Er stond dus een deck in de editor dat zijn eigen lagen niet kende, en omdat elke schrijfweg de deckmap vervangt, ruimde de **eerstvolgende gewone opslag** ze al op. Geen merge nodig, geen botsing, geen melding. Jouw aanvulling zag terecht dat ook een geslaagde merge de cijfers kostte; het was nog een slag breder dan dat. Drie plekken gerepareerd: openen hydrateert nu net als het REST-pad, samenvoegen doet dat per kant, en de terugvalweg houdt onze hele kant vast in plaats van alleen `deck.md` — git's tekst-merge is daar al over de map gegaan, dus die sidecars kunnen conflictmarkeringen dragen. **De contractvraag ging mee**, langs jouw tweede voorstel. `mergeRemote` beloofde dat wat de resolver niet noemt weg mag; die volledigheid kón hij niet waarmaken. Nu bijwerken: `files` schrijven, `deletes` verwijderen, de rest laten staan — hetzelfde als `commitFiles(upserts:, deletes:)` op het REST-vlak. `commitDeck` blijft wél vervangen, want daar kent de app de hele set écht; die asymmetrie staat uitgeschreven in GIT_STORAGE §9.7. **Regressietest**: de twee gevallen die je vroeg, plus de gewone opslag zonder merge. Eén opmerking daarbij, want hij is leerzaam: mijn eerste versie van de terugval-test toetste tegen de origin en stond daardoor onterecht groen — bij `clean: false` wordt er niets gepusht, dus de origin bewijst daar niets. Hij toetst nu de clone, en dat is ook de plek die ertoe doet: dát is de werkkopie waar de editor uit leest. **Wat er níét in zat:** de ink-sidecar en het zegel reizen nog steeds niet mee naar git (§9.1); daar verandert dit niets aan. En een grafiekbestand van een dia die de merge weghaalde blijft nu als wees achter in plaats van opgeruimd te worden — een achtergebleven bestand in ruil voor geen dataverlies, dezelfde afweging als op het REST-vlak.

brenno closed this issue

2026-07-22 19:51:42 +00:00

brenno referenced this issue

2026-07-22 20:05:48 +00:00

refactor(git): guard-basis en merge-na-conflict uit de state-laag, met de scanners eindelijk gedraaid (#518) #679

brenno referenced this issue

2026-07-22 20:06:23 +00:00

[Feature] Structural debt: layering, class size, SOURCE_MAP #518

Rows
Columns

[Bug] A failed native merge wipes every file in the deck folder except deck.md #670