Skip to content

feat(servers): add cache integrity alert resolution - #339

Closed
RonenMars wants to merge 3 commits into
mainfrom
feat/cache-integrity-alert
Closed

feat(servers): add cache integrity alert resolution#339
RonenMars wants to merge 3 commits into
mainfrom
feat/cache-integrity-alert

Conversation

@RonenMars

@RonenMars RonenMars commented Jul 18, 2026

Copy link
Copy Markdown
Owner

Summary

Adds the mobile cache-integrity alert flow described in the implementation plan. When the streamer detects that cached conversation histories no longer match the JSONL files on disk, the app can now surface the pending alert, explain its severity, and let the user resolve it safely.

The implementation combines WebSocket delivery with a per-server REST sync so reconnects, foreground transitions, and missed unicast messages converge on the current server state.

Changes

Protocol and synchronization

  • Add shared cache-alert severity, resolution-action, and pending-alert types.
  • Handle cache_alert and cache_alert_resolved WebSocket messages alongside the existing server-level cache events.
  • Fetch GET /api/cache/alert for each connected server through React Query to recover from backgrounding or WebSocket delivery races.
  • Treat a 404 response as backward-compatible feature detection for older streamer versions.

State and conflict handling

  • Store pending alerts per server in the servers Zustand store.
  • Clear resolved alerts only when fingerprints match, preventing stale resolution messages from erasing newer alerts.
  • Reset pending alert state when a server disconnects.
  • Add POST /api/cache/alert/resolve support with typed success, already-resolved, and fingerprint-conflict outcomes.
  • Refetch and rerender the current alert after a 409 fingerprint_mismatch instead of retrying a destructive action against changed state.

User experience

  • Show low-severity alerts as a tappable banner on the hub.
  • Auto-open high-severity alerts in a modal using render-time store derivation, and close automatically when another client resolves the alert.
  • Offer prune-all, prune-selected, ignore, and reset-and-rescan actions.
  • Require an explicit confirmation step for every destructive action.
  • Provide a checkbox list for selecting individual missing conversations.
  • Surface the backup path in the success message when the server returns one.
  • Localize the complete flow in English, Hebrew, Russian, and Arabic.

Tests

  • Cover alert storage, replacement, fingerprint-guarded clearing, and disconnect cleanup.
  • Cover WebSocket dispatch for both cache-alert message types.
  • Cover banner visibility and interaction behavior.
  • Cover modal severity copy, confirmations, selection validation, all resolution request shapes, success handling, and conflict refresh.
  • Exercise the complete 404, success, 409, and server-error paths through a mocked fetch transport.

Verification

  • npm run test:ci — 110 suites passed; 1,064 tests passed and 1 skipped.
  • ESLint on every staged JavaScript/TypeScript file — passed.
  • Locale key-parity tests — passed.
  • Staged diff whitespace validation — passed.

npm run typecheck still reports existing Expo Router typed-route errors in navigation calls across touched and untouched files. This change does not modify those calls; the feature-specific code produced no TypeScript diagnostic.

Manual testing

Not run. End-to-end manual verification requires a streamer build that implements the cache-integrity alert contract.

Implementation plan

https://github.com/RonenMars/threadbase-mobile/blob/94145f1b82bd8d97754b595c005f1df7161d221f/docs/2026-07-18-cache-integrity-alert-mobile-plan.md

PR type

  • Feature
  • Fix
  • Refactor
  • Documentation
  • Test-only change

