fix: async-safe repository construction, backup policy, moderation hydration + cancellation (P1-5, P1-6, P2-1, P2-2)

starsetbyte commented

2026-07-13 00:58:48 +00:00

Owner

Fixes P1-5, P1-6, P2-1, P2-2 (moderation half) from the 2026-07-11 adversarial delta review.

P1-5 — repository construction no longer blocks on Room/network

Removed runBlocking/I/O from ModerationActionsRepository.init; local state derives from Room flows and expired-mute cleanup runs in an explicit suspend initializer.
Deviation (user decision after task review): the async-construction change opened a seed-vs-mutation race on the non-atomic _mutedDids/_blockedDids sets, so every mutation (muteAccount, unmuteAccount, blockAccount, unblockAccount, pruneExpiredMutes) was converted to update { } for atomicity.

P1-6 — Android backup disabled by default

android:allowBackup="false" in AndroidManifest.xml, with an explanatory comment, since restore isn't a tested feature yet.

P2-1 — moderation hydration reconciles stale Room rows

hydrateFromServer now deletes Room rows absent from a completed server snapshot instead of only upserting. Reconciliation is delete-after-complete-snapshot (not a Room transaction, since DAOs are interface-mocked in the JVM suite) — the property that matters, a partial snapshot never deletes, holds.

P2-2 (moderation half) — cancellation-safe mutation + hydration serialization

All ModerationActionsRepository mutation methods (plus hidePost/pruneExpiredMutes/hydrateFromServer) converted to runCatchingCancellable/attemptWithRevert so cancellation is rethrown, not swallowed.
Deviation (user decision): a self-flagged race from P2-1 review — hydrateFromServer's reconcile could delete a Room row for an account muted/blocked concurrently, since the server snapshot predates the mutation — was deferred into this task rather than fixed in isolation. A hydrationMutex now serializes hydrateFromServer's full body against muteAccount/unmuteAccount/blockAccount/unblockAccount (deliberately excluding muteThread/unmuteThread, an unseeded field). Deadlock-freedom and race closure independently re-verified by an Opus-tier reviewer.

Accepted, disclosed residuals (not blocking): pruneExpiredMutes remains unlocked against the mutex (lower-severity, self-healing); unmuteAccount's revert can still clobber richer Room metadata from a concurrent mute (pre-existing, not introduced here).

Gate: ./gradlew testDebugUnitTest lintDebug — 0 failures, 0 lint errors. (2 pre-existing skips in AtProtoOAuthClientTest remain on this branch, same as branch B — resolves once branch A merges first.)

Merge order: this is branch C of four (A → B → C → D — DM Phase A work should not proceed until A and C are merged).

Fixes P1-5, P1-6, P2-1, P2-2 (moderation half) from the [2026-07-11 adversarial delta review](https://durandal.exe.xyz/starsetbyte/peregrine/src/branch/main/docs/reviews/2026-07-11-adversarial-delta-review.md). ## P1-5 — repository construction no longer blocks on Room/network - Removed `runBlocking`/I/O from `ModerationActionsRepository.init`; local state derives from Room flows and expired-mute cleanup runs in an explicit suspend initializer. - **Deviation (user decision after task review):** the async-construction change opened a seed-vs-mutation race on the non-atomic `_mutedDids`/`_blockedDids` sets, so every mutation (`muteAccount`, `unmuteAccount`, `blockAccount`, `unblockAccount`, `pruneExpiredMutes`) was converted to `update { }` for atomicity. ## P1-6 — Android backup disabled by default - `android:allowBackup="false"` in `AndroidManifest.xml`, with an explanatory comment, since restore isn't a tested feature yet. ## P2-1 — moderation hydration reconciles stale Room rows - `hydrateFromServer` now deletes Room rows absent from a completed server snapshot instead of only upserting. Reconciliation is delete-after-complete-snapshot (not a Room transaction, since DAOs are interface-mocked in the JVM suite) — the property that matters, a partial snapshot never deletes, holds. ## P2-2 (moderation half) — cancellation-safe mutation + hydration serialization - All `ModerationActionsRepository` mutation methods (plus `hidePost`/`pruneExpiredMutes`/`hydrateFromServer`) converted to `runCatchingCancellable`/`attemptWithRevert` so cancellation is rethrown, not swallowed. - **Deviation (user decision):** a self-flagged race from P2-1 review — `hydrateFromServer`'s reconcile could delete a Room row for an account muted/blocked concurrently, since the server snapshot predates the mutation — was deferred into this task rather than fixed in isolation. A `hydrationMutex` now serializes `hydrateFromServer`'s full body against `muteAccount`/`unmuteAccount`/`blockAccount`/`unblockAccount` (deliberately excluding `muteThread`/`unmuteThread`, an unseeded field). Deadlock-freedom and race closure independently re-verified by an Opus-tier reviewer. Accepted, disclosed residuals (not blocking): `pruneExpiredMutes` remains unlocked against the mutex (lower-severity, self-healing); `unmuteAccount`'s revert can still clobber richer Room metadata from a concurrent mute (pre-existing, not introduced here). **Gate:** `./gradlew testDebugUnitTest lintDebug` — 0 failures, 0 lint errors. (2 pre-existing skips in `AtProtoOAuthClientTest` remain on this branch, same as branch B — resolves once branch A merges first.) **Merge order:** this is branch **C** of four (A → B → C → D — DM Phase A work should not proceed until A and C are merged).

starsetbyte added 5 commits

2026-07-13 00:58:48 +00:00

fix: seed moderation state off-thread, no runBlocking in construction (P1-5) 3afb576ad6

fix: make moderation mute/block state mutations atomic (Task 7 review fix) 366ef8991a

Task 7 made ModerationActionsRepository's Room seed async (externalScope.launch
instead of runBlocking in init), which removed the implicit guarantee that the
seed always completed before any mutation method could run. The seed's own
update{} on _mutedDids/_blockedDids is atomic, but the existing mutation
methods (muteAccount, unmuteAccount, blockAccount, unblockAccount,
pruneExpiredMutes) used non-atomic read-then-write (_x.value = _x.value <op>).
A concurrent seed write racing with one of these could silently drop an
update.

