Skip to content

fix: non-owner Resolve/Un-Resolve blocked in server-sync mode#31

Merged
kasunben merged 1 commit into
mainfrom
fix/resolve-ownership
Jun 8, 2026
Merged

fix: non-owner Resolve/Un-Resolve blocked in server-sync mode#31
kasunben merged 1 commit into
mainfrom
fix/resolve-ownership

Conversation

@kasunben

@kasunben kasunben commented Jun 8, 2026

Copy link
Copy Markdown
Owner

Summary

  • Root cause: the resolve button handler called syncThread(t), which sends a full POST /threads upsert — an endpoint gated by checkOwnership. Any non-owner click → 403. The failure was silent (a console.warn) so the resolved state appeared to toggle locally but never reached the server, creating an invisible inconsistency with offline/BC/P2P modes where resolve is open to everyone
  • Server fix: PATCH /threads/:id/resolve now accepts { resolved: boolean, resolvedBy?, resolvedAt? } and handles both directions — resolved=true resolves, resolved=false un-resolves; no ownership check on this endpoint
  • Client fix: added syncResolve(thread) — same BC/P2P broadcast and dirty-flag cleanup as syncThread, but calls PATCH /threads/:id/resolve instead of POST /threads; resolve button handler now calls syncResolve instead of syncThread
  • Bumped version to 0.5.15; rebuilt annotate.min.js

Type

  • Bug fix

Affected modes

Server-sync (Mode 3) only — the bug was a server-side ownership gate. Offline, BroadcastChannel, and P2P modes were already unrestricted and are unaffected.

Testing

New integration tests (tests/integration/resolve-access.test.mjs — 7 tests):

  • Owner can resolve their own thread → 200
  • Non-owner can resolve a thread they did not create → 200
  • Non-owner can un-resolve a thread they did not create → 200
  • Un-resolve clears resolvedBy and resolvedAt
  • Resolve bumps updatedAt so incremental ?since= pulls see the change
  • Un-resolve bumps updatedAt so incremental pulls see the change
  • Returns 404 for a thread id that does not exist

Updated E2E test (tests/e2e/server-sync.spec.js):

  • expectResolve() helper added — watches for PATCH .../resolve instead of POST /threads
  • "User A resolves → User B sees resolved card after pull" passes with the new PATCH endpoint

Full suite: 89 unit + integration ✅ · 15/15 E2E ✅

Breaking changes

None. PATCH /threads/:id/resolve now accepts a resolved boolean field (previously it only resolved, never un-resolved). Callers that omit resolved get the same behaviour as before (resolved=true is the default path).

Checklist

  • server/routes/threads.js — endpoint expanded and ownership check confirmed absent
  • assets/js/annotate.jssyncResolve() added; resolve handler updated
  • tests/integration/resolve-access.test.mjs — 7 new tests
  • tests/e2e/server-sync.spec.js — resolve test updated to await PATCH
  • annotate.min.js rebuilt (npm run build)
  • Version bumped (0.5.140.5.15)
  • CLAUDE.md updated (Phase G, REST API table, Key Implementation Notes, Future work item removed)
  • README.md updated (Roadmap item moved to Shipped, sync checklist updated)

Closes #22

🤖 Generated with Claude Code

…22)

- server: expand PATCH /threads/:id/resolve to handle both resolve
  (resolved=true) and un-resolve (resolved=false); no ownership check on
  this endpoint — collaborative action open to any user
- client: add syncResolve(thread) that broadcasts to BC/P2P then calls
  PATCH /threads/:id/resolve; resolve button handler now calls syncResolve
  instead of syncThread, bypassing the ownership-gated POST /threads upsert
- tests: add resolve-access.test.mjs (7 integration tests) — non-owner
  resolve, non-owner un-resolve, updatedAt bump, 404 on missing id;
  update server-sync E2E helper to await PATCH /resolve instead of POST
- docs: remove closed item from Phase F future work and README roadmap;
  add to Phase G, REST API table, Key Implementation Notes, Shipped list,
  and sync checklist
- bump version to 0.5.15; rebuild annotate.min.js

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@kasunben kasunben marked this pull request as ready for review June 8, 2026 22:25
@kasunben kasunben merged commit 1ff2769 into main Jun 8, 2026
4 checks passed
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.

Server incorrectly rejects non-owner Resolve and Un-Resolve requests

1 participant