Never overwrite filters we could not read #78

New Issue

Closed

opened 2026-08-26 22:04:21 +00:00 by jcoffey-dev · 0 comments

jcoffey-dev commented

2026-08-26 22:04:21 +00:00

Owner

Closes #76. Adding a filter from a message reported success while the script on the server never held more than two rules. Rules were being destroyed, and the confirmation was a lie — which is why this jumped the queue.

The chain

Three links, each defensible on its own:

load()  catch → contents[id] = ""     a failed blob fetch looks like an empty script
sieveToRules("") → []                  "no rules", indistinguishable from "unreadable"
saveRules([] + new) → whole script     every existing rule deleted; the write succeeded,
                                       so the UI reported success

No fetch failure was even required. rules() did contents[id] ?? "", so a script whose content simply hadn't loaded yet read as empty. That explains bitbln's "sometimes the last rule shows, sometimes earlier ones but not the last, never more than two" — it's a race with the content load, and the count oscillates around one or two.

And saveScript cached the content it had just written, then called load(), which replaced the whole map — discarding it whenever the refetch came back short.

The fix: keep "unknown" and "empty" apart

a failed fetch leaves the key absent rather than storing ""
load() merges rather than replacing, so a reload can't throw away what saveScript just wrote
rules() returns null for content it doesn't have — which every caller already treats as "don't touch this script"
saveRules refuses outright when the baseline is unknown

That last one is the real safety net. Refusing is recoverable; overwriting isn't.

rules() now also reports whether the script was actually read, because "written by hand" and "couldn't be read" deserve different advice — one is permanent, the other is a reload away. Telling someone the wrong one sends them hunting for a problem they don't have.

Ruled out on the way

The rule codec round-trips cleanly — eight rules in, eight out. sieveToRules reads the # rule: JSON comments rather than parsing Sieve, so the generated script's shape was never the issue. Worth recording, since it was the obvious first suspect.

Tests

Seven new, pinning the distinction rather than the symptom: an unreadable script reports its rules as unknown and refuses to save; a genuinely empty one is distinguishable from it; a script that was read hands back every rule; and a reload doesn't discard content already held.

272 web + 77 server tests, typecheck and build clean.

Merged 2026-08-26 as coffey-labs/ihasmail@c647744470

_{Rebuilt from: git history, session transcript.}

Closes #76. Adding a filter from a message reported success while the script on the server never held more than two rules. **Rules were being destroyed, and the confirmation was a lie** — which is why this jumped the queue. ## The chain Three links, each defensible on its own: ``` load() catch → contents[id] = "" a failed blob fetch looks like an empty script sieveToRules("") → [] "no rules", indistinguishable from "unreadable" saveRules([] + new) → whole script every existing rule deleted; the write succeeded, so the UI reported success ``` **No fetch failure was even required.** `rules()` did `contents[id] ?? ""`, so a script whose content simply hadn't loaded yet read as empty. That explains bitbln's "sometimes the last rule shows, sometimes earlier ones but not the last, never more than two" — it's a race with the content load, and the count oscillates around one or two. And `saveScript` cached the content it had just written, then called `load()`, which **replaced** the whole map — discarding it whenever the refetch came back short. ## The fix: keep "unknown" and "empty" apart - a failed fetch leaves the key **absent** rather than storing `""` - `load()` **merges** rather than replacing, so a reload can't throw away what `saveScript` just wrote - `rules()` returns `null` for content it doesn't have — which every caller already treats as "don't touch this script" - `saveRules` **refuses outright** when the baseline is unknown That last one is the real safety net. Refusing is recoverable; overwriting isn't. `rules()` now also reports whether the script was actually read, because *"written by hand"* and *"couldn't be read"* deserve different advice — one is permanent, the other is a reload away. Telling someone the wrong one sends them hunting for a problem they don't have. ## Ruled out on the way The rule codec round-trips cleanly — eight rules in, eight out. `sieveToRules` reads the `# rule:` JSON comments rather than parsing Sieve, so the generated script's shape was never the issue. Worth recording, since it was the obvious first suspect. ## Tests Seven new, pinning the distinction rather than the symptom: an unreadable script reports its rules as unknown and refuses to save; a genuinely empty one is distinguishable from it; a script that was read hands back every rule; and a reload doesn't discard content already held. 272 web + 77 server tests, typecheck and build clean. **Merged** 2026-08-26 as coffey-labs/ihasmail@c647744470e9 <sub>Rebuilt from: git history, session transcript.</sub>

This repo is archived. You cannot comment on issues.

1 Participants

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: coffey-labs/ihasmail-github-archive#78