Converts every _mutedDids/_blockedDids mutation to atomic
StateFlow.update { it <op> x }. For unmuteAccount/unblockAccount's failure
revert paths, this also drops the "restore captured snapshot" pattern in
favor of atomically re-adding the DID that was optimistically removed — same
intent, but no longer able to clobber a concurrent writer.

muteThread/unmuteThread (_mutedThreadUris) and hydrateFromServer's full-set
overwrites (_mutedDids.value = mutes / _blockedDids.value = blocks) are
out of scope: the former isn't seeded at construction so isn't part of this
race, and the latter is an authoritative replace rather than a
read-derived write.

fix: hydration removes Room mutes/blocks absent from a complete server snapshot (P2-1) 914a87b230

fix: moderation mutations revert under cancellation and rethrow it, serialize against hydration (P2-2) 28830d1738

Part A (P2-2): runCatching in ModerationActionsRepository converted a cancelled
coroutine's CancellationException into a Result.failure instead of propagating
it, violating structured concurrency. Adds runCatchingCancellable (rethrows
CancellationException, wraps other exceptions) and attemptWithRevert (runs a
revert closure inside NonCancellable on any failure including cancellation,
then rethrows cancellation or returns the failure). muteAccount, unmuteAccount,
muteThread, unmuteThread, blockAccount, and unblockAccount now use
attemptWithRevert with their existing revert logic as the revert closure;
hidePost, pruneExpiredMutes (including its best-effort inner unmute), and
hydrateFromServer use runCatchingCancellable.

Part B (deferred from Task 8 review): hydrateFromServer's reconcile-by-delete
(P2-1, commit 914a87b) races a concurrent muteAccount/blockAccount/etc — a
mutation landing between hydration's pagination and its reconcile pass isn't in
the server snapshot hydration reconciles against, so reconcile deletes the row
the mutation just wrote. Adds a single hydrationMutex wrapping the entire body
of hydrateFromServer and the entire body of muteAccount/unmuteAccount/
blockAccount/unblockAccount (acquired once per call, released after
confirm-or-revert), so a mutation and an in-flight hydration can never
interleave. muteThread/unmuteThread operate on the separate unseeded
_mutedThreadUris field and are correctly left unlocked.

Note: the lock now also serializes the four mutation methods against each
other (not just against hydration) since it's held across each method's
network call — an intentional tradeoff for correctness. pruneExpiredMutes
remains unlocked and still races hydrateFromServer at startup; this is
pre-existing and out of scope (only the four named mutation methods were in
scope for the hydration race per this task).

The brief's literal cancellation test only checks reverted state, which
doesn't actually distinguish "rethrows cancellation" from "swallows it into
Result.failure and returns normally" — mocked suspend DAO calls don't really
suspend, so revert side effects fire either way. Verified empirically (see
task-9-report.md) and strengthened the test with a returnedNormally flag that
proves the call site is never reached when cancellation is rethrown.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_013ugq53zM9XpUqM4Watci33

fix: disable Android backup for account-sensitive local state (P1-6)

