Skip to content

feat(chat): gate edit and delete on the server's message windows - #722

Merged
bmc08gt merged 2 commits into
feat/user-flags-message-windowsfrom
feat/message-window-gating
Sep 3, 2026
Merged

feat(chat): gate edit and delete on the server's message windows#722
bmc08gt merged 2 commits into
feat/user-flags-message-windowsfrom
feat/message-window-gating

Conversation

@bmc08gt

@bmc08gt bmc08gt commented Sep 3, 2026

Copy link
Copy Markdown
Collaborator

Stacked on #721 — review that first; the base is feat/user-flags-message-windows, not main.

What this does

UserFlags.messageEditWindow / messageDeleteWindow landed in #721 but nothing read them: ConversationLoadCoordinator passed MessagePolicy.default, which carried no windows, so Edit and Delete were offered on every confirmed message of your own regardless of age.

  • MessagePolicy gains deleteWindow next to editWindow, and MessageCapability.resolve gates .delete on it the same way it already gated .edit. .delete was previously seeded unconditionally.
  • MessagePolicy(userFlags:) builds the policy from the flags and applies the fallbacks. The call site passes it instead of .default.

Fallbacks

15 minutes for edit, 48 hours for delete, named as MessagePolicy.fallbackEditWindow / fallbackDeleteWindow so they are greppable and comparable against Android.

The fallback sits at policy construction, not in UserFlags.init(_:), so nil on the model keeps meaning "the server said nothing". Session.userFlags is optional and updateUserFlags() never assigns on failure, so optional-chaining collapses three cases into one expression: no flags yet, a failed fetch with no cached row, and flags whose window fields are unset. The failed-fetch case needs no separate path, and a test covers it alongside the present-but-unset case.

This inverts the reasoning the old doc comment recorded — it defaulted to nil because a client-side window would only hide an action the server would have accepted, and with a fallback in force that can now happen. The comment is rewritten to state the new rule and the trade-off: an affordance the server rejects is the worse failure, and both clients applying the same values matters more than matching server behaviour exactly.

The clock

map's comment named this as the hard part. now: message.date made elapsed time always zero, so a window could never lapse; reading Date.now inside map would make it impure and break the Inputs-equality short-circuit that lets an unrelated tick skip the remap and lets the snapshot cross to a background task.

now is carried in Inputs and advanced only by scheduleWindowExpiry, which asks MessageCapability.nextExpiry for the soonest deadline among the window's messages and sleeps until it, then advances the clock and re-maps. map stays pure and the short-circuit keeps working; the cost is that the clock is only as fresh as the last thing that advanced it, which is sound because every boundary crossing has a timer scheduled for it. The old comment is replaced with one describing this.

Details worth a look in review:

  • Nothing polls. No expiring message means no timer at all, and each firing schedules only the next deadline.
  • The timer wakes one second past the deadline. Firing exactly on it would still resolve the capability as granted (the boundary is inclusive) and then compute the same deadline again, so the row would never drop.
  • Sleeps are clamped to an hour so a 48-hour delete window doesn't park a task behind a transcript nobody is reading. The cost is at most one extra remap per hour on a transcript left open that long.
  • refresh(with:) is split out of observeInputs because the timer drives a re-map too and must not install a second observation arm.
  • nextExpiry runs eligibility through resolve rather than re-deriving it, so a message with no windowed capability to lose contributes no deadline by construction.

Parity

Same fallback values, same treatment of unset windows, and <= at the boundary so a message at exactly the window length is still actionable — matching Android's MessageCapability.kt. No deliberate deviations.

Out of scope

No change to the server-side authority model or to CANNOT_EDIT / CANNOT_DELETE handling. Client-side affordance gating only.

Tests

MessageCapabilityTests covers both capabilities inside their windows, past them, at exactly the boundary and a hair past it, independent gating, nil windows, the fallback from absent and from present-but-unset flags, server-sent windows overriding the fallbacks, and the nextExpiry deadline selection.

The transcript offered Edit and Delete on every confirmed message of your own.
`UserFlags` has carried `messageEditWindow` and `messageDeleteWindow` since the
parent branch, but nothing read them: `ConversationLoadCoordinator` passed
`MessagePolicy.default`, which set no windows at all.

`MessagePolicy` now takes a `deleteWindow` alongside `editWindow` and is built
from the flags at the call site. Where the server sends nothing, it falls back to
15 minutes for edit and 48 hours for delete, as `fallbackEditWindow` and
`fallbackDeleteWindow`. `Session.userFlags` is optional and a failed fetch never
assigns, so optional-chaining collapses absent flags, a cached row, and an unset
field into that one fallback — there is no separate failure path.

That inverts the old rule, and the doc comment says so: the client can now hide
an edit the server would have accepted. An affordance the server rejects is the
worse failure, and Android applies the same two values, so both clients offer
the same rows.

Resolving against a real clock is the part `map` was written to avoid. Its
comment recorded the constraint — `now: message.date` made elapsed time zero, so
a window could never lapse, and reading `Date.now` inside `map` would cost the
`Inputs` equality short-circuit that keeps an unrelated tick free. `now` is now
carried in `Inputs` and advanced only by `scheduleWindowExpiry`, which asks
`MessageCapability.nextExpiry` for the soonest deadline in the window and sleeps
until it. Nothing polls: with no expiring message there is no timer, and each
firing schedules only the next one. Waking a second past the deadline avoids
re-granting at the inclusive boundary; a one-hour clamp keeps a 48-hour delete
window from parking a task behind a transcript nobody is reading.

