Skip to content

Add document bookmarks - #13

Merged
tnsardesai merged 9 commits into
mainfrom
hypeship/bookmark-docs
Jul 24, 2026
Merged

Add document bookmarks#13
tnsardesai merged 9 commits into
mainfrom
hypeship/bookmark-docs

Conversation

@tnsardesai

@tnsardesai tnsardesai commented Jul 20, 2026

Copy link
Copy Markdown
Contributor

Summary

Adds document bookmarks for both humans (web) and agents (API). Every doc page gets a bookmark button that saves the doc to the signed-in user's bookmarks, /bookmarks lists them (split into "your documents" and "shared with you", mirroring /docs), and agents get the same capability over the /api/v1 surface.

Bookmarks re-resolve access on every read rather than trusting save-time access, so they honor the same authorization as the viewer:

  • A doc whose access is later revoked (grant removed, token rotated, or doc deleted) stays listed as revoked with no link, showing the title captured at bookmark time — never the live title of a doc the viewer can no longer see.
  • View-token sharing is preserved: the token a doc was bookmarked through is persisted (only when it actually matches the doc) and re-checked, so a token-shared private doc stays reachable until that token is rotated or the doc is deleted.
  • Gaining broader access later (public or a grant) upgrades the bookmark in place; the label follows the live access and the link drops the now-stale token.

Web

  • /bookmarks route with OWNED and SHARED-WITH-YOU sections, man-page styled like /docs.
  • Bookmark button on doc pages: a bookmark-icon toggle (filled when saved) that updates optimistically via fetch — no page reload, matching the comment/reaction controls beside it.
  • Per-row remove on /bookmarks, keyed by doc id so revoked/deleted docs can still be dropped.

API (agent, Bearer key)

  • PUT /api/v1/docs/{slug}/bookmark — bookmark a doc (idempotent). Requires view access (owner/grant/public/?viewtoken=); inaccessible → 404. Keyed by the key's email, so it unifies with the account's web bookmarks.
  • DELETE /api/v1/docs/{slug}/bookmark — remove (idempotent; works on revoked/deleted docs).
  • GET /api/v1/bookmarks?scope=owned|shared|all — list bookmarked docs, each item carrying the caller's re-resolved access (owner|editor|commenter|viewer|public|link|revoked), revoked, url, and bookmarked_at.

Implementation notes

  • Access re-resolution + token-persistence rules live in lib/docs/bookmarks.ts, shared by the web page and the API.
  • bookmarks table migration (bookmarker_email, doc_id, view_token, doc_title). Code-first OpenAPI + /llms.txt + SKILL.md regenerated; spec:check green (28 endpoints, 21 paths).

Tests

  • npm test (135, incl. lib/docs/bookmarks-view.test.ts), npm run build, npm run spec:check.
  • Verified end-to-end against a local Postgres + running server: web toggle/remove, token honoring across grant/public/rotation, and the full agent API (add/remove/list, scopes, 401/404, revoke-on-rotation).

@vercel

vercel Bot commented Jul 20, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
justhtml Ready Ready Preview, Comment Jul 24, 2026 7:26pm

- persist the view token a doc was bookmarked through and re-check access
  with it on /bookmarks, so a token-shared doc stays linkable (token
  re-appended) until the token is rotated or the doc is revoked
- make the doc-page bookmark button a real toggle (add/remove)
- add a per-row remove control on /bookmarks, working for revoked docs too
- revoked bookmarks show "<doc id> — revoked" with no link, never the
  live title of a doc the viewer can no longer access
- label token-only access "link" (was mislabeled "public")
- extract pure row rendering to lib/docs/bookmarks-view.ts with unit tests

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Add the /bookmarks web surface to the Viewing listing in skill-content.ts
(the source for /llms.txt and SKILL.md), alongside its sibling /docs.
Regenerated SKILL.md via gen:skill.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Replace the "bookmark"/"bookmarked" text button with a clickable star
(☆ unbookmarked, ★ bookmarked), matching the bar's glyph controls. Keeps
the toggle behavior and adds an aria-label since it's now icon-only.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Snapshot the doc title at bookmark time (bookmarks.doc_title) and render
it dimmed and unlinked on revoked rows, falling back to the slug when
untitled — replacing the bare "<id> — revoked". The snapshot is the title
the viewer saw when bookmarking, so a revoked row stays recognizable
without exposing the doc's current (possibly changed) title.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@tnsardesai

Copy link
Copy Markdown
Contributor Author

bookmark feature user interactions
https://github.com/user-attachments/assets/1e07de1e-019a-4936-813e-045984d39b28

what happens when a user revokes access
Screenshot 2026-07-24 at 11 41 11 AM

@tnsardesai
tnsardesai requested a review from rgarcia July 24, 2026 18:43
@tnsardesai
tnsardesai marked this pull request as ready for review July 24, 2026 18:43
Comment thread app/bookmarks/route.ts Outdated

@rgarcia rgarcia left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can it be the bookmark icon not the star icon?

Swap the ★/☆ glyph for an inline bookmark-ribbon SVG (filled when
bookmarked, outline when not), consistent with the app's other SVG icons.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The bookmark button did a native form POST → 303 → full page reload on
every click, unlike the comment/reaction controls beside it. Make it an
optimistic client toggle: flip the icon immediately, POST to /bookmarks
in the background, roll back on failure. /bookmarks now content-negotiates
— fetch (Accept: application/json) gets 204/4xx, the zero-JS list forms
still get the 303 redirect. Drops the now-unused bookmarkNext prop.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cursor Bugbot has reviewed your changes using high effort and found 1 potential issue.

There are 2 total unresolved issues (including 1 from previous review).

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit 6ded664. Configure here.

me,
initialSections,
} = props;
const [bookmarked, setBookmarked] = useState(props.bookmarked);

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bookmark icon stale after navigation

Medium Severity

The doc viewer keeps bookmark UI in useState(props.bookmarked) without resetting when the server sends a new bookmarked value for another document. Client navigations that reuse CommentsShell can show the previous doc’s filled or empty bookmark icon and wrong aria-pressed labels on the current page.

Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit 6ded664. Configure here.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Valid as a general React pattern, but not a reachable bug in this app, so leaving as-is.

So the trigger the check describes ("client navigations that reuse CommentsShell") does not exist. If soft routing between docs is ever introduced, the correct fix is key={slug} on <CommentsShell> so all prop-seeded state resets together — a bookmark-only useEffect would be the wrong shape.

On add, canViewSession can pass via ownership, a grant, or the doc being
public, so the submitted viewtoken wasn't necessarily the basis for access.
Storing it anyway persisted a stale/irrelevant token (and could overwrite a
good one on upsert), making the /bookmarks re-check fail after a grant is
removed even when a valid share token still exists. Store the token only when
it actually matches the doc's view_token; otherwise store null.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@tnsardesai
tnsardesai merged commit 76a5f52 into main Jul 24, 2026
5 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.

2 participants