peregrine-ci / assembleDebug Build succeeded (46s)

Details

Peregrine CI / Build & Test (JDK 17) (pull_request) Successful in 5m3s

Details

Peregrine CI / Signed Minified Release Build (pull_request) Successful in 4m18s

Details

Peregrine CI / Instrumented Tests (API 29+) (pull_request) Has been skipped

Details

32e01e5931

starsetbyte commented

2026-07-13 01:30:20 +00:00

Author

Owner

Review pass 1 — revise

P1 — the async Room seed can republish stale moderation state across mutations and account boundaries. The app-lifetime init coroutine seeds flows outside hydrationMutex (ModerationActionsRepository.kt:71-80), while hydrate and mutations are mutex-protected. A delayed seed can re-add old mute/block DIDs after hydration, unmute/unblock, or logout cleanup; those flows drive moderation enforcement. Put seed/prune behind the same serialization plus account/session-generation boundary, and add deterministic seed-vs-hydrate/mutation/logout tests.

P2 — pruneExpiredMutes() still bypasses the mutex. It reads/deletes Room and modifies _mutedDids without hydrationMutex (:88-99), allowing hydration to republish an expired mute after local expiry removal. Serialize prune with hydration/mutations and test that interleaving.

The backup change itself is good: allowBackup=false is the appropriate conservative default. Targeted ModerationActionsRepository tests pass locally, but do not cover either race.

## Review pass 1 — revise **P1 — the async Room seed can republish stale moderation state across mutations and account boundaries.** The app-lifetime init coroutine seeds flows outside `hydrationMutex` (`ModerationActionsRepository.kt:71-80`), while hydrate and mutations are mutex-protected. A delayed seed can re-add old mute/block DIDs after hydration, unmute/unblock, or logout cleanup; those flows drive moderation enforcement. Put seed/prune behind the same serialization plus account/session-generation boundary, and add deterministic seed-vs-hydrate/mutation/logout tests. **P2 — `pruneExpiredMutes()` still bypasses the mutex.** It reads/deletes Room and modifies `_mutedDids` without `hydrationMutex` (`:88-99`), allowing hydration to republish an expired mute after local expiry removal. Serialize prune with hydration/mutations and test that interleaving. The backup change itself is good: `allowBackup=false` is the appropriate conservative default. Targeted ModerationActionsRepository tests pass locally, but do not cover either race.

starsetbyte referenced this pull request

2026-07-13 02:33:07 +00:00

fix: establish account operation lifecycle boundary #102

starsetbyte closed this pull request

2026-07-13 12:55:32 +00:00

starsetbyte commented

2026-07-13 13:01:21 +00:00

Author

Owner

Closed — merged via direct push

Branch content on main via shared commits. Remaining P1: async Room seeding in ModerationActionsRepository.init launches outside hydration mutex. Follow-up recommended.

## Closed — merged via direct push Branch content on main via shared commits. Remaining P1: async Room seeding in ModerationActionsRepository.init launches outside hydration mutex. Follow-up recommended.

starsetbyte commented

2026-07-26 03:22:31 +00:00

Author

Owner

Content from fix/p1-startup-and-backup shipped to main via a local --no-ff merge, which is why Forgejo shows this closed-unmerged rather than merged. Reconciled in docs/reviews/2026-07-25-branch-pr-reconciliation.md, which landed on main via #103. Branch deleted as part of that reconciliation.

Content from `fix/p1-startup-and-backup` shipped to `main` via a local `--no-ff` merge, which is why Forgejo shows this closed-unmerged rather than merged. Reconciled in `docs/reviews/2026-07-25-branch-pr-reconciliation.md`, which landed on `main` via #103. Branch deleted as part of that reconciliation.

peregrine-ci / assembleDebug Build succeeded (46s)

Required

Details

Peregrine CI / Build & Test (JDK 17) (pull_request) Successful in 5m3s

Details

Peregrine CI / Signed Minified Release Build (pull_request) Successful in 4m18s

Details

Peregrine CI / Instrumented Tests (API 29+) (pull_request) Has been skipped

Details

Pull request closed

This pull request cannot be reopened because the branch was deleted.

Rows
Columns

fix: async-safe repository construction, backup policy, moderation hydration + cancellation (P1-5, P1-6, P2-1, P2-2) #99

P1-5 — repository construction no longer blocks on Room/network

P1-6 — Android backup disabled by default

P2-1 — moderation hydration reconciles stale Room rows

P2-2 (moderation half) — cancellation-safe mutation + hydration serialization

Review pass 1 — revise

Closed — merged via direct push

Pull request closed