Skip to content

refactor(notes-ui): subclass app-client VaultClient (drop duplicated request loop) - #155

Merged
unforced merged 2 commits into
mainfrom
notes-subclass-vault-client
May 22, 2026
Merged

refactor(notes-ui): subclass app-client VaultClient (drop duplicated request loop)#155
unforced merged 2 commits into
mainfrom
notes-subclass-vault-client

Conversation

@unforced

Copy link
Copy Markdown
Contributor

Summary

app-client 0.1.0-rc.3 lifted request / requestWithRetry /
requestCursorWithRetry to protected (parachute-app#10),
closing the loop from notes#153's reviewer follow-up. Notes' VaultClient
now subclasses @openparachute/app-client's base class instead of
cloning the request loop.

  • 219 lines deleted from packages/notes-ui/src/lib/vault/client.ts
    (486 → 267).
  • 691 lines deleted from packages/notes-ui/src/lib/vault/client.test.ts
    (997 → 306). Base-class behaviors are covered by app-client's own
    suite; 14 Notes-specific tests retained.

Notes-specific surface retained on the subclass

Method Why retained
renameTag / mergeTags / deleteTag Tag-curation endpoints not in app-client's surface
listTagsWithSchema /api/tags?include_schema=true for schema-audit runner (notes#129)
linkAttachment Alias of base addAttachment — preserves Notes' caller-facing name + avoids a sweeping rename
fetchAttachmentBlob Audio/image render path; carries its own retry loop because the base's request* always parses JSON

Behaviour change worth flagging

Notes' previous narrow updateTag override was dropped. The base
class's updateTag accepts a wider TagUpsertPayload and returns a
TagRecord; the existing call sites (schema-ensure.ts → 2 calls)
pass {description, parent_names} (a subset of TagUpsertPayload) and
await without consuming the return — so the surface is
backward-compatible at the call site.

Bundle delta

Main chunk: 415.90 → 419.70 kB (+3.8 kB raw, +0.9 kB gzip). The
subclass mirrors a handful of auth-callback fields on the instance so
the blob path can drive its own retry loop without reaching into the
base's private state — see file header for the rationale.

Cross-refs

  • Initial Phase 2 adoption: notes#153 (f5b70cb)
  • request*protected in app-client: parachute-app#10 (rc.3)

Test plan

  • bun run typecheck — clean
  • bun run lint — clean
  • bun run test — 767/767 pass (90 files)
  • bun run build — clean
  • Manual smoke: install notes-ui under parachute-app, verify
    vaultInfo / queryNotes / createNote / linkAttachment /
    fetchAttachmentBlob (audio playback) / renameTag all still work
  • Auth-revoke smoke: confirm the blob path still fires
    onAuthRevoked when the post-refresh retry also 401s

🤖 Generated with Claude Code

unforced and others added 2 commits May 22, 2026 05:10
…request loop)

app-client 0.1.0-rc.3 lifted `request`, `requestWithRetry`, and
`requestCursorWithRetry` to `protected` (parachute-app#10), closing
the loop from notes#153's reviewer-flagged follow-up. Notes' VaultClient
now subclasses `@openparachute/app-client`'s base instead of cloning the
request/retry/error-classification loop.

- 219 lines deleted from `packages/notes-ui/src/lib/vault/client.ts`
- 691 lines deleted from `packages/notes-ui/src/lib/vault/client.test.ts`
  (base-class behaviors are covered by app-client's own suite; 14
  Notes-specific tests retained)

Notes-specific surface retained on the subclass:
  - `renameTag` / `mergeTags` / `deleteTag` — tag-curation endpoints
  - `listTagsWithSchema` — `/api/tags?include_schema=true` (schema-audit)
  - `linkAttachment` — alias of base `addAttachment` (preserves Notes'
    semantic + avoids a sweeping rename)
  - `fetchAttachmentBlob` — audio/image render path; carries its own
    retry loop because the base's `request*` always parses JSON

Notes' previous narrow `updateTag` override was dropped — the base
class's `TagUpsertPayload`-shaped `updateTag` accepts the same
`{description, parent_names}` shape `schema-ensure.ts` already passes,
and the caller ignores the return value.

Bundle delta: +3.8 kB raw / +0.9 kB gzip on the main chunk (the
subclass mirrors a handful of auth-callback fields on the instance so
the blob path can drive its own retry loop without reaching into the
base's `private` state).

Verification:
  - `bun run typecheck` clean
  - `bun run lint` clean
  - `bun run test` — 767/767 pass
  - `bun run build` — clean

Bumps `@openparachute/notes-ui` to 0.1.0-rc.3.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…ob onAuthRevoked detail

Two reviewer nits from notes#155:

1. `linkAttachment` now delegates to the inherited `addAttachment`
   rather than re-implementing the POST via `this.request(...)`. The
   wire shape is identical (POST `/api/notes/:id/attachments`); the
   thin wrapper preserves the Notes-facing name + semantic ("link an
   already-uploaded blob") while inheriting any future wire-shape
   changes from app-client automatically. Many callers reference
   `linkAttachment` directly (NoteNew, queue, queries, tests) so the
   alias stays.

2. `fetchBlobWithRetry` (the 401/403 path on attachment-blob loads)
   now parses the response body for `error_type` + `message` and
   forwards them to `onAuthRevoked` as the detail object, matching
   notes#150's enhanced-error shape and the base class's
   `requestWithRetry` behavior. Previously the blob path called
   `onAuthRevoked?.(res.status)` and dropped the detail, so banners
   triggered by attachment failures lost the parsed reason.

Body is read once with `res.text().catch(() => "")` before the
refresh-and-retry branch, mirroring the base class. Non-JSON bodies
leave detail undefined.

Reviewer nit #3 (a test specifically exercising the `setAccessToken`
override) is intentionally deferred — the dual-token sync is already
exercised indirectly by the `fetchAttachmentBlob` 401-retry test,
which verifies the rotated token reaches the second fetch call.

Tests: 768 passing (one new test added for the enhanced-detail blob
path). Typecheck + lint clean.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@unforced
unforced merged commit e974a00 into main May 22, 2026
@unforced
unforced deleted the notes-subclass-vault-client branch May 22, 2026 13:07
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