@RonenMars RonenMars mentioned this pull request Jul 18, 2026
Merged
2 tasks
RonenMars added a commit to RonenMars/threadbase-streamer that referenced this pull request Jul 18, 2026
@RonenMars
docs: add cache integrity alert design and streamer plan (#244)
e0255bc 
## Summary
- Add the cache integrity alert design doc and streamer implementation
plan under `docs/plans/`.
- Companion to the implementation work in
[#232](#232)
(`feat/cache-integrity-alert`).
- Mobile docs for the same feature already landed in
[threadbase-mobile#338](RonenMars/threadbase-mobile#338);
mobile implementation is
[#339](RonenMars/threadbase-mobile#339).

## Test plan
- [ ] Skim the two new plan docs for accuracy against #232
- [ ] Confirm no product code changed

Made with [Cursor](https://cursor.com)
RonenMars added 3 commits July 20, 2026 21:14
@RonenMars
feat(servers): add cache integrity alert resolution
40dd7ef 
Add typed cache integrity alert contracts across REST, WebSocket, and per-server Zustand state. Sync pending alerts through websocket broadcasts and foreground-aware REST queries so clients converge after reconnects or missed unicasts.

Present low-severity alerts in a banner and auto-open high-severity alerts in a localized resolution modal. Support prune-all, prune-selected, ignore, and reset-rescan actions; require confirmation for destructive choices; refetch on fingerprint conflicts; and surface backup paths on success.

Cover store lifecycle, websocket dispatch, banner and modal interactions, locale parity, backward-compatible 404 handling, successful resolution, and fingerprint conflicts with unit and mocked-transport integration tests.
@RonenMars
fix(cache): clear server state after destructive actions
53f0bcd
@RonenMars
test(cache): extend modal render timeout in CI
c8802b6
@RonenMars
RonenMars force-pushed the feat/cache-integrity-alert branch from 1990c81 to c8802b6 Compare July 20, 2026 18:15
@RonenMars RonenMars closed this Jul 31, 2026
RonenMars added a commit that referenced this pull request Aug 1, 2026
@RonenMars
docs(runbooks): retire the 2026-07-22 PR-landing runbook (#473)
c818bc0 
## Why

The runbook described itself as a "live procedure — edit it as PRs
land", but the chain it sequenced is gone.

Verified 2026-08-01 — every PR it orchestrated is `CLOSED` with
`mergedAt=null`, i.e. **closed, not merged**:

```
#339 #341 #343 #345 #346 #347 #353 #354 #355 #356 #357 #358 #359
#360 #361 #362 #363 #364 #368 #372 #373 #376 #385 #386 #387
```

The work reached `main` through the integration branch instead, and the
PRs were closed as redundant.
The integration branch the runbook treated as proof the set could
coexist, `integration-dev/v1.0.0-2026-07-22`, has been deleted.

That invalidates the phase order, the per-PR loop, the pre-flight sweep,
the decision to close `#355`, the conflict **A–I** resolutions, and the
definition of done.
The only PR row still accurate is `#291`, which is still open and still
excluded by standing request.

## What changed

The body becomes a tombstone: what the runbook was for, what actually
happened, and a table pointing at where each still-true trap now lives.
The file is kept rather than deleted so inbound links resolve and nobody
rebuilds the same plan from the merge report.

Two of its four traps were already in `docs/troubleshooting.md` (the
`.claude/` worktree gotcha and serial `SessionScreen` verification). The
other two are lifted there now under a new "CI signals" section, because
both are repo-wide facts rather than properties of those branches:

- a `[skip-ci]` head commit reports every required context green in
seconds having run nothing
- a stacked PR (base ≠ `main`) still gets full CI here, since
`test.yml`'s `pull_request` trigger has no `branches:` filter

Conflict **J** is obsolete for a separate reason: those
`ios/Podfile.lock` checksums encode the checkout's absolute path, so
resolving it was never meaningful. #469 added
`scripts/reset-podfile-lock-path-noise.sh`, which drops that drift.

## Two live findings, one already fixed

The runbook's `[skip-ci]` section claimed the required contexts were
`Type check`, `Unit tests`, `Integration tests`, `Lint`. The ruleset
actually required **only `Lint` and `Type check`** — unit and
integration tests could not block a merge at all, which makes the
`[skip-ci]` trap worse than documented, not better.

Its one outstanding post-merge action (add `i18n` to required checks,
never done since #368) is now complete: `Unit tests`, `Integration
tests` and `i18n` were added to the `main` ruleset on 2026-08-01.
Confirmed safe first — all three report `pass` on a real `chore/bump-**`
version-bump PR (#472), so the deploy pipeline's bumps are not
deadlocked.

Note: `.github/workflows/test.yml:25` carries the same wrong claim about
which contexts are required. Left alone here to keep this docs-only.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

1 participant

@RonenMars