Repository navigation
fix(engine): stop treating a bumped version token as a dropped write - #1182
Conversation
A compare-and-set write sends the version it read and expects the server to bump it. write-field-ignored had no way to tell that token apart from an ordinary persisted field, so the response's incremented version read as a dropped write on an otherwise healthy save -- reported on four separate apps, POST /trpc/reports.submit among them (reticlehq#984). version, _version, etag, rev and revision are now excluded from the per-key comparison the same way identity keys already are: the changed value is the proof the write applied, not a field the caller asked the server to preserve. Closes reticlehq#984 Signed-off-by: drakeo338 <[email protected]>
|
Thanks for your first pull request to Reticle! A maintainer aims to review within two days. Before then, |
 |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| // for crying wolf. | ||
| if (wanted.size !== 1) continue; | ||
| if (isIdentityKey(key)) continue; | ||
| if (isVersionKey(key)) continue; |
There was a problem hiding this comment.
If an app writes an ordinary field named version, such as a software release, and a successful response echoes the old value, this check skips the field solely because of its name. The same happens for the other four excluded names, even in nested objects. A genuinely ignored write can therefore produce no WRITE_FIELD_IGNORED finding.
| * incremented `version: 2` was read as a dropped write — because nothing here distinguishes an | ||
| * optimistic-concurrency token from an ordinary persisted field. Reported on four separate apps. | ||
| */ | ||
| import { describe, expect, it } from 'vitest'; |
There was a problem hiding this comment.
The new comment says the behavior was reported on “four separate apps.” That is a user-derived field count, which the repository's no-field-numbers directive prohibits in shipped code comments. Remove the count before merging; the technical description can stay.
Context Used: CLAUDE.md (source)
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
What & why
Refs #984 (umbrella): fixes one of the five reported shapes,
write-field-ignoredfiring on a compare-and-set write.A CAS write sends the
versionit read and expects the server to bump it.write-field-ignoredhad no way to tell that token from an ordinary persisted field, so a bumpedversionread as a dropped write on an otherwise healthy save —POST /trpc/reports.submitamong the four apps reporting it.version,_version,etag,revandrevisionare now excluded from the comparison the same way identity keys already are: the changed value proves the write applied, not a field the caller asked the server to keep.How it was verified
RED: a test reproducing the report (
version: 1sent,version: 2echoed) failed againstmain. GREEN: it passes with the fix; a second test confirms a real dropped field next to the same bump is still caught. Baseline 0/2, candidate 2/2.Gates run
pnpm lint && pnpm typecheck && pnpm test:unit(~2 min — always)pnpm test:e2e(~8 min) — touched the tool surface,core, an observer, or telemetrypnpm gate:install(~15 min) — touchedreticle init,vite-plugin,next, orbabel-pluginpnpm test:e2e:desktop(~3 min) — touchedadapters/realm/electron,adapters/realm/tauri, or desktop captureChecklist
git commit -s) — CI's DCO check fails the PR without it. Already pushed?git rebase --signoff origin/main && git push --force-with-leaseany, no free strings (wire strings live in@reticlehq/core), no non-null!console.logor internal tracking codes left in the diff.changes/(never editCHANGELOG.md— that file is assembled at release time, and editing it is what makes PRs conflict; format in.changes/README.md)docs/telemetry.md) and are covered by a test