The boundary is `<=`, matching Android — a message at exactly the window length
is still actionable.
@bmc08gt bmc08gt self-assigned this Sep 3, 2026
Comment thread FlipcashCore/Sources/FlipcashCore/Models/Conversation/MessageCapability.swift Outdated
`isWithin` grants a capability at exactly the window's length, but `nextExpiry`
dropped a deadline equal to `now`. At that instant the row still offers Edit or
Delete with no timer armed to take it away, so it stays offered until an
unrelated re-map runs.

`now` is an injected parameter in both functions, so this is reachable from a
test rather than only from `Date`'s resolution — the new case pins it.

`>=` cannot re-arm on itself: `ConversationLoadCoordinator` wakes at
`deadline + expiryGrace`, a second past the instant it just scheduled for.
@bmc08gt
bmc08gt merged commit a28dc4e into feat/user-flags-message-windows Sep 3, 2026
bmc08gt added a commit that referenced this pull request Sep 3, 2026
* feat(account): carry message edit/delete windows on UserFlags

UserFlags gained two message-typed Duration fields (17, 18) upstream:
message_edit_window and message_delete_window. Map them onto UserFlags
as optional TimeIntervals, gated on hasMessageEditWindow /
hasMessageDeleteWindow so an absent window is not read as zero,
matching the existing billExchangeDataTimeout convention.

Scaffolding only, nothing reads these yet. A follow-up wires them into
MessagePolicy for the chat edit/delete affordances.

* build: pin flipcash2-client-protocol 0.4.0

Picks up messageEditWindow and messageDeleteWindow on UserFlags. Blocked until
0.4.0 is published; ocp-client-protocol is unaffected and stays at 0.2.0.

* build: resolve flipcash2-client-protocol 0.4.0 in the workspace lockfile

b5432cc moved the pin in FlipcashAPI/Package.swift to 0.4.0 but left the
workspace Package.resolved resolving 0.2.0, so the manifest and the lockfile
disagreed and the next build to touch the workspace rewrote it. CLAUDE.md
requires the workspace Package.resolved be committed.

The revision matches the 0.4.0 tag (27e3f09a). ocp-client-protocol is untouched
and stays at 0.2.0, which its pin and lockfile entry already agree on.

* build: bump ocp-client-protocol to 0.3.0

Keeps iOS on the same package version as Android. 0.3.0 over 0.2.0 is
Android-only content — R8 keep rules for the generated messages, plus CHANGELOG
and README. No .proto and no Swift changed, so this carries no contract change
and nothing in FlipcashAPI moves.

Pin and workspace lockfile updated together; the revision matches the 0.3.0 tag
(7c37ecc0). Verified with Scripts/build.sh, which resolved 0.3.0 and left the
lockfile entry as written.

* feat(chat): gate edit and delete on the server's message windows (#722)

* feat(chat): gate edit and delete on the server's message windows

The transcript offered Edit and Delete on every confirmed message of your own.
`UserFlags` has carried `messageEditWindow` and `messageDeleteWindow` since the
parent branch, but nothing read them: `ConversationLoadCoordinator` passed
`MessagePolicy.default`, which set no windows at all.

`MessagePolicy` now takes a `deleteWindow` alongside `editWindow` and is built
from the flags at the call site. Where the server sends nothing, it falls back to
15 minutes for edit and 48 hours for delete, as `fallbackEditWindow` and
`fallbackDeleteWindow`. `Session.userFlags` is optional and a failed fetch never
assigns, so optional-chaining collapses absent flags, a cached row, and an unset
field into that one fallback — there is no separate failure path.

That inverts the old rule, and the doc comment says so: the client can now hide
an edit the server would have accepted. An affordance the server rejects is the
worse failure, and Android applies the same two values, so both clients offer
the same rows.

Resolving against a real clock is the part `map` was written to avoid. Its
comment recorded the constraint — `now: message.date` made elapsed time zero, so
a window could never lapse, and reading `Date.now` inside `map` would cost the
`Inputs` equality short-circuit that keeps an unrelated tick free. `now` is now
carried in `Inputs` and advanced only by `scheduleWindowExpiry`, which asks
`MessageCapability.nextExpiry` for the soonest deadline in the window and sleeps
until it. Nothing polls: with no expiring message there is no timer, and each
firing schedules only the next one. Waking a second past the deadline avoids
re-granting at the inclusive boundary; a one-hour clamp keeps a 48-hour delete
window from parking a task behind a transcript nobody is reading.

The boundary is `<=`, matching Android — a message at exactly the window length
is still actionable.

* fix(chat): keep the deadline that lands exactly on now

`isWithin` grants a capability at exactly the window's length, but `nextExpiry`
dropped a deadline equal to `now`. At that instant the row still offers Edit or
Delete with no timer armed to take it away, so it stays offered until an
unrelated re-map runs.

`now` is an injected parameter in both functions, so this is reachable from a
test rather than only from `Date`'s resolution — the new case pins it.

`>=` cannot re-arm on itself: `ConversationLoadCoordinator` wakes at
`deadline + expiryGrace`, a second past the instant it just scheduled for.
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