Skip to content

fix(connections): make the tag library report its refusals and survive a broken store - #2622

Merged
datlechin merged 1 commit into
mainfrom
fix/tag-storage-refusals
Sep 3, 2026
Merged

fix(connections): make the tag library report its refusals and survive a broken store#2622
datlechin merged 1 commit into
mainfrom
fix/tag-storage-refusals

Conversation

@datlechin

Copy link
Copy Markdown
Member

TagStorage is GroupStorage's sibling and carried the same four defects, found while fixing
those in #2619. The store is now the same shape as the group store, so the two cannot drift apart
again on the next reader.

What was wrong

One unreadable entry replaced the whole library with the presets. loadTags caught the decode
error, returned ConnectionTag.presets and cached them. Every mutation rewrites the whole array,
so the next tag operation persisted the presets over the user's own tags. Worse than losing them
to an empty list: the store then looks healthy, because a fresh install looks exactly like that.

A refused tag was discarded without a word. addTag returned early on a name the library
already held. The create sheet dismissed anyway and selected the tag that was already there, with
its colour rather than the one just picked, so the user got a different tag from the one they
described and nothing said so.

Nothing announced a tag change. No AppEvents anywhere in the file, so a tag created in one
window did not reach another until relaunch.

Two Macs traded the whole library. saveTags marks every tag dirty and the push uploads every
dirty tag, and the remote apply wrote a record without asking whether it differed from the one
already stored. One changed tag re-uploaded all of them, the other Mac wrote them and re-uploaded
them back.

Alongside those, the import loop read existingTags once for the whole envelope, so two tags
sharing a name in one file both passed the check taken before either was added.

What changed

  • loadTags decodes element by element, so one unreadable entry costs that entry. A payload it
    cannot read at all leaves the store untouched and refuses the write instead of overwriting it.
  • addTag throws TagStorageError. The create sheet keeps itself open and says the name is taken.
  • saveTags returns Bool, matching ConnectionStorage's documented convention.
  • addTag and deleteTag announce connectionUpdated after a successful write. saveTags stays
    silent so the sync pull keeps its single coalesced event.
  • applyRemoteTag on the store, skipping a record identical to the one already held, replacing the
    copy of that logic in SyncCoordinator.
  • The store takes its defaults, sync tracker and event bus by injection, like GroupStorage, which
    is what makes any of this testable.
  • The import loop re-reads per tag.

Verification

  • verify.sh generate, verify.sh build: PASS
  • verify.sh test TagStorageTests GroupStorageTests StringCatalogIntegrityTests: PASS, 37 cases
  • swiftlint --strict on every changed file: clean
  • scripts/localization.py verify: both catalogs still round-trip byte for byte

TagStorageTests is new: each thrown error, an event per mutation and none from a remote apply, an
unchanged remote record leaving nothing dirty, a preset refusing deletion, an unreadable store left
untouched, and an unreadable entry not taking the library down.

One thing the tests taught me about the store: init seeds the presets through loadTags, so an
instance built before a test writes its payload has a warm cache and never reads it. The two
recovery tests write the store first and build the instance after, which is also what actually
happens: the app starts, and the file is already there.

https://claude.ai/code/session_01L81TaoPWxkLd15CGw2riPq

@chatgpt-codex-connector

Copy link
Copy Markdown

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.

@datlechin
datlechin merged commit b6a882a into main Sep 3, 2026
8 checks passed
@datlechin
datlechin deleted the fix/tag-storage-refusals branch September 3, 2026 08:05
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