Conversation
chore: sync main to develop
* fix(mobile): widen bottom-nav item padding and add bottom margin Increase --app-bottom-nav-height from 3.5rem to 4.5rem on mobile and add py-2 to each BottomNavTab and the MeTab. The taller bar gives each tap target more vertical breathing room, and because <main>'s padding-bottom is already derived from --app-bottom-nav-height, scrolled content now clears the translucent BottomNav by an extra 16px. モバイルでボトムナビ各項目の上下余白を広げ、メイン領域の下端マージンも 連動して大きくして、最終行がボトムナビに隠れにくくした。 https://claude.ai/code/session_01Rqq8FUGwBUcZEipPDPcFJh * fix(mobile): align CSS var fallbacks with new 4.5rem nav height Update three stale `var(--app-bottom-nav-height, 3.5rem)` fallbacks left over from the previous height. If the custom property ever fails to resolve, the fallback now matches the actual mobile height, so <main>'s padding-bottom and the nav bar's own height stay consistent. Addresses Devin Review feedback on PR #799. https://claude.ai/code/session_01Rqq8FUGwBUcZEipPDPcFJh --------- Co-authored-by: Claude <noreply@anthropic.com>
* chore(deps): bump the minor-and-patch group with 2 updates Bumps the minor-and-patch group with 2 updates: [@anthropic-ai/sdk](https://github.com/anthropics/anthropic-sdk-typescript) and [prettier-plugin-tailwindcss](https://github.com/tailwindlabs/prettier-plugin-tailwindcss). Updates `@anthropic-ai/sdk` from 0.91.1 to 0.92.0 - [Release notes](https://github.com/anthropics/anthropic-sdk-typescript/releases) - [Changelog](https://github.com/anthropics/anthropic-sdk-typescript/blob/main/CHANGELOG.md) - [Commits](anthropics/anthropic-sdk-typescript@sdk-v0.91.1...sdk-v0.92.0) Updates `prettier-plugin-tailwindcss` from 0.7.4 to 0.8.0 - [Release notes](https://github.com/tailwindlabs/prettier-plugin-tailwindcss/releases) - [Changelog](https://github.com/tailwindlabs/prettier-plugin-tailwindcss/blob/main/CHANGELOG.md) - [Commits](tailwindlabs/prettier-plugin-tailwindcss@v0.7.4...v0.8.0) --- updated-dependencies: - dependency-name: "@anthropic-ai/sdk" dependency-version: 0.92.0 dependency-type: direct:production update-type: version-update:semver-minor dependency-group: minor-and-patch - dependency-name: prettier-plugin-tailwindcss dependency-version: 0.8.0 dependency-type: direct:development update-type: version-update:semver-minor dependency-group: minor-and-patch ... Signed-off-by: dependabot[bot] <support@github.com> * chore(deps): sync bun.lock with package.json changes --------- Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
* feat(api): add Sentry error capture Co-authored-by: Cursor <cursoragent@cursor.com> * fix: address PR #810 review comments - Switch from @sentry/bun to @sentry/node to match the Node.js runtime - Limit Sentry event scrubbing to PII-bearing fields and guard against Date/RegExp loss and cyclic references - Use Hono c.req.routePath for Sentry "routePath" extra to avoid leaking capability tokens (e.g., /api/invite/:token) - Redact sensitive URL query params embedded in string values - Use vi.importActual partial mock so errorHandler tests exercise the real shouldCaptureApiException predicate Co-authored-by: Cursor <cursoragent@cursor.com> * fix: address PR #810 review comments Redact all request cookie values before sending Sentry events so session identifiers cannot leak through cookie names that are not explicitly listed as sensitive. Co-authored-by: Cursor <cursoragent@cursor.com> --------- Co-authored-by: Cursor <cursoragent@cursor.com>
…811) * feat(api,db): add api_errors table and apiErrorService (Epic #616 Phase 1) Adds the aggregated `api_errors` table and `apiErrorService` defined in Epic #616 Phase 1 / sub-issue #802. Raw stack traces stay in Sentry; this table only stores per-issue state (occurrences, severity, status, AI analysis output, GitHub issue mapping) keyed on `sentry_issue_id`. - Schema: `server/api/src/schema/apiErrors.ts` with severity / status enums and indexes for the admin error page hot paths. - Migration: `0019_add_api_errors.sql` + `_journal.json` entry. - Service: `apiErrorService.upsertFromSentrySummary` increments `occurrences` and advances `last_seen_at` (GREATEST) while preserving `first_seen_at`; list / get / status-update helpers; status transition validation rejects illegal jumps such as `ignored -> resolved`. - Tests: 25 Vitest cases covering transition rules, the upsert contract (initial insert + recurrence with occurrences increment and first_seen_at preservation), list filtering, single-row lookups, and status update with invalid / same-state / not-found branches. Refs #616 Closes #802 * fix(api): harden api_errors upsert/update and add DB enum checks Addresses PR #811 review feedback: - upsertFromSentrySummary now defensively validates external webhook input (null-safe sentry_issue_id and title, finite occurrencesDelta) before building the insert payload. - On recurrence the upsert reopens rows whose status was 'resolved' so regressions surface in the admin's open view; 'ignored' stays put to respect the explicit admin decision. - updateApiErrorStatus is now race-free: the UPDATE WHERE clause pins on the previously-read status, and a 0-rows result throws the new ApiErrorStatusConflictError (caller maps to HTTP 409). - Migration adds CHECK constraints for status/severity so the DB rejects values outside the workflow; the Drizzle column types use enum literals to mirror the constraint at the type level. - Tests grow to 30 cases covering the new validation, recurrence reopen, and concurrent-modification branches. * fix(api): validate statusCode/timestamps on api_errors upsert Hardens the webhook entry point with two more boundary checks and adds the symmetric "ignored stays ignored" recurrence test, addressing the follow-up CodeRabbit review on #811. - statusCode that is not a finite integer (NaN, Infinity, fractional) now throws "statusCode must be a finite integer". - firstSeenAt / lastSeenAt that resolve to Invalid Date throw "firstSeenAt|lastSeenAt must be a valid Date". - Validation extracted into normalizeUpsertInput so the public upsertFromSentrySummary stays under the cyclomatic-complexity ceiling. - New test asserts that a recurrence on a row whose status is "ignored" preserves the ignored decision (locks down the CASE expression's ELSE branch, complementing the resolved → open test). * fix(api): coerce listApiErrors pagination to integers and reattach TSDoc Two CodeRabbit follow-ups on PR #811: - listApiErrors now floors limit/offset before clamping. A fractional or NaN value from a query string would otherwise reach Postgres as a non-integer LIMIT/OFFSET and fail at runtime; non-finite inputs fall back to the documented defaults. Adds a regression test for fractional and NaN/Infinity inputs. - Restored the upsertFromSentrySummary TSDoc directly above its declaration. The earlier extraction of normalizeUpsertInput inserted the helper between the public function and its docs, which left the exported API undocumented at the declaration site. * fix(api): enforce firstSeenAt <= lastSeenAt monotonicity on upsert Addresses CodeRabbit follow-up on PR #811: a webhook supplying firstSeenAt > lastSeenAt could persist an inverted timeline into api_errors and silently break the first/last-seen invariant. The helper now throws "firstSeenAt must be less than or equal to lastSeenAt" when the two timestamps disagree, and threads the normalized firstSeenAt through to the insert payload so the public function relies on a single source of truth. Adds tests for the inverted-timeline rejection and the equal-timestamps boundary. --------- Co-authored-by: Claude <noreply@anthropic.com>
* feat(api): add Sentry webhook receiver and admin errors API (#803) - POST /api/webhooks/sentry with HMAC-SHA256 signature verification using SENTRY_WEBHOOK_SECRET; payload is normalized via extractSentrySummary and upserted via apiErrorService.upsertFromSentrySummary. - /api/admin/errors GET list (paginated + status/severity filters), GET :id, PATCH :id status (delegates to apiErrorService updateApiErrorStatus, maps ApiErrorStatusConflictError to 409, invalid transitions to 400). - Mount new routes in app.ts and admin/index.ts; add SENTRY_WEBHOOK_SECRET to server/api/.env.example. - Vitest coverage: signature OK / NG / missing / unconfigured secret, ignored payloads (no sentry_issue_id), invalid JSON, plus admin route auth/role and PATCH transition / 404 / 409 cases. Refs Epic #616 (Phase 1). * refactor(api): typed errors, UUID validation, and data.error.id extraction (#803) Address review feedback on PR #812: - apiErrorService: introduce ApiErrorValidationError and ApiErrorInvalidTransitionError so route handlers can branch on `instanceof` instead of regex-matching error messages. - routes/webhooks/sentry: replace regex-based 400 detection with `instanceof ApiErrorValidationError`, and extend extractSentryIssueId to also read `data.error.id` so payloads where the identifier only lives under `data.error` are no longer silently dropped (Codex P1). - routes/admin/errors: validate `:id` matches the UUID pattern up front (return 404 instead of letting Postgres surface a 500), guard against `c.req.json()` returning literal `null`, and use the typed transition error for 400 mapping. - Tests: cover invalid-UUID 404, JSON-null body 400, and the data.error.id extraction path. * refactor(api): scrub Sentry route URLs and de-duplicate status enum (#803) CodeRabbit review feedback (PR #812): - routes/webhooks/sentry: reorder extractRoute to prefer the scrubbed `transaction` tag, then issue.metadata.transaction, then issue.shortId, before falling back to request.url. URL fallback now reduces the value to its pathname (origin / query / fragment dropped) so capability tokens and absolute hosts never persist into api_errors.route. Adds tests for the new ordering and the URL-stripping behavior. - routes/admin/errors: derive VALID_STATUSES from ALLOWED_API_ERROR_STATUS_TRANSITIONS so adding a new ApiErrorStatus automatically expands the accepted PATCH inputs without requiring a manual edit on this list. --------- Co-authored-by: Claude <noreply@anthropic.com>
…egration (#813) * feat(admin,frontend): add Sentry React SDK + ErrorBoundary and admin /errors UI (#804) Wires up `@sentry/react` for the frontend (`src/`) and admin (`admin/`) SPAs with `Error Boundary` to forward unhandled exceptions, both gated on the `VITE_SENTRY_DSN_WEB` / `VITE_ADMIN_SENTRY_DSN` env vars (no-op when blank). Adds an admin `/errors` page (list + detail + status update) backed by the existing `/api/admin/errors` endpoints, plus a sidebar badge that polls for the count of active (`open` + `investigating`) errors. Phase 1 uses simple polling at a 30s interval; realtime sync is deferred to a later phase. * fix(admin): guard useApiErrorActiveCount against overlapping requests Polling ticks and visibilitychange callbacks can race. Track the latest request id (mirroring `useApiErrors`) and discard stale responses so a slow earlier fetch can't overwrite a fresher count. Addresses gemini-code-assist review on PR #813. * chore(admin): address CodeRabbit review feedback on PR #813 - i18n: add `_one` / `_other` plural keys for `errors.detail.occurrencesShort` (was rendering "1 occurrences"). Pass `count` as a number so i18next plural resolution actually fires. - i18n: make `nav.unreadBadgeAriaLabel` self-describing for screen readers ("{{count}} unresolved API errors"). - sentry: extend `captureException` with an optional `{ extra }` context and forward `componentStack` from both `ErrorBoundary` components so Sentry events capture the failing subtree. - admin/src/api/admin.ts: expose `API_ERROR_STATUS_VALUES` and `API_ERROR_SEVERITY_VALUES` as the single source of truth; drop the local copies in `ErrorsContent` and `ErrorDetailDialog`. - useApiErrorActiveCount: fix JSDoc to match actual behavior (preserves last successful count on errors), and guard the bootstrap fetch with the same visibility check as the polling tick so a hidden tab doesn't pay for the initial fan-out. Skipped: extracting `ErrorBoundary` into a shared package — out of scope here. --------- Co-authored-by: Claude <noreply@anthropic.com>
* feat(api): repository_dispatch + AI analysis callback (#805) Wire up Epic #616 Phase 2: when the Sentry webhook upserts a brand-new sentry_issue_id, fire-and-forget a GitHub `repository_dispatch` (`event_type: analyze-error`) so a downstream Actions workflow can run AI analysis on the error. Add a callback endpoint `PUT /api/webhooks/github/ai-result/:id` for the workflow to write back ai_summary, ai_suspected_files, ai_root_cause, ai_suggested_fix, and severity, authenticated via the GitHub App installation token. - New `lib/githubAppAuth.ts`: App JWT (RS256) → installation token with in-memory caching, repository_dispatch trigger, installation-token verification. - New `routes/webhooks/githubAiCallback.ts`: PUT endpoint mounted outside the admin gate; validates Bearer installation tokens via the GitHub API and rejects non-matching installation IDs. - `services/apiErrorService.ts`: `updateAiAnalysis` helper with boundary validation for severity / suspected-files shape. - `routes/webhooks/sentry.ts`: pre-upsert SELECT to detect first-sight, then fire dispatch only on isNew=true. Failures are logged, never thrown — issue #805 acceptance criterion: API stays functional even when the Actions workflow is not deployed yet. - Adds `GITHUB_APP_ID`, `GITHUB_APP_PRIVATE_KEY`, `GITHUB_APP_INSTALLATION_ID`, `GITHUB_DISPATCH_REPOSITORY` env vars. - 25 new Vitest cases covering token cache, dispatch flow, callback auth/validation, and isNew/recurrence branching. Closes #805 * fix(api): tighten installation-token verification + drop test PEM (#805) Address review feedback on PR #814: 1. P1 security: `verifyInstallationToken` previously fell back to `true` whenever `x-github-installation-id` was missing — and that header is not actually documented as a response of `GET /installation/repositories`, so the fallback was always taken. This let any installation token of the same App write AI fields into arbitrary `api_errors` rows. Switch to `GET /installation`, which returns the installation's own metadata (including `id`), and require the `id` to equal `GITHUB_APP_INSTALLATION_ID`. Tokens for other installations of the same App now fail closed. 2. Security CI (gitleaks): the test file embedded a real-looking RSA PEM to drive `createAppJWT`'s signing path, which gitleaks rightly flagged. Replace it with a `jose` module mock (a constructible stub class for `SignJWT` + a no-op `importPKCS8`) so the test no longer ships a key. Adds 6 new tests covering the new `verifyInstallationToken` branches (matching id, mismatched id, non-2xx, malformed body, missing id field, empty token). * chore: suppress historical PEM finding in .gitleaksignore (#805) The Security CI scans full git history, not just HEAD. Commit d10d11f (in PR #814) included a disposable RSA test key that f6825d7 later removed by switching to a `jose` module mock. The blob still exists in the branch's git history, so gitleaks keeps flagging it. Add the fingerprint to .gitleaksignore — same pattern the repo already uses for test-only mock secrets (see the invite.test.ts entry). The key was never used to sign anything outside the test process, so this is a true false-positive at this point. Verified locally: `gitleaks detect --log-opts="develop..HEAD"` reports 0 leaks after the suppression. * fix(api): timeouts + distinguish transient GitHub failures (#805) Address two CodeRabbit review findings on PR #814: 1. Add AbortController-based timeouts (10s) to both outbound GitHub fetch calls in `getInstallationToken` and `triggerRepositoryDispatch` via a shared `fetchGitHubWithTimeout` helper. Without this, a stalled GitHub API leaves the fire-and-forget Sentry path with a forever-pending promise; under bursty webhooks those accumulate. 2. Make `verifyInstallationToken` differentiate auth failures from GitHub-side outages. Previously it returned `false` for everything — including 5xx, network errors, timeouts, and malformed JSON — so the callback's existing 503 path never fired. Now: - 401/403/404 + id mismatch + missing id → `false` → route 403 - 5xx, network, abort, malformed JSON → throws `GitHubInstallationVerificationError` → route 503 (retryable) This stops a transient GitHub outage from silently dropping a valid AI result as a permanent auth failure. Skipped the third (nitpick) finding on single-flight cache: the dispatch path runs at most once per new Sentry issue, so concurrent cache misses are extremely rare and benign (last write wins). Adding in-flight tracking is maintenance burden for a non-measurable win. Adds 4 new tests covering 5xx / network / malformed-JSON throws and a 503-from-route case via the callback. * fix(api): use `data` key for AI callback success response (#805) Switch the AI callback's success payload from `{ error: updated }` to `{ data: updated }`. The admin api_errors routes use `{ error: row }` because their consumer (admin/src/api/admin.ts) is internal and tightly coupled to that shape, but this webhook is consumed by external GitHub Actions workflows where reusing the `error` key for success makes "presence-of-error means failure" no longer hold and is genuinely confusing. No backward-compat concern: the AI workflow that calls this endpoint has not been written yet, so there is no existing consumer to break. Test updated to expect the new shape. --------- Co-authored-by: Claude <noreply@anthropic.com>
* feat(ci): analyze-error.yml + Claude AI analysis composite action (#806) Implements Epic #616 Phase 2 / sub-issue #806: a `repository_dispatch` (`event_type: analyze-error`) workflow that asks Claude (Anthropic) for a structured analysis of a Sentry-detected API error and PUTs the result back to the API callback (`PUT /api/webhooks/github/ai-result/:id`) landed in #805. - `.github/workflows/analyze-error.yml` — repository_dispatch + workflow_dispatch (with `dry_run` / `skip_callback` defaults of true so it stays green pre-secrets-rollout). Mints a short-lived installation token via `actions/create-github-app-token@v2`. Concurrency-grouped by `sentry_issue_id` so duplicate webhooks do not double-bill Anthropic. - `.github/actions/claude-analyze/action.yml` — composite action orchestrating bun install → analyze script → curl PUT (2 retries on 5xx, immediate fail on 4xx). - `.github/actions/claude-analyze/analyze.mjs` — derives keywords from title/route, runs `git grep -l` to gather up to 6 candidate files (head 80 lines each), renders `prompt.md`, calls Anthropic SDK with retry, validates output via Zod, writes JSON to `$GITHUB_OUTPUT` path. - `.github/actions/claude-analyze/schema.mjs` — Zod schema mirroring the server's `updateAiAnalysis` boundary (severity enum + suspected-file shape) so a malformed Claude response fails CI rather than writing garbage back. - `.github/actions/claude-analyze/prompt.md` — bilingual prompt template with the Epic #616 severity rubric (high/medium/low/unknown) and a fixed JSON output contract. - `__tests__/schema.test.mjs` + `fixtures/*.json` — 12 fixture cases using Node 24's built-in test runner (no new vitest workspace). - README.md documents the `client_payload` contract, required secrets, local + workflow_dispatch dry-run recipes, and retry semantics. Closes #806 * fix(ci): address PR #815 review feedback (#806) CodeRabbit / Codex / Gemini review fixes — all minimal, no architectural changes: - analyze.mjs: replace `path.dirname(new URL(...).pathname)` with `import.meta.dirname` (Node 20.11+, Windows-safe). - analyze.mjs: switch keyword extraction regex to Unicode property classes (`\p{L}\p{N}`, `u` flag) so non-ASCII Sentry titles (Japanese error messages, …) keep their tokens instead of being stripped to empty. - analyze.mjs: collapse the per-keyword `git grep` loop into a single `git grep -e KW1 -e KW2 ...` call. OR semantics are preserved (set union); avoids per-keyword process startup overhead. - analyze.mjs: only require `ANTHROPIC_API_KEY` when `dryRun` is false, so the default `workflow_dispatch` (`dry_run=true` + `skip_callback=true`) works in fork PRs and pre-secrets environments. - analyze.mjs: redact `title` / `route` in the startup log; emit `title_len` / `route_present` and a `keywords_count` only. Sentry's data scrubbing remains the primary defense; CI logs are a separate retention plane (defense-in-depth per Epic #616). - analyze.mjs: cap the dry-run stub's `ai_suspected_files` at 5 entries to keep it consistent with the new schema cap (below). - action.yml: add `--connect-timeout 10` and `--max-time 30` to the callback `curl` so a hung TCP session can't burn the 10-minute job timeout on a single PUT before the retry loop progresses. - prompt.md: add `text` language hint to the `{{repo_excerpts}}` fence (markdownlint MD040). - schema.mjs: add `.max(5)` to `ai_suspected_files`. The 5-entry cap was already documented in `prompt.md` and the README; enforce it at the schema layer so a Claude response that ignores the instruction fails CI rather than being PUT to the API. - __tests__: add `invalid-too-many-files.json` fixture + a 13th test case asserting `parseAndValidate` rejects 6+ entries. Skipped: `actions/setup-node@v6` Gemini suggestion — `v6` is the established repo convention used 16+ times in existing workflows (ci.yml, deploy-dev.yml, deploy-prod.yml, …) and the PR's Lint / Build jobs already pass on the same major. * fix(ci): use fileURLToPath for entry-point check on Windows (#806) CodeRabbit review nitpick on PR #815: the `invokedDirectly` check still used `new URL(import.meta.url).pathname`, which yields a POSIX-style `/C:/path/...` on Windows that never matches `path.resolve(process.argv[1])` (`C:\path\...`). CI runs on Linux so this didn't affect the workflow, but the local-dev path on Windows would silently skip `main()` and exit 0 with no output. `fileURLToPath` normalizes correctly across platforms and is consistent with the earlier `import.meta.dirname` migration for `HERE`. * fix(ci): git grep -F + disable SDK retries (#806) Two more CodeRabbit nitpicks on PR #815: - analyze.mjs (grepCandidateFiles): add `git grep -F` so keywords match literally. Sentry titles often contain regex metacharacters (`.`, `(`, …) and the previous regex mode was broadening matches and diluting the candidate-file ranking with unrelated hits. - analyze.mjs (Claude client): pass `maxRetries: 0` when constructing the Anthropic client. The SDK defaults to `maxRetries: 2`, which combined with the outer `callClaudeWithRetry` loop (`MAX_ATTEMPTS=2`) produced a worst-case 2×2=4 upstream calls — outside issue #806's "1〜2 回まで" budget. The outer loop now owns retries exclusively. --------- Co-authored-by: Claude <noreply@anthropic.com>
* feat(admin): sse realtime updates for /admin/errors (Epic #616 Phase 2 / #807) - add in-memory `apiErrorBroadcaster` for fan-out of `api_errors` updates - expose `GET /api/admin/errors/stream` (text/event-stream, adminRequired) with initial `ready` event, periodic keep-alive comments, and subscriber cap - publish from Sentry webhook, GitHub AI callback, and admin PATCH so the UI receives push updates from every mutation path - switch `useApiErrors` to subscribe via `EventSource` with reconnect/cleanup; fallback polling kicks in only when the stream is down - tests: broadcaster unit, SSE route headers + push delivery, hook merge/filter Closes #807 * fix(admin): address PR #816 review feedback for SSE realtime - route SSE URL through `getApiUrl` so split-origin admin builds reach the API host instead of the Cloudflare Pages origin - pre-check subscriber cap before `streamSSE` so capacity exhaustion returns a real 503 instead of 200 + SSE error event - clear pending `reconnectTimer` in the visibilitychange handler to avoid spawning a duplicate `EventSource` - move SSE-updated rows to the front of the list (matches the server's `last_seen_at DESC` ordering on a fresh REST fetch) * fix(admin): use raw SSE comment line for keep-alive ping `stream.writeSSE({ event: "ping" })` always emits an `event:` field, so the client receives a named ping event — not the invisible keep-alive the comment claimed. Switch to `stream.write(": ping\n\n")` so the heartbeat is a true SSE comment line (ignored by EventSource per the spec) while still keeping idle proxies from tearing down the TCP connection. PR #816 review feedback. --------- Co-authored-by: Claude <noreply@anthropic.com>
…616 Phase 3) (#817) * feat(ci): auto-file or comment GitHub Issues from analyze-error severity (#808) AI 判定 severity が high / medium のときだけ GitHub Issue を起票し、同一 sentry_issue_id の既存オープン Issue があればコメント追記で再発を表現する (連続 100 回でも 1 件に集約)。low / unknown はスキップ (DB / Sentry のみ)。 Adds an `autoIssue.mjs` helper module with pure builders and a `runAutoIssue` orchestrator (fetch is injectable for tests), plus a runner script wired into the composite action. The workflow declares `issues: write` for visibility; the actual writes use the existing GitHub App installation token. Issue body omits the Sentry URL to avoid leaking the org / project slug — the `sentry-issue:<id>` label and id are sufficient for cross-reference. Implements Epic #616 Phase 3. * fix(ci): address auto-issue review feedback (truncation, pipe escape, race) Three review-driven fixes to autoIssue.mjs: - buildIssueTitle: append `...` when the title is sliced past TITLE_BODY_MAX so readers see the truncation at a glance (still ≤ 256 chars). - inlineOrNone: escape `|` to `\|` so a stray pipe in `route` or AI-generated text cannot break the surrounding Markdown table. - ensureLabel: tolerate 422 from POST /labels — different sentry_issue_id runs can race on the shared `auto-reported` / `monitoring` labels, and a 422 just means the label already exists. Adds three test cases (ellipsis suffix, pipe escape in body, 422 race tolerance + GET-only fast path). * fix(ci): escape backslashes before pipes in inlineOrNone (CodeQL) CodeQL flagged the previous one-pass `\|` → `\\|` replace as incomplete string escaping: an input of `\|` would emerge as `\\|`, which Markdown reads as a literal backslash plus a raw pipe — breaking the surrounding table cell. Escape backslashes first, then pipes, so `\|` → `\\\|` (literal `\` + escaped `|`). Adds a regression test covering the `\|` input case. --------- Co-authored-by: Claude <noreply@anthropic.com>
…818) * feat(api): add email-only notifier for high/medium API errors (#809) Epic #616 Phase 3 / sub-issue #809。重要 API エラー (severity=high/medium) の メール通知サービス `services/notifier.ts` を新設し、AI 解析コールバック (`PUT /api/webhooks/github/ai-result/:id`) の成功時に 1 回だけ呼ぶよう配線する。 Slack はこのリポジトリでは使わないため非対応。`MONITORING_NOTIFY_EMAIL` 未設定 / severity が low/unknown の場合は no-op。本文には sentry_issue_id / severity / title / 管理画面 URL のみ載せ、Authorization / Cookie / 内部 URL など PII は混入させない。 Phase 3 / #809: introduce `notifier.ts` for email-only alerts on high/medium severity API errors. Slack is intentionally unsupported. The notifier no-ops when `MONITORING_NOTIFY_EMAIL` is unset or severity is low/unknown. The AI callback is the single call site so duplicate alerts can't happen, and sends are fire-and-forget so the webhook response isn't delayed by Resend. * fix(api): address review feedback on notifier (#809) - gate `notifyApiErrorAlert` on severity transitions so retries / partial callbacks no longer re-send (Codex P1). Route now reads the row before `updateAiAnalysis` and only fires when severity transitions from a non-notifiable value into `high`/`medium`. - validate `ADMIN_BASE_URL` scheme (`http:` / `https:` only) and drop malformed values to prevent dangerous URLs in alert HTML (CodeRabbit). - reorder `.env.example` so `ADMIN_BASE_URL` precedes `MONITORING_NOTIFY_EMAIL` to satisfy dotenv-linter (CodeRabbit). - adopt `getOptionalEnv` for env access for parity with `emailService.ts` and strip CR/LF from email subject as defense-in-depth against header injection (gemini-code-assist). Tests cover: first-sight escalation fires once; retry (high→high), partial callback (no severity), and downgrade do NOT fire; non-HTTP schemes never produce an `href`; CR/LF never survives in the subject. --------- Co-authored-by: Claude <noreply@anthropic.com>
* fix(api): seed thumbnail tier quotas to unblock web-clipper saves Without this seed `thumbnail_tier_quotas` is empty, so `getStorageQuotaBytes` silently falls back to a 10 MB ceiling (commitService.ts) and every Web Clipper save hits STORAGE_QUOTA_EXCEEDED (HTTP 413) after only a handful of clips — typical og:image assets are 0.5–2 MB each. Seeds free=100 MB and pro=10 GB to align with the AI tier seed approach (`0002_seed_ai_tier_budgets.sql`); reuses ON CONFLICT DO UPDATE so reseeds pick up limit changes safely. [skip drizzle-check] * feat(thumbnail): gc orphans on page delete + upgrade prompt on quota exceed Two changes that share the same response/UI surface, kept in one commit because they touch overlapping code paths (commit response shape and useWebClipperDialogSubmit's error branches). GC mechanism: - Add pages.thumbnail_object_id (migration 0021). Nullable, no FK — thumbnail lifecycle is owner-scoped and managed by the API layer rather than relying on cascade. Without this, deleted pages leave thumbnails in S3 and DB forever, slowly burning every user's quota. - commitImage now also returns objectId. POST /api/thumbnail/commit surfaces it so the client can persist it on the page row. - POST /api/pages accepts thumbnail_object_id; DELETE /api/pages/:id reads it back, NULLs it on the soft-delete, and calls a shared thumbnailGcService.deleteThumbnailObject helper that does an ownership-scoped DB delete first, then S3 delete (logging orphans). GC is best-effort and never blocks page deletion. - thumbnailGcService lazy-inits its S3 client so unrelated pages tests don't have to set STORAGE_* just to import the route. - commitService fallback quota raised from 10 MB to 100 MB to match the new free seed (drizzle/0020_…); kept as a safety net for unseeded envs. Upgrade prompt on STORAGE_QUOTA_EXCEEDED: - /api/thumbnail/commit's 413 response now includes code: "STORAGE_QUOTA_EXCEEDED" so clients can branch reliably. - thumbnailCommit throws a typed QuotaExceededError on 413 (or on matching code, regardless of status). - useWebClipperDialogSubmit catches QuotaExceededError, aborts the submit (so we never silently create a thumbnail-less page on a failure the user should know about), and shows a toast with an Upgrade action that routes to /pricing. ja/en strings added. - Renamed useWebClipperDialogSubmit.ts → .tsx so the JSX action element type-checks against ToastActionElement without a cast. Tests: - commitService: covers objectId pairing with imageUrl, plus the 100 MB fallback path when the quota table is unseeded. - /api/thumbnail/commit: response now exposes objectId and 413 carries the STORAGE_QUOTA_EXCEEDED code. - thumbnailGcService: covers DB-only no-op, owner-scoped delete pair, best-effort S3-failure swallowing, and NoSuchKey idempotency. - thumbnailCommit (client): success shape, 401 → AuthRedirectError, 413 → QuotaExceededError, code-matching fallback on non-413, and generic Error otherwise. * fix(api): gc thumbnails using page owner id, not deleter id In a shared note the user performing the page delete may differ from the page owner, but `thumbnail_objects` is owner-scoped — `deleteThumbnailObject` matches `(id, user_id = …)`. Passing the actor's `userId` made the predicate miss when a collaborator deleted someone else's page: the page row was already soft-deleted and `thumbnail_object_id` cleared, so the blob got orphaned and continued to count against the real owner's quota. Capture `pages.ownerId` alongside `thumbnailObjectId` and pass that into the GC helper so the predicate matches in collaborative deletions too. Reported by gemini-code-assist and chatgpt-codex on PR #819. * fix(web-clipper): roll back thumbnail when page create fails Two related issues raised on PR #819 by coderabbitai. Together they let a transient page-create failure orphan a committed thumbnail and burn quota until manually reclaimed: 1. handleWebClipped used to swallow the createPage error (toast + log + return), so the submit hook never learned about the failure and closed the dialog. The thumbnail row had already been written by POST /api/thumbnail/commit but had no page row referencing it, making it invisible to the page-delete GC path added in the prior commit. 2. useWebClipperDialogSubmit invoked onClipped synchronously and immediately closed the dialog, so even when the page-create promise later rejected the user only saw a stale toast — the dialog was already gone. Adds a best-effort deleteCommittedThumbnail helper that DELETEs /api/thumbnail/serve/:id (the same owner-scoped endpoint already used by the editor), and wires: - handleWebClipped catches createPage errors, calls the rollback for the committed thumbnail, then rethrows so callers can react. - onClipped is typed as Promise<void> | void; the submit hook awaits the result, leaves the dialog open + isSubmitting truthy on rejection, and closes only on successful resolve. submitGenerationRef guard preserved so a dialog re-open mid-await does not stomp state. Also fills the previously-empty TSDoc on the two exported interfaces in useWebClipperDialogSubmit.tsx (per PR feedback) and adds tests: - deleteCommittedThumbnail: credentialed DELETE shape, network-error swallowing, no-op on missing baseUrl/objectId, URL encoding. * fix(web-clipper): close two thumbnail-leak windows after commit Both raised by coderabbitai on PR #819, same root cause: a thumbnail that was already committed but never made it onto a page row leaks quota until manually reclaimed. 1. useWebClipperDialogSubmit had two early-return paths between the thumbnail commit and the hand-off to onClipped — the `submitGeneration !== submitGenerationRef.current` guard (dialog re-opened mid-submit) and the `if (tiptapContent)` skip when getTiptapContent returned null. Both exited without DELETE-ing the committed object. Added a `commitHandedOff` flag and a finally branch that calls deleteCommittedThumbnail when the commit was made but onClipped was never reached. The `onClipped` path is owned by the callee (handleWebClipped) for rollback, so we gate on the flag to avoid a duplicate DELETE. 2. deleteCommittedThumbnail only logged when fetch threw. A 500 / 429 / 403 from DELETE /api/thumbnail/serve/:id resolved as `response.ok === false` and was silently treated as success, so a failed rollback left an orphan with no signal. Now we check `response.ok` and warn on unexpected statuses while still treating 401 (signed out mid-rollback) and 404 (already gone) as expected no-ops. Tests added: - non-OK (500) rollback response is warned with status + objectId - 401/404 are silent no-ops --------- Co-authored-by: Claude <noreply@anthropic.com>
* feat(api): add default note ("マイノート") foundation
各ユーザーに 1 件のデフォルトノート (`<users.name>のノート`) を持たせる
土台を追加する。これは「ホーム廃止 → /notes/me 着地」UX 再設計の Phase 1a で、
既存の個人ページや note_pages テーブルには触れない(後続の Phase 1b で
個人ページをデフォルトノートに移行し、note_pages を廃止する予定)。
Adds the foundation for the default-note ("マイノート") model where every user
owns exactly one note titled `<users.name>のノート`. This is Phase 1a of the
"/home → /notes/me" UX refactor; existing personal pages and the note_pages
link table are intentionally left untouched and will be migrated in a
follow-up.
主な変更点 / Highlights:
- `notes.is_default` カラムを追加し、partial unique index で 1 ユーザー
1 件を担保 (`server/api/drizzle/0020_add_default_note.sql`)
- 既存ユーザー全員にデフォルトノートをバックフィル
- `defaultNoteService` で `ensureDefaultNote` / `getDefaultNoteIdOrNull` /
`formatDefaultNoteTitle` を提供(idempotent、並行安全)
- `GET /api/notes/me` 新設:呼び出し元のデフォルトノートを返す(未作成なら
自動作成)
- `DELETE /api/notes/:id` でデフォルトノートを 400 で拒否
- `NoteApiFields.is_default` を露出してフロントから識別可能に
- `welcomePageService.insertWelcomePage` がウェルカムページをデフォルト
ノートに所属させるよう更新(新規ユーザー向け)
- 単体テストとルートテストを追加(defaultNoteService、/api/notes/me、
デフォルトノート削除拒否)
https://claude.ai/code/session_01QjBBYn2DSouRkUUkLPZXxx
* fix(api): address PR review feedback for default note PR 1a
レビューコメント対応 / Address review comments on PR #821:
- Codex P1: ウェルカムページの作成挙動を旧来どおり個人ページ
(`note_id = NULL`) に戻す。`note_pages` 行を作らずに `note_id` だけ
埋めると、`GET /api/notes/:id/pages` と note search が `note_pages`
経由でページを引くため、ウェルカムページがデフォルトノート画面でも
`/home` でも見えなくなる回帰になる。PR 1b で個人ページ全体をデフォルト
ノートへ移行する際に一緒に扱う。
Revert welcome page assignment to default note. Setting `note_id` without
also creating a `note_pages` row hid the page from `GET /api/notes/:id/pages`,
note search, and the legacy `/home` listing. PR 1b will migrate personal
pages — including welcome pages — into the default note in one step.
- Gemini medium: `ensureDefaultNote` / `getDefaultNoteIdOrNull` を
`Note` 全体を返す形にリファクタ(`getDefaultNoteOrNull`)。これにより
`GET /api/notes/me` の重複 SELECT を削除して 1 クエリ少なくする。
Refactor the service to return the full `Note` (rename to
`getDefaultNoteOrNull`). `GET /api/notes/me` no longer needs a follow-up
SELECT after `ensureDefaultNote`, dropping one query per request.
- 関連テスト (me.test.ts) を新しいクエリ計画に合わせて更新。
Updated `me.test.ts` to match the new query plan.
https://claude.ai/code/session_01QjBBYn2DSouRkUUkLPZXxx
* docs(api): fix stale migration number in 0022 header comments
CodeRabbit review feedback: マイグレーションファイルを 0020 → 0022 に
リネームした際、本文のヘッダコメント (1行目と7行目) の "0020:" を
更新し漏れていた。ファイル名と journal タグに合わせて "0022:" に修正。
Address CodeRabbit review note: the migration was renamed 0020 → 0022 to
avoid colliding with develop's `0020_seed_thumbnail_tier_quotas`, but the
header comment lines kept the old "0020:" prefix. Sync them with the
filename and journal tag (`0022_add_default_note`).
https://claude.ai/code/session_01QjBBYn2DSouRkUUkLPZXxx
* test(api): address CodeRabbit nitpicks for default note PR 1a
3 件の CodeRabbit nitpick (Quick win / Optional) に対応:
1. `defaultNoteService.ts`: `onConflictDoNothing()` に明示的な target +
where を指定し、無関係なユニーク制約衝突を黙って飲み込まないように。
partial unique index `idx_notes_unique_default_per_owner` の述語と揃える。
2. `defaultNoteService.test.ts`: タイトル整形のみのテストに加え、
`ensureDefaultNote` と `getDefaultNoteOrNull` の冪等性 / 並行ハンドリング /
エラー経路のテストを追加(3 → 10 件)。
3. `crud.test.ts`: デフォルトノート削除拒否テストで、`update` チェーンが
走っていないことも併せて検証。「拒否したのに更新は適用された」を防ぐ
契約として固定。
Address three CodeRabbit nitpick suggestions:
1. `defaultNoteService.ts`: explicitly target the partial unique index in
`onConflictDoNothing()` so unrelated unique-constraint violations are not
silently swallowed.
2. `defaultNoteService.test.ts`: extend the suite from formatter-only to
cover idempotency, race-recovery, 404, and 500 paths of
`ensureDefaultNote` / `getDefaultNoteOrNull` (3 → 10 tests).
3. `crud.test.ts`: tighten the default-note delete test to assert that
no `update` chain fires when deletion is rejected.
https://claude.ai/code/session_01QjBBYn2DSouRkUUkLPZXxx
---------
Co-authored-by: Claude <noreply@anthropic.com>
Bumps the cargo group with 1 update in the /src-tauri directory: [tauri](https://github.com/tauri-apps/tauri). Updates `tauri` from 2.11.0 to 2.11.1 - [Release notes](https://github.com/tauri-apps/tauri/releases) - [Commits](tauri-apps/tauri@tauri-v2.11.0...tauri-v2.11.1) --- updated-dependencies: - dependency-name: tauri dependency-version: 2.11.1 dependency-type: direct:production dependency-group: cargo ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
…es (#823) (#831) * feat(api): migrate personal pages into default note and drop note_pages (#823) PR 1b: backfill all `pages.note_id IS NULL` rows into each owner's default note, promote `pages.note_id` to NOT NULL, and drop the `note_pages` junction table. After this PR every page belongs to exactly one note via `pages.note_id`, so authorization, search, and sync all flow through the note model uniformly. API: - `GET /api/pages` returns 410 Gone with `Deprecation: true` header. - `POST /api/pages` requires `note_id`; falls back to `ensureDefaultNote`. - `POST /api/notes/:noteId/pages` accepts only the title path; `page_id` linking is rejected with 400. - `PUT /api/notes/:noteId/pages` is now a noop (order is `updated_at DESC`). - `copy-from-personal` / `copy-to-personal` routes are removed. - Note search / global search drop the `note_pages` join and use `pages.note_id` directly. - `assertPageViewAccess` / `assertPageEditAccess` resolve roles solely through `getNoteRole` on `pages.note_id` (no personal-page branch). - `welcomePageService`, `clipAndCreate`, `wikiSchema`, `indexBuilder`, `syncPages`, etc. all set `noteId` from `ensureDefaultNote`. Schema / migration: - New SQL migration `0023_migrate_personal_pages_drop_note_pages.sql` with safety inserts, default-note backfill, NOT NULL promotion, and `DROP TABLE note_pages` (idempotent guards). - `schema/notes.ts` removes `notePages`; `schema/pages.ts` makes `noteId` non-null; `schema/relations.ts` drops `notePagesRelations`. Tests: - Rewrites for `pageAccessService`, route tests for pages / notes pages / search / pageSnapshots / syncPages / media to match the new contract (note-role chains, `HeadObjectCommand` mock, `ensureDefaultNote` mock, removal of `sort_order` / `added_by` fields). Refs: #823 (PR 1b), #821 (PR 1a), #824 (PR 2 meta). Co-authored-by: Cursor <cursoragent@cursor.com> * fix: address PR #831 review (list pages shim, POST note guard, search prefetch) - Restore GET /api/pages as a deprecation shim (200 + Deprecation header) using pages.note_id and the same own/shared access model as search, so MCP listPages keeps working. - POST /api/pages: require getNoteRole + canEdit when the client supplies note_id. - GET /api/search scope=own: resolve default note via getDefaultNoteOrNull instead of a SQL subquery; early-empty when missing. - Drop redundant Array.isArray guards on Drizzle select results in helpers, pageAccessService, pageSnapshots, and syncPages. Co-authored-by: Cursor <cursoragent@cursor.com> * fix: address PR #831 CodeRabbit follow-up (migration idempotency, docs, comments) - 0023 migration: append ON CONFLICT DO NOTHING targeting the partial unique index on owner_id for default-note inserts. - notes/search.ts: clarify JSDoc that authRequired applies; guest role is for authenticated callers with note access. - media.test.ts: restore JP/EN paired comments on HeadObject stub and beforeEach. Co-authored-by: Cursor <cursoragent@cursor.com> --------- Co-authored-by: Cursor <cursoragent@cursor.com>
#832) * feat(frontend): retire /home in favor of /notes/me landing (#825) PR 2a derived from #824. Replaces the legacy `/home` route with a single-step redirect into `/notes/me`, a thin landing that resolves the caller's default note via `GET /api/notes/me` and forwards to `/notes/:noteId`. The default note is treated as a regular note, so `NoteView` is reused without `is_default` branching (A plan). - Add `apiClient.getMyNote()` and the `MyNoteResponse` type. - Add `useMyNote` React Query hook keyed per user. - Add `NoteMeRedirect` page (skeleton while resolving, inline error on failure) and wire `/notes/me` behind `ProtectedRoute`. Replace the `/home` route with `<Navigate to="/notes/me" replace />` so existing bookmarks still resolve. - Swap the primary nav `Home` entry for `My Note`: path `/notes/me`, i18n key `nav.myNote` (ja: マイノート / en: My Note), drop the unused `nav.home` translation. Active match stays exact so unrelated `/notes/:noteId` views do not light up the My Note tab. - Update post-auth landings (`Landing`, `Onboarding`, `AuthCallback`) and the OAuth allowlist default to `/notes/me`. `/home` stays in the allowlist so legacy returnTo values still flow through the redirect. - Tighten `Page.noteId` / `PageSummary.noteId` and the API row types (`SyncPageItem.note_id`, `GetNoteResponse.pages[].note_id`, `SearchSharedResponse.results[].note_id`) to non-null `string` to match the post-#823 API contract; drop the `?? null` mapper fallback. Tests: update `navigationItems` / Header `NavigationMenu` / `BottomNav` suites for the new entry; add `NoteMeRedirect` skeleton/redirect/error coverage and an `apiClient.getMyNote` URL/method/shape contract test; update `AuthCallback` fallbacks to assert `/notes/me`. Out of scope (per the issue, deferred to follow-ups #826/#828): removing `Home.tsx` / `HomePageCount` / `usePagesSummary` / clipUrl handoff, and purging `noteId: null` from the IndexedDB / zustand local cache. https://claude.ai/code/session_01S7kfxhyvt9pNBqaWsWGjX7 * chore(knip): ignore Home.tsx and friends pending follow-up migration Removing the `/home` route in #832 orphans `src/pages/Home.tsx` and its direct dependencies (`PageGrid`, `PageCard`, `EmptyState`, `useSeedData`) because nothing else imports them. Issue #825 explicitly defers their removal to follow-up PRs (#826 ports clipUrl handoff, #828 generalizes NotePageCount), so deleting them in this PR would conflict with that plan. Add them to the workspace `ignore` list so the Knip CI gate passes without losing the unused-code signal for genuinely new dead code. The follow-up PRs that delete each file should also remove the matching entry here. https://claude.ai/code/session_01S7kfxhyvt9pNBqaWsWGjX7 --------- Co-authored-by: Claude <noreply@anthropic.com>
…833) * feat(frontend): port clipUrl handoff to /notes/me (issue #826) Move the Chrome-extension `clipUrl` hand-off from the retired `/home` page to `/notes/me`, completing the routing migration started in #825 (#832). - `LegacyHomeRedirect` preserves search and hash on `/home → /notes/me`, so existing extension installs that hit `/home?clipUrl=...` keep working until the extension is updated separately in #829. - `NoteMeRedirect` now validates `?clipUrl=<URL>` with `isClipUrlAllowed`, forwards the survivor to `/notes/:noteId?clipUrl=...`, drops invalid values, and bounces unfinished setup-wizard users to `/onboarding` before resolving the default note (ported from the old `Home.tsx`). - `ProtectedRoute` stamps the original `pathname + search + hash` into a `?returnTo=` query when redirecting guests to `/sign-in`, so the unauthenticated `clipUrl` flow round-trips through the OAuth callback back into `/notes/me?clipUrl=...`. The existing `getSafeReturnTarget` allowlist on `AuthCallback` keeps this safe against open-redirect payloads. - `NoteView` reads and validates `clipUrl`, passes it to `FloatingActionButton.initialClipUrl`, and strips only the `clipUrl` query (preserving the rest of the search and hash) when the dialog closes — so a refresh doesn't relaunch it. - Drop the orphan `Home.tsx` and `useSeedData.ts`; `useOnboarding` ported into `NoteMeRedirect`. - Tests cover clipUrl forwarding, URL-policy rejection, onboarding redirect, and the post-close URL cleanup; the e2e spec now exercises both `/notes/me?clipUrl=...` and the legacy `/home?clipUrl=...` path. https://claude.ai/code/session_0154o6RbuRDiPWkd1rmqemnt * fix: address PR #833 review comments Co-authored-by: Cursor <cursoragent@cursor.com> * fix: address PR #833 review comments Co-authored-by: Cursor <cursoragent@cursor.com> --------- Co-authored-by: Claude <noreply@anthropic.com> Co-authored-by: Cursor <cursoragent@cursor.com>
* feat(frontend): add NoteSwitcher dropdown to AppLayout (#827) - New NoteSwitcher dropdown lives in the desktop Header next to the logo and in MobileHeader before the search bar so users can hop between notes from any AppLayout-wrapped page (#825 default-note landing makes this the dominant nav pattern). - The default note ("マイノート") is pinned to the top with a `Default` badge; the rest of the rows are sorted by `updatedAt` descending and capped at 50 (overflow funnels through the "see all" footer link). - Active note is detected from the URL via `matchPath`, and reserved segments (`/notes/me`, `/notes/discover`, `/notes/official-guide`) are excluded from the active-note resolver. - Footer shortcuts: "Create new note" deep-links to `/notes?new=1`, which the Notes page consumes via a new `useNewNoteDeepLink` hook that opens the existing create dialog and strips the param so refreshes do not reopen it. "See all notes" goes to `/notes`. - Existing Header / MobileHeader tests updated to mock NoteSwitcher (it pulls in auth + react-query and would otherwise break their lightweight test setup). Closes #827. * fix(notes): react to ?new=1 deep-link after Notes is already mounted Per gemini / codex review on PR #834: when a signed-in user is already on /notes and clicks "Create new note" in the header NoteSwitcher, React Router updates only `location.search` and keeps `Notes` mounted. The old `useState(() => wantsCreate)` initializer only ran on first mount, so the dialog never opened in this flow. `useNewNoteDeepLink` now takes an `onOpen` callback and watches `location.search` for transitions, firing `onOpen` and stripping `?new=1` inside the same effect so the open + replace happen atomically. The callback is held in a ref updated via a post-render effect so callers can pass an inline function without re-triggering the URL watch on every render. Also adds focused tests for the on-mount and after-mount paths to lock in the regression. * test(NoteSwitcher): exercise click-to-navigate, type mocks, bilingual comments Addresses CodeRabbit review feedback on PR #834: - "navigates when selected" now actually clicks the row and asserts the router location updated, so the test catches breakage in the `Link` wiring beyond just the static `href`. - Mocks for `useNotes`, `useMyNote`, and `useAuth` are now typed via `Mock<...>` so interface drift in the real hooks surfaces in the test harness instead of being silently masked. - Filled in Japanese counterparts for the remaining English-only inline comments to match the repo's bilingual-comment policy. --------- Co-authored-by: Claude <noreply@anthropic.com>
…) (#835) `HomePageCount` was wired to the personal-page summary path that no longer fits the post-#823 default-note model. Replace it with `NotePageCount`, which takes a `noteId` prop, reads from the existing `useNotePages` React Query cache (no extra fetch), and renders the same desktop-only badge above the FAB on every NoteView (including `/notes/me`). `HomePageCount` had no remaining production callers, so it is removed along with the now-orphaned `home` i18n namespace. The `notes.totalPages` and `notes.pageCountLabel` keys take over the badge copy, and the NoteView floatingAction wrapper is restructured so the count stays visible for read-only viewers even when the FAB is hidden. `HomePageCount` は #823 後の default-note モデルに合わなくなった個人ページ 集計に依存していたため、`noteId` を prop に受ける `NotePageCount` に汎化 した。`useNotePages` の React Query キャッシュを共有するため追加 fetch は 発生せず、`/notes/me` を含む任意のノートで FAB 上にバッジが表示される。 プロダクション参照のなかった `HomePageCount` と `home` i18n namespace は 削除し、コピーは `notes.totalPages` / `notes.pageCountLabel` に集約した。 Co-authored-by: Claude <noreply@anthropic.com>
* chore(extension): switch clipUrl hand-off to /notes/me (#829) Chrome 拡張のクリップ送信先を `/home?clipUrl=...` から `/notes/me?clipUrl=...` に切り替える。フロント側の `/home` 互換 redirect は 古い拡張ユーザー保護のため残す。manifest version を 1.0.1 に bump。 Switch the Chrome extension clip hand-off from `/home?clipUrl=...` to the canonical `/notes/me?clipUrl=...` endpoint introduced in #826/#833. The frontend `/home` compatibility redirect stays in place for users on older extension builds. Manifest version bumped to 1.0.1 for store review. * chore(extension): address review feedback (#836) レビュー指摘事項への対応: - SemVer に従い manifest version を 1.0.1 → 1.1.0(新機能)に変更。 - popup.js から到達不能な `openZediWithClipUrl` を削除。popup の クリップフローは `clipAndCreate` のエラーをメッセージで通知する 設計のため、フォールバック関数は使用されていなかった。 - クリップ先パスを `extension/constants.js`(`self` 経由で popup と service worker の両 context から参照可能)に切り出し、background.js から参照するよう変更。 Address PR review: - Bump manifest to 1.1.0 per SemVer (this is a feature change, not a fix). - Remove `openZediWithClipUrl` from popup.js — it was unreachable. The popup surfaces clip failures as inline messages rather than falling back to a tab open, so the helper had no callers. - Extract the clip path into `extension/constants.js`, loaded via `<script>` from popup.html and `importScripts()` from background.js (a single file works in both contexts via `self`). --------- Co-authored-by: Claude <noreply@anthropic.com>
…830) (#837) Surfaces a warning-only AlertDialog before switching a default note (`is_default = true`) from a non-shareable visibility to `public` / `unlisted`, so users do not accidentally publish private pages they had saved into their personal default note. The dialog chains with the existing `public + any_logged_in` warning when both apply (Plan A: two sequential dialogs). - Adds `isDefault` to the `Note` domain type and propagates `is_default` through `apiNoteToNote` / API response types. - Adds `shouldConfirmDefaultNotePublicSave` and `isShareableVisibility` helpers in `noteSharingRisk` with focused unit tests. - Adds `DefaultNotePublicWarningDialog` and a hook test suite covering the no-warn / single-warn / chained-warn paths and cancel behavior. - Adds `notes.defaultNotePublicWarning{Title,Description}` i18n keys in ja/en. The note URL is unchanged (`/notes/:id`) per the issue. https://claude.ai/code/session_011eYyc6ydz1X6zt4jrzcHfj Co-authored-by: Claude <noreply@anthropic.com>
* feat(frontend): unify note page list with /home grid
Generalize PageGrid/PageCard/EmptyState so the note detail screen shares
the same layout the legacy /home view used: container-width-driven
columns (2-6), skeleton placeholders, the Sparkles empty-state CTA, and
right-click context-menu / drag-and-drop affordances on each card. The
note view now mounts <PageGrid noteId={note.id} canEdit={canEdit} />
instead of the bespoke NoteViewMainContent/NoteViewPageGrid pair, which
have been removed. Inside a note context PageCard navigates to
/notes/:noteId/:pageId and routes deletes through the note-scoped
DELETE so the server's canEdit guard applies; the duplicate menu item
is hidden in note context for now (the personal-page copy flow does not
fit there) and is tracked separately.
ノート詳細のページ一覧を旧 /home (`PageGrid` + `PageCard`) と同じ見た目
に統一する。コンテナ幅に応じた 2〜6 列のレスポンシブグリッド、ロード中の
スケルトン、Sparkles の空状態 CTA、カード右クリックメニューと AI チャット
へのドラッグ&ドロップを共通化。`NoteView` は `<PageGrid noteId
canEdit>` を呼ぶだけになり、`NoteViewMainContent` / `NoteViewPageGrid`
は削除した。`PageCard` はノート文脈で `/notes/:noteId/:pageId` に遷移し、
削除はノートスコープの DELETE を経由してサーバ側 `canEdit` ガードを通す。
ノート文脈での複製はスコープ外として一旦非表示にする。
https://claude.ai/code/session_01QjBBYn2DSouRkUUkLPZXxx
* chore: remove unused NotePageCard after grid unification
PageGrid/PageCard now serve the note context, so the bespoke
NotePageCard component left behind by #824 is unused. Knip flags it as
the only unused-files error, blocking CI.
#824 のグリッド統一により `NotePageCard` は参照されなくなったため削除する。
Knip CI が unused file として検出して落ちていた失敗の修正。
https://claude.ai/code/session_01QjBBYn2DSouRkUUkLPZXxx
* fix(frontend): restore per-page delete check and mobile delete affordance
Two follow-ups from the unified PageGrid review:
- PageGrid now accepts an optional `canDeletePage(page)` callback. When
provided it gates the per-card delete UI per row instead of falling
back to the coarse `canEdit` flag, restoring the previous
NoteViewPageGrid rule (owners may delete any page, editors only the
ones they added). NoteView wires this through `access.canDeletePage`.
- PageCard surfaces a permanent trash overlay button on mobile when the
card is in a note context with delete permission. Touch users can't
open the desktop ContextMenu, so without this they had no way to
remove pages from a note after the grid unification. Desktop keeps
the ContextMenu as the primary action to avoid double UI.
統一 PageGrid のレビュー指摘 2 点を反映する:
- `PageGrid` に `canDeletePage(page)` コールバック prop を追加。渡された
場合は `canEdit` の一律判定ではなくページごとに削除可否を判定し、
「オーナーは全削除可・エディターは自分が追加したページのみ」という旧
`NoteViewPageGrid` の挙動を復活させる。`NoteView` 側で
`access.canDeletePage(addedByUserId)` を経由するよう接続。
- モバイルではノート文脈で削除権限を持つカードに常設のゴミ箱ボタンを
表示する。タッチデバイスはデスクトップ ContextMenu を呼べないため、
グリッド統一以降ノート配下のページを削除できない状態だった。デスク
トップは ContextMenu を主動線として維持する。
https://claude.ai/code/session_01QjBBYn2DSouRkUUkLPZXxx
* fix(frontend): keep delete dialog open while mutation is pending
Address CodeRabbit review on PR #838.
- PageCard: Radix's `AlertDialogAction` auto-closes the dialog on click,
which dismissed the dialog before the delete mutation resolved and
hid the `isDeletePending` UI. Switch to a controlled `open` /
`onOpenChange` pair that ignores close attempts while the mutation is
in flight, and `preventDefault` on the action button so the dialog's
close happens explicitly inside the mutation's `onSuccess`/`onError`
callbacks. Cancel is also disabled while pending.
- NoteView.test: surface the `canDeletePage` prop on the `PageGrid`
mock and assert it is wired as a function so a future regression in
the per-page delete permission check is caught by the test suite.
CodeRabbit のレビュー指摘 2 点を反映する:
- `PageCard` の削除ダイアログ: Radix `AlertDialogAction` がクリック時に
ダイアログを自動で閉じてしまい、`isDeletePending` の UI が見えない
まま終わっていた。`open` / `onOpenChange` を制御し、ミューテーション
実行中は閉じる操作を無視する。アクションボタン側でも
`preventDefault` して、ミューテーションの `onSuccess` / `onError` で
明示的に閉じるよう統一。Cancel も pending 中は無効化する。
- `NoteView.test`: `PageGrid` モックで `canDeletePage` を受け取り、
`data-can-delete-fn` 属性に関数かどうかを露出。アサーションを追加し
ページごとの削除権限が wiring されていることを保証する。
https://claude.ai/code/session_01QjBBYn2DSouRkUUkLPZXxx
---------
Co-authored-by: Claude <noreply@anthropic.com>
… guard (#839) * feat(api): refuse thumbnail deletion when a live page references it (#820) Add a referential guard inside `deleteThumbnailObject` that aborts the delete when any non-deleted `pages` row still points at the thumbnail via `thumbnail_object_id`. This makes the client-side rollback in `useFloatingActionButtonHandlers` / `useWebClipperDialogSubmit` fully safe: a phantom DELETE fired after a successful page commit (e.g. when the response was lost in transit) now leaves the live page's thumbnail intact instead of stripping it. The service now returns a discriminated outcome (`deleted` / `not_found` / `referenced`) and the DELETE /api/thumbnail/serve/:id route maps `referenced` to 409. Ownership is verified before the reference check so unauthorized callers always see 404 — never 409 — and cannot probe other users' thumbnail state. The client rollback helper now treats 409 as an expected no-op (the guard preserved the blob because the page commit actually succeeded), so phantom rollbacks no longer log spurious warnings. * fix(api): scope thumbnail referential guard to thumbnail owner (#820) Address review feedback on PR #839. - Scope the live-reference SELECT in `deleteThumbnailObject` to `pages.owner_id = userId`. Without this, a third party can call `POST /api/pages` with the victim's `thumbnail_object_id` (the route currently accepts the field unverified) to plant a phantom referrer; the victim's `DELETE /api/thumbnail/serve/:id` would then return 409 indefinitely, blocking GC and burning the victim's quota. Closing this end-to-end requires `POST /api/pages` to validate thumbnail ownership too, which is tracked as a follow-up; the owner-scoped guard caps the blast radius today. - Document the residual TOCTOU race: a plain transaction under READ COMMITTED does not serialize against concurrent INSERTs from other connections, so a page row with the same `thumbnail_object_id` could still slip in between our referrer SELECT and our DELETE. The race does not arise on the typical single-user "commit thumbnail → create page → maybe rollback" flow because those steps are serialized client-side; a proper fix (FK ON DELETE RESTRICT or coordinated `FOR UPDATE`/`FOR SHARE` locks across the page-insert path) is out of scope for this issue and tracked as a follow-up. - Fold the new 409 contract into the exported TSDoc on `deleteCommittedThumbnail` so the function-level spec matches the inline behavior. --------- Co-authored-by: Claude <noreply@anthropic.com>
Bumps [actions/create-github-app-token](https://github.com/actions/create-github-app-token) from 2 to 3. - [Release notes](https://github.com/actions/create-github-app-token/releases) - [Commits](actions/create-github-app-token@v2...v3) --- updated-dependencies: - dependency-name: actions/create-github-app-token dependency-version: '3' dependency-type: direct:production update-type: version-update:semver-major ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
…r-and-patch group (#841) * chore(deps): bump @anthropic-ai/sdk in the minor-and-patch group Bumps the minor-and-patch group with 1 update: [@anthropic-ai/sdk](https://github.com/anthropics/anthropic-sdk-typescript). Updates `@anthropic-ai/sdk` from 0.92.0 to 0.95.1 - [Release notes](https://github.com/anthropics/anthropic-sdk-typescript/releases) - [Changelog](https://github.com/anthropics/anthropic-sdk-typescript/blob/main/CHANGELOG.md) - [Commits](anthropics/anthropic-sdk-typescript@sdk-v0.92.0...sdk-v0.95.1) --- updated-dependencies: - dependency-name: "@anthropic-ai/sdk" dependency-version: 0.95.1 dependency-type: direct:production update-type: version-update:semver-minor dependency-group: minor-and-patch ... Signed-off-by: dependabot[bot] <support@github.com> * chore(deps): sync bun.lock with package.json changes --------- Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
* chore(deps-dev): bump @commitlint/cli from 20.5.3 to 21.0.0 Bumps [@commitlint/cli](https://github.com/conventional-changelog/commitlint/tree/HEAD/@commitlint/cli) from 20.5.3 to 21.0.0. - [Release notes](https://github.com/conventional-changelog/commitlint/releases) - [Changelog](https://github.com/conventional-changelog/commitlint/blob/master/@commitlint/cli/CHANGELOG.md) - [Commits](https://github.com/conventional-changelog/commitlint/commits/v21.0.0/@commitlint/cli) --- updated-dependencies: - dependency-name: "@commitlint/cli" dependency-version: 21.0.0 dependency-type: direct:development update-type: version-update:semver-major ... Signed-off-by: dependabot[bot] <support@github.com> * chore(deps): sync bun.lock with package.json changes --------- Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
* chore(deps): bump react-day-picker from 9.14.0 to 10.0.0 Bumps [react-day-picker](https://github.com/gpbl/react-day-picker/tree/HEAD/packages/react-day-picker) from 9.14.0 to 10.0.0. - [Release notes](https://github.com/gpbl/react-day-picker/releases) - [Changelog](https://github.com/gpbl/react-day-picker/blob/main/packages/react-day-picker/CHANGELOG.md) - [Commits](https://github.com/gpbl/react-day-picker/commits/v10.0.0/packages/react-day-picker) --- updated-dependencies: - dependency-name: react-day-picker dependency-version: 10.0.0 dependency-type: direct:production update-type: version-update:semver-major ... Signed-off-by: dependabot[bot] <support@github.com> * chore(deps): sync bun.lock with package.json changes --------- Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
…conventional 21.0.0 (#854) Combine two conflicting Dependabot PRs into a single resolution branch: - @google/genai: 1.34.0 -> 2.0.1 (PR #844) Breaking changes are confined to the Interactions API (interaction.created / interaction.completed SSE rename, response_format deprecation). The codebase uses only GoogleGenAI.generateContent / generateContentStream, so the bump is non-breaking here. - @commitlint/config-conventional: 20.5.3 -> 21.0.0 (PR #845) Follows the @commitlint/cli 21.0.0 bump already in develop (#842). Both packages now require Node v22+, consistent with the CLI. Regenerated bun.lock; lint, typecheck, and full test suite (2,587 tests) all pass. https://claude.ai/code/session_01C5SK65rRcMrtMmehEqv7LB Co-authored-by: Claude <noreply@anthropic.com>
) - Replace header NoteSwitcher with NoteTitleSwitcher on note flows - Redesign NoteSettings as sidebar + nested routes and sections - Remove ShareModal; fold sharing into settings (members, links, domains) - Add NoteShareUrlCopyButton for public/unlisted; simplify NoteView actions - Remove NotePageCount; header logo links to /notes/me - Redirect legacy /notes/:id/members to settings/members Co-authored-by: Cursor <cursoragent@cursor.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c1417f99b2
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| const existing = await getApiErrorBySentryIssueId(db, summary.sentryIssueId); | ||
| const isNew = existing === null; | ||
|
|
There was a problem hiding this comment.
Make new-issue detection atomic before dispatching analysis
isNew is computed from a separate pre-upsert read, so concurrent webhook deliveries for the same brand-new sentry_issue_id can both observe existing === null and both fire dispatchAnalyzeError. That creates duplicate repository_dispatch events for one logical issue, which means redundant Claude runs and duplicate downstream issue/comment churn. Derive “newness” from the upsert result itself (for example, only dispatch when the returned row indicates first insertion) or wrap the read+write decision in an atomic DB flow.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
【対応見送り】sentry.ts 内で並行 webhook による二重 dispatch を許容するトレードオフと、ワークフロー側での冪等前提が既にコメントされています。upsert と検知を原子的にする変更は別設計 PR とします。
[Won't fix in PR #908] The handler documents the race and relies on workflow-side idempotency; making new-issue detection atomic would be a separate design PR.
There was a problem hiding this comment.
Actionable comments posted: 17
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
server/api/src/routes/notes/members.ts (1)
267-275:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winEmit
note.permission_changedonly after an actual delete mutation.At Line 267, the delete query does not verify an active-member match, and at Line 274 the event is always published. This can emit false permission-change notifications (and unnecessary client refetches) on no-op deletes.
Suggested fix
- await db + const deleted = await db .update(noteMembers) .set({ isDeleted: true, updatedAt: new Date() }) - .where(and(eq(noteMembers.noteId, noteId), eq(noteMembers.memberEmail, memberEmail))); + .where( + and( + eq(noteMembers.noteId, noteId), + eq(noteMembers.memberEmail, memberEmail), + eq(noteMembers.isDeleted, false), + ), + ) + .returning({ noteId: noteMembers.noteId }); + + if (deleted.length === 0) { + throw new HTTPException(404, { message: "Member not found" }); + } // Issue `#860` Phase 4: メンバー削除も `getNoteRole` の挙動を変えるため通知。 // Issue `#860` Phase 4: removing a member affects `getNoteRole`; notify too. publishNoteEvent({ type: "note.permission_changed", note_id: noteId });🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@server/api/src/routes/notes/members.ts` around lines 267 - 275, The update currently marks noteMembers as deleted and always calls publishNoteEvent("note.permission_changed") even when no active member was matched; modify the mutation to target only active members (include eq(noteMembers.isDeleted, false) or equivalent) and capture the update result/affected row count from the db.update call (the update on noteMembers that sets isDeleted and updatedAt). Only call publishNoteEvent({ type: "note.permission_changed", note_id: noteId }) when the update affected at least one row; leave the rest of the logic (setting isDeleted and updatedAt) unchanged.
🧹 Nitpick comments (3)
admin/src/pages/errors/useApiErrors.test.ts (1)
225-239: ⚡ Quick winAdd a case where an existing visible row becomes filter-mismatched after SSE update
Current coverage only checks ignoring a brand-new mismatched row. Please add a case where an already-listed row receives an
updatethat changes status/severity and should be removed from the filtered list.✅ Suggested additional test
+ it("removes an existing row when an SSE update makes it fail the active filter", async () => { + const { result } = renderHook(() => + useApiErrors({ status: "open", intervalMs: 0 }), + ); + await waitFor(() => expect(result.current.loading).toBe(false)); + + const base = getBaseRow(); + expect(base.status).toBe("open"); + + act(() => { + getInstance().dispatch("ready", ""); + getInstance().dispatch("update", { ...base, status: "resolved" }); + }); + + expect(result.current.errors.find((r) => r.id === base.id)).toBeUndefined(); + });🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@admin/src/pages/errors/useApiErrors.test.ts` around lines 225 - 239, Add a new test to cover the case where an already-visible error becomes filter-mismatched after an SSE update: renderHook with useApiErrors({ status: "open", severity: "high", intervalMs: 0 }), seed an initial matching item via makePushed({ status: "open", severity: "high" }) and dispatch it with getInstance().dispatch("ready", "") and dispatch("update", initialItem), assert it appears in result.current.errors, then dispatch an update that changes its fields to mismatched values (e.g., dispatch("update", { ...initialItem, status: "resolved", severity: "low" })) and assert the item is removed from result.current.errors and result.current.total updates accordingly; use waitFor/act around async state assertions as in the existing test.scripts/gen-pdf-fixture.ts (1)
54-55: ⚡ Quick winUse byte length for PDF stream
/Lengthfields to ensure compatibility with non-ASCII content.The
/Lengthentry must specify stream byte size, not character count. For ASCII-only content,string.lengthcurrently works, but if the fixture ever includes non-ASCII characters, the mismatch will cause parser failures. UseTextEncoder().encode().byteLengthto guarantee correctness.♻️ Proposed fix
+ const page1Length = new TextEncoder().encode(page1Content).byteLength; + const page2Length = new TextEncoder().encode(page2Content).byteLength; // 各 obj を文字列の配列として生成する。xref で参照するバイトオフセットは // 後段で連結時に計測する。 // Each object's body. The xref byte offsets are computed below as we // concatenate the actual file bytes. const objects: string[] = [ // 1: Catalog "<< /Type /Catalog /Pages 2 0 R >>", // 2: Pages "<< /Type /Pages /Kids [3 0 R 5 0 R] /Count 2 >>", // 3: Page 1 "<< /Type /Page /Parent 2 0 R /MediaBox [0 0 612 792] " + "/Contents 4 0 R /Resources << /Font << /F1 7 0 R >> >> >>", // 4: Page 1 content stream - `<< /Length ${page1Content.length} >>\nstream\n${page1Content}endstream`, + `<< /Length ${page1Length} >>\nstream\n${page1Content}endstream`, // 5: Page 2 "<< /Type /Page /Parent 2 0 R /MediaBox [0 0 612 792] " + "/Contents 6 0 R /Resources << /Font << /F1 7 0 R >> >> >>", // 6: Page 2 content stream - `<< /Length ${page2Content.length} >>\nstream\n${page2Content}endstream`, + `<< /Length ${page2Length} >>\nstream\n${page2Content}endstream`, // 7: Font "<< /Type /Font /Subtype /Type1 /BaseFont /Helvetica >>", ];🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@scripts/gen-pdf-fixture.ts` around lines 54 - 55, The `/Length` value uses character count (page1Content.length) which breaks for non-ASCII bytes; compute the stream byte length instead by encoding the content with TextEncoder and using its byte length when writing `/Length` (e.g., replace references to page1Content.length with the byte length from TextEncoder().encode(page1Content)); apply the same change for any other stream content variables (e.g., page2Content) so all `/Length` fields reflect actual byte sizes.server/api/src/routes/internal.ts (1)
56-59: 💤 Low valueConsider timing-safe comparison for secret validation.
The direct string comparison
provided !== expectedis vulnerable to timing attacks. While this is an internal-only endpoint, usingcrypto.timingSafeEqualwould follow security best practices.Since this is internal service-to-service communication behind a firewall, the risk is low.
Proposed fix
+import { timingSafeEqual } from "node:crypto"; + +function safeEqual(a: string, b: string): boolean { + const bufA = Buffer.from(a); + const bufB = Buffer.from(b); + if (bufA.length !== bufB.length) return false; + return timingSafeEqual(bufA, bufB); +} + function assertInternalAuth(c: { req: { header: (name: string) => string | undefined } }): void { const expected = process.env.BETTER_AUTH_SECRET?.trim(); // ... const provided = c.req.header("x-internal-secret")?.trim(); - if (!provided || provided !== expected) { + if (!provided || !safeEqual(provided, expected)) { throw new HTTPException(401, { message: "Unauthorized" }); } }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@server/api/src/routes/internal.ts` around lines 56 - 59, Replace the direct string comparison of the header secret (variables provided and expected in the internal route) with a timing-safe comparison: ensure both values are converted to Buffers of equal length (pad or early-fail only if lengths differ) and use crypto.timingSafeEqual to compare them; update the check around the block that currently throws HTTPException(401, { message: "Unauthorized" }) so it uses the timing-safe result instead of provided !== expected and still throws the same 401 on mismatch.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/actions/claude-analyze/action.yml:
- Around line 159-163: The echoed workflow annotation currently injects raw
callback body via "$(cat /tmp/callback-response.txt)" into the ::error command;
instead create an escaped version of the response (e.g., in a helper shell
function like escape_for_workflow_annotation or a variable named
escaped_response) that replaces "%" -> "%25", CR -> "%0D", LF -> "%0A" and also
substitutes "::" with "%3A%3A" before embedding, then use the escaped variable
in the two echo lines (the ones using ::error title=Callback rejected ... and
::error title=Callback failed after ...) so annotations cannot be corrupted or
used to inject extra workflow commands.
In @.github/actions/claude-analyze/autoIssueRunner.mjs:
- Around line 35-39: The env var validation currently checks the raw string from
process.env[name], which lets whitespace-only values pass; change the logic to
trim the retrieved value before validating and returning it: get value =
process.env[name], ensure it's a string, compute trimmed = value.trim(), throw
Error if trimmed.length === 0, and return the trimmed string (use these symbols:
value, process.env[name], trimmed) so callers get a cleaned value.
In @.github/actions/claude-analyze/schema.mjs:
- Around line 95-100: The current JSON extraction using
raw.indexOf("{")/lastIndexOf("}") fails when braces appear in prose; update the
parsing in the Claude response handler to (1) first try extracting a fenced JSON
block (use the existing fence/parsedFence/tryParse flow and validate with
analysisOutputSchema.safeParse), (2) then attempt to parse the entire raw.trim()
as JSON and validate, and (3) as a fallback scan for valid object-like slices by
iterating every "{" start index and every "}" end index to build candidates,
tryParse each candidate until one parses, then validate with
analysisOutputSchema.safeParse; if none succeed throw "Could not locate a valid
JSON object in Claude response". Ensure you reference and reuse tryParse,
parsedFence, fence, raw, and analysisOutputSchema in the implementation.
In `@admin/src/api/admin.test.ts`:
- Around line 288-295: The test claims to verify URL-encoding but uses a
URL-safe UUID; change the test to use an ID containing reserved characters
(e.g., spaces, slashes, question marks, percent signs) so encoding actually
matters: update or add a test-local id (e.g., reservedId) and set
sampleErrorRow.id or pass reservedId into getApiErrorById, keep mocking
adminFetch to return the row, then assert adminFetch was called with the path
`/api/admin/errors/${encodeURIComponent(reservedId)}`; reference
getApiErrorById, adminFetch, and sampleErrorRow in the test to locate and update
the code.
In `@admin/src/i18n/locales/en/nav.json`:
- Line 5: The ARIA string "unreadBadgeAriaLabel" currently always uses "errors"
and is not plural-aware; update the "unreadBadgeAriaLabel" entry to use ICU
pluralization (or your i18n plural syntax) so it renders "1 unresolved API
error" for count=1 and "N unresolved API errors" otherwise, ensuring screen
readers receive grammatically correct singular/plural text.
In `@admin/src/pages/errors/ErrorDetailDialog.tsx`:
- Around line 55-60: The pendingFor state (and thus pendingStatus) can remain
stale when the dialog is closed; ensure you clear it by calling
setPendingFor(null) whenever the dialog is closed or the row changes—add
setPendingFor(null) to the dialog close handler(s) used in this component (the
function that closes the ErrorDetailDialog) and/or in a useEffect that resets
pendingFor when row becomes null or the dialog open flag changes so reopening
the same error doesn't show an unsaved status or enable Save unexpectedly;
reference pendingFor, setPendingFor, setPendingStatus and pendingStatus when
making this change.
In `@admin/src/pages/errors/ErrorsContent.tsx`:
- Around line 128-131: The Select component's value prop is being given the
sentinel string "all" while its option sentinel is the constant ANY ("__any__"),
causing mismatch; update the Select value to pass statusFilter === "all" ? ANY :
statusFilter (or otherwise ensure you convert "all" to ANY) and keep the
onValueChange mapping as (v) => onStatusFilterChange(v === ANY ? "all" : (v as
ApiErrorStatus)) so the control's selected value and the internal "all" <-> ANY
mapping remain consistent; update the same fix for both usages around the
statusFilter Select and the duplicated block that uses onStatusFilterChange.
In `@admin/src/pages/errors/useApiErrors.ts`:
- Around line 225-229: The SSE update path currently drops incoming rows that
don't match the active filter (using filterRef.current and matchesFilter) which
leaves stale entries if that row already exists in the current data; change the
handler so when a row fails matchesFilter you check the existing state inside
setData (or read data.errors) and, if the row.id exists, remove that row from
the errors array and decrement/adjust any filtered totals before returning the
new state; otherwise (if the row isn't present) return prev unchanged. Keep
isMountedRef.current check and use mergeRow only when matchesFilter is true.
In `@server/api/src/__tests__/routes/admin/errors.test.ts`:
- Around line 340-355: The readUntil function's timeoutMs isn't enforced because
await reader.read() can block indefinitely; update readUntil (and its use of
reader.read) to race each read() against a timeout promise so the loop can break
when the per-iteration or overall timeout elapses, and on timeout call
reader.cancel() (or close the stream) and return the accumulated text;
specifically wrap reader.read() in a Promise.race that rejects/resolves after
the remaining timeoutMs, preserve decoding/accumulation logic and predicate
checks, and ensure timeout handling returns acc instead of hanging.
In `@server/api/src/__tests__/routes/pages.test.ts`:
- Around line 70-71: Update the two Japanese-only JSDoc comments (the one above
const NOTE_ID and the one at lines ~96-97) to include English translations
alongside the Japanese; specifically, for the comment referencing pages.note_id
and findActiveNoteById (related to NOTE_ID = "note-access-test-001"), add an
English sentence that mirrors the Japanese meaning so both languages appear in
the same JSDoc block, and do the same for the other comment near
findActiveNoteById to satisfy the bilingual documentation rule.
In `@server/api/src/__tests__/routes/pageSnapshots.test.ts`:
- Line 57: The inline documentation comment "/** 1: pages row 2: caller email
3: findActiveNoteById */" is English-only; add a Japanese counterpart right next
to or below it (e.g. "/** 1: pages row 2: caller email 3: findActiveNoteById /
1: ページ行 2: 呼び出し元メール 3: findActiveNoteById */") so the test file
server/api/src/__tests__/routes/pageSnapshots.test.ts contains both English and
Japanese, preserve the comment formatting and UTF-8 encoding and ensure it
remains a single block comment used by the test file.
In `@server/api/src/__tests__/routes/syncPages.test.ts`:
- Line 20: The comment at the top of the test ("GET/POST sync は
ensureDefaultNote を先に叩く。モックしてチェーンをページ同期クエリに寄せる。") is Japanese-only; add an
English translation on the same line or immediately above/below so the comment
is bilingual (e.g., "GET/POST sync should call ensureDefaultNote first. Mock it
and steer the chain toward the page sync query. / GET/POST sync は
ensureDefaultNote を先に叩く。モックしてチェーンをページ同期クエリに寄せる。"), ensuring the test file's
comment remains clear and follows the bilingual guideline.
In `@server/api/src/__tests__/routes/webhooks/sentry.test.ts`:
- Around line 281-314: The test "does NOT fire repository_dispatch on a
recurrence (isNew=false)" stubs global fetch with vi.stubGlobal but only calls
vi.unstubAllGlobals() at the end, risking leakage if the test throws; wrap the
section that stubs fetch and exercises the app in a try/finally so
vi.unstubAllGlobals() (or vi.unstubGlobal("fetch")) is always called in the
finally block; update the test around the vi.stubGlobal("fetch", fetchMock) /
app.request(...) / assertions to ensure cleanup (refer to fetchMock,
vi.stubGlobal, vi.unstubAllGlobals/vi.unstubGlobal, and createApp).
In `@server/api/src/__tests__/services/pageAccessService.test.ts`:
- Around line 146-148: The test uses noRoleChains(USER_ID) which still models
the caller as the owner; change the mock to model a different user so the
non-owner branch is exercised: replace the role chain setup passed to
createMockDb from noRoleChains(USER_ID) to a role chain for a different id (e.g.
ANOTHER_USER_ID or OWNER_ID depending on existing fixtures) so that
assertPageEditAccess(asDb(db), PAGE_ID, USER_ID) rejects as intended; update any
related constants in the test to ensure USER_ID is not the underlying row owner
and all mocks (createMockDb, noRoleChains) reference the distinct id.
In `@server/api/src/lib/githubAppAuth.ts`:
- Around line 155-191: The getInstallationToken function currently lets every
caller perform its own POST when the cache is cold or within REFRESH_MARGIN_MS;
add a module-scoped in-flight promise (e.g., installationTokenRefreshPromise) to
coalesce concurrent refreshes: at the top of getInstallationToken, after
checking cachedInstallationToken, if installationTokenRefreshPromise exists
return await it; otherwise create and store a promise that performs the existing
fetch/validation flow, sets cachedInstallationToken = { token, expiresAt } on
success, and clears installationTokenRefreshPromise in a finally block (also
clear on error) so subsequent callers either await the single inflight request
or trigger a new one once cleared; reference getInstallationToken,
cachedInstallationToken and REFRESH_MARGIN_MS for locating where to add the
promise logic.
In `@server/api/src/lib/sentry.ts`:
- Around line 99-107: The scrubSentryEvent function currently copies
event.request through unchanged; update it to drop or normalize
event.request.url to avoid leaking path tokens (e.g., capability IDs) — locate
scrubSentryEvent and, when rebuilding event.request, replace event.request.url
with a sanitized value (either null/empty string or a normalized path-only value
such as ApiErrorContext.routePath or a function that strips dynamic segments and
query strings), reusing existing helpers like scrubQueryStringField where
appropriate so the URL no longer contains raw path tokens before returning the
event.
In `@server/api/src/routes/admin/errors.ts`:
- Line 280: The handler returns a success payload with the key "error"
containing row data (e.g., the response at the return c.json({ error: row }) in
server/api/src/routes/admin/errors.ts and the analogous return in the PATCH
handler), which is misleading; update both handlers to use a descriptive key
such as data or apiError (for example change { error: row } to { data: row } in
the GET handler and the PATCH handler) so the response accurately reflects that
this is successful row data rather than an error.
---
Outside diff comments:
In `@server/api/src/routes/notes/members.ts`:
- Around line 267-275: The update currently marks noteMembers as deleted and
always calls publishNoteEvent("note.permission_changed") even when no active
member was matched; modify the mutation to target only active members (include
eq(noteMembers.isDeleted, false) or equivalent) and capture the update
result/affected row count from the db.update call (the update on noteMembers
that sets isDeleted and updatedAt). Only call publishNoteEvent({ type:
"note.permission_changed", note_id: noteId }) when the update affected at least
one row; leave the rest of the logic (setting isDeleted and updatedAt)
unchanged.
---
Nitpick comments:
In `@admin/src/pages/errors/useApiErrors.test.ts`:
- Around line 225-239: Add a new test to cover the case where an already-visible
error becomes filter-mismatched after an SSE update: renderHook with
useApiErrors({ status: "open", severity: "high", intervalMs: 0 }), seed an
initial matching item via makePushed({ status: "open", severity: "high" }) and
dispatch it with getInstance().dispatch("ready", "") and dispatch("update",
initialItem), assert it appears in result.current.errors, then dispatch an
update that changes its fields to mismatched values (e.g., dispatch("update", {
...initialItem, status: "resolved", severity: "low" })) and assert the item is
removed from result.current.errors and result.current.total updates accordingly;
use waitFor/act around async state assertions as in the existing test.
In `@scripts/gen-pdf-fixture.ts`:
- Around line 54-55: The `/Length` value uses character count
(page1Content.length) which breaks for non-ASCII bytes; compute the stream byte
length instead by encoding the content with TextEncoder and using its byte
length when writing `/Length` (e.g., replace references to page1Content.length
with the byte length from TextEncoder().encode(page1Content)); apply the same
change for any other stream content variables (e.g., page2Content) so all
`/Length` fields reflect actual byte sizes.
In `@server/api/src/routes/internal.ts`:
- Around line 56-59: Replace the direct string comparison of the header secret
(variables provided and expected in the internal route) with a timing-safe
comparison: ensure both values are converted to Buffers of equal length (pad or
early-fail only if lengths differ) and use crypto.timingSafeEqual to compare
them; update the check around the block that currently throws HTTPException(401,
{ message: "Unauthorized" }) so it uses the timing-safe result instead of
provided !== expected and still throws the same 401 on mismatch.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 3536d434-5118-45d2-81a8-12265c62c0c4
⛔ Files ignored due to path filters (4)
bun.lockis excluded by!**/*.locke2e/fixtures/sample.pdfis excluded by!**/*.pdfserver/api/bun.lockis excluded by!**/*.locksrc-tauri/Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (296)
.env.example.github/actions/claude-analyze/README.md.github/actions/claude-analyze/__tests__/autoIssue.test.mjs.github/actions/claude-analyze/__tests__/fixtures/invalid-bad-severity.json.github/actions/claude-analyze/__tests__/fixtures/invalid-missing-summary.json.github/actions/claude-analyze/__tests__/fixtures/invalid-suspected-file.json.github/actions/claude-analyze/__tests__/fixtures/invalid-too-many-files.json.github/actions/claude-analyze/__tests__/fixtures/valid-high.json.github/actions/claude-analyze/__tests__/fixtures/valid-low-nulls.json.github/actions/claude-analyze/__tests__/schema.test.mjs.github/actions/claude-analyze/action.yml.github/actions/claude-analyze/analyze.mjs.github/actions/claude-analyze/autoIssue.mjs.github/actions/claude-analyze/autoIssueRunner.mjs.github/actions/claude-analyze/prompt.md.github/actions/claude-analyze/schema.mjs.github/workflows/analyze-error.yml.github/workflows/ci.yml.gitignore.gitleaksignoreadmin/.env.exampleadmin/e2e/errors-page.spec.tsadmin/package.jsonadmin/playwright.config.tsadmin/src/App.tsxadmin/src/api/admin.test.tsadmin/src/api/admin.tsadmin/src/api/client.tsadmin/src/components/ErrorBoundary.test.tsxadmin/src/components/ErrorBoundary.tsxadmin/src/i18n/index.tsadmin/src/i18n/locales/en/errors.jsonadmin/src/i18n/locales/en/nav.jsonadmin/src/i18n/locales/ja/errors.jsonadmin/src/i18n/locales/ja/nav.jsonadmin/src/lib/sentry.tsadmin/src/main.tsxadmin/src/pages/Layout.tsxadmin/src/pages/errors/ErrorDetailDialog.tsxadmin/src/pages/errors/ErrorsContent.test.tsxadmin/src/pages/errors/ErrorsContent.tsxadmin/src/pages/errors/index.tsxadmin/src/pages/errors/useApiErrorActiveCount.tsadmin/src/pages/errors/useApiErrors.test.tsadmin/src/pages/errors/useApiErrors.tse2e/auth-mock.tse2e/fixtures/README.mde2e/page-editor.spec.tse2e/pdf-knowledge.spec.tse2e/web-clipper.spec.tse2e/wikilink-create-dialog.spec.tsextension/README.mdextension/background.jsextension/constants.jsextension/manifest.base.jsonextension/manifest.jsonextension/popup.htmlextension/popup.jsknip.jsonpackage.jsonpackages/ui/package.jsonscripts/check-drizzle-schema-drift.mjsscripts/check-drizzle-schema-drift.test.mjsscripts/copy-pdfjs-assets.mjsscripts/gen-pdf-fixture.tsserver/api/.env.exampleserver/api/drizzle/0019_add_api_errors.sqlserver/api/drizzle/0020_seed_thumbnail_tier_quotas.sqlserver/api/drizzle/0021_add_pages_thumbnail_object_id.sqlserver/api/drizzle/0022_add_default_note.sqlserver/api/drizzle/0023_migrate_personal_pages_drop_note_pages.sqlserver/api/drizzle/0024_add_pages_note_active_updated_index.sqlserver/api/drizzle/0025_add_pdf_source_metadata.sqlserver/api/drizzle/0026_add_pdf_highlights.sqlserver/api/drizzle/0027_add_pages_note_active_updated_id_index.sqlserver/api/drizzle/0028_add_page_snapshots.sqlserver/api/drizzle/0029_invite_links_user_fk_cascade.sqlserver/api/drizzle/0030_add_notes_tag_filter_bar.sqlserver/api/drizzle/meta/_journal.jsonserver/api/package.jsonserver/api/src/__tests__/middleware/errorHandler.test.tsserver/api/src/__tests__/routes/activity.test.tsserver/api/src/__tests__/routes/admin/errors.test.tsserver/api/src/__tests__/routes/media.test.tsserver/api/src/__tests__/routes/notes/crud.test.tsserver/api/src/__tests__/routes/notes/events.test.tsserver/api/src/__tests__/routes/notes/me.test.tsserver/api/src/__tests__/routes/notes/page-titles.test.tsserver/api/src/__tests__/routes/notes/pages.test.tsserver/api/src/__tests__/routes/notes/search.test.tsserver/api/src/__tests__/routes/notes/setup.tsserver/api/src/__tests__/routes/onboarding.test.tsserver/api/src/__tests__/routes/pageSnapshots.test.tsserver/api/src/__tests__/routes/pages.test.tsserver/api/src/__tests__/routes/search.test.tsserver/api/src/__tests__/routes/syncPages.test.tsserver/api/src/__tests__/routes/thumbnail/commit.test.tsserver/api/src/__tests__/routes/thumbnail/serve.test.tsserver/api/src/__tests__/routes/webhooks/githubAiCallback.test.tsserver/api/src/__tests__/routes/webhooks/sentry.test.tsserver/api/src/__tests__/services/apiErrorBroadcaster.test.tsserver/api/src/__tests__/services/commitService.test.tsserver/api/src/__tests__/services/noteEventBroadcaster.test.tsserver/api/src/__tests__/services/pageAccessService.test.tsserver/api/src/__tests__/services/pageGraphSyncService.test.tsserver/api/src/__tests__/services/snapshotService.test.tsserver/api/src/__tests__/services/thumbnailGcService.test.tsserver/api/src/app.tsserver/api/src/index.tsserver/api/src/lib/clipAndCreate.tsserver/api/src/lib/githubAppAuth.test.tsserver/api/src/lib/githubAppAuth.tsserver/api/src/lib/sentry.test.tsserver/api/src/lib/sentry.tsserver/api/src/lib/welcomePageService.tsserver/api/src/middleware/csrfOrigin.tsserver/api/src/middleware/errorHandler.tsserver/api/src/routes/activity.tsserver/api/src/routes/admin/errors.tsserver/api/src/routes/admin/index.tsserver/api/src/routes/internal.tsserver/api/src/routes/notes/crud.tsserver/api/src/routes/notes/eventHelpers.tsserver/api/src/routes/notes/events.tsserver/api/src/routes/notes/helpers.tsserver/api/src/routes/notes/index.tsserver/api/src/routes/notes/me.tsserver/api/src/routes/notes/members.tsserver/api/src/routes/notes/pages.tsserver/api/src/routes/notes/search.tsserver/api/src/routes/notes/tags.tsserver/api/src/routes/notes/titleIndex.tsserver/api/src/routes/notes/types.tsserver/api/src/routes/onboarding.tsserver/api/src/routes/pageSnapshots.tsserver/api/src/routes/pages.tsserver/api/src/routes/pdfSources.tsserver/api/src/routes/search.tsserver/api/src/routes/syncPages.tsserver/api/src/routes/thumbnail/commit.tsserver/api/src/routes/thumbnail/serve.tsserver/api/src/routes/webhooks/githubAiCallback.tsserver/api/src/routes/webhooks/sentry.tsserver/api/src/routes/wikiSchema.tsserver/api/src/schema/apiErrors.tsserver/api/src/schema/index.tsserver/api/src/schema/notes.tsserver/api/src/schema/pages.tsserver/api/src/schema/pdfHighlights.tsserver/api/src/schema/relations.tsserver/api/src/schema/sources.tsserver/api/src/services/apiErrorBroadcaster.tsserver/api/src/services/apiErrorService.test.tsserver/api/src/services/apiErrorService.tsserver/api/src/services/commitService.tsserver/api/src/services/defaultNoteService.test.tsserver/api/src/services/defaultNoteService.tsserver/api/src/services/indexBuilder.tsserver/api/src/services/noteEventBroadcaster.tsserver/api/src/services/notifier.test.tsserver/api/src/services/notifier.tsserver/api/src/services/pageAccessService.tsserver/api/src/services/pageGraphSyncService.tsserver/api/src/services/snapshotService.tsserver/api/src/services/thumbnailGcService.tsserver/api/src/services/titleRenamePropagationService.tsserver/api/src/types/dbOrTx.tsserver/hocuspocus/src/health.test.tsserver/hocuspocus/src/health.tsserver/hocuspocus/src/index.tsserver/hocuspocus/src/pageEditPermission.test.tsserver/hocuspocus/src/pageEditPermission.tsserver/hocuspocus/src/snapshotUtils.tsserver/hocuspocus/src/ydocWikiLinkNormalizer.test.tsserver/hocuspocus/src/ydocWikiLinkNormalizer.tsserver/mcp/README.mdserver/mcp/src/__tests__/client/httpClient.test.tsserver/mcp/src/__tests__/e2e/stdio-roundtrip.test.tsserver/mcp/src/__tests__/server.test.tsserver/mcp/src/__tests__/tools/index.test.tsserver/mcp/src/client/ZediClient.tsserver/mcp/src/client/httpClient.tsserver/mcp/src/tools/index.tssrc-tauri/Cargo.tomlsrc-tauri/src/lib.rssrc-tauri/src/pdf_sources.rssrc/App.tsxsrc/components/ErrorBoundary.tsxsrc/components/ai-chat/AIChatWikiLink.tsxsrc/components/ai-chat/PromoteToWikiDialog.tsxsrc/components/auth/ProtectedRoute.tsxsrc/components/editor/PageEditor/PageEditorAlerts.tsxsrc/components/editor/PageEditor/PageEditorDialogs.tsxsrc/components/editor/PageEditor/PageEditorHeader.test.tsxsrc/components/editor/PageEditor/PageEditorHeader.tsxsrc/components/editor/PageEditor/PageEditorLayout.test.tsxsrc/components/editor/PageEditor/PageEditorLayout.tsxsrc/components/editor/PageEditor/types.tssrc/components/editor/PageEditor/useEditorAutoSave.test.tssrc/components/editor/PageEditor/useEditorAutoSave.tssrc/components/editor/PageEditor/usePageDeletion.test.tssrc/components/editor/PageEditor/usePageDeletion.tssrc/components/editor/PageEditor/usePageEditor.tssrc/components/editor/PageEditor/usePageEditorAIEffects.tssrc/components/editor/PageEditor/usePageEditorAutoSaveWithMutation.tssrc/components/editor/PageEditor/usePageEditorEffects.test.tsxsrc/components/editor/PageEditor/usePageEditorEffects.tssrc/components/editor/PageEditor/usePageEditorHandlers.tssrc/components/editor/PageEditor/usePageEditorKeyboard.tssrc/components/editor/PageEditor/usePageEditorState.tssrc/components/editor/PageEditor/usePageEditorStateAndSync.tssrc/components/editor/PageEditor/usePageEditorStateAndSyncReturnSlices.tssrc/components/editor/PageEditor/usePageEditorWikiCollab.tssrc/components/editor/PageEditor/usePendingChatPageGeneration.test.tsxsrc/components/editor/PageEditor/usePendingChatPageGeneration.tssrc/components/editor/PageEditorView.tsxsrc/components/editor/TiptapEditor/types.tssrc/components/editor/TiptapEditor/useEditorLifecycle.tssrc/components/editor/TiptapEditor/useTagStatusSync.test.tsxsrc/components/editor/TiptapEditor/useTagStatusSync.tssrc/components/editor/TiptapEditor/useWikiLinkNavigation.test.tssrc/components/editor/TiptapEditor/useWikiLinkNavigation.tssrc/components/editor/TiptapEditor/useWikiLinkStatusSync.test.tsxsrc/components/editor/TiptapEditor/useWikiLinkStatusSync.tssrc/components/editor/WebClipperDialog.tsxsrc/components/editor/extensions/slashSuggestionPlugin.test.tssrc/components/editor/extensions/slashSuggestionPlugin.tssrc/components/editor/useWebClipperDialogSubmit.tssrc/components/editor/useWebClipperDialogSubmit.tsxsrc/components/layout/AppLayout.test.tsxsrc/components/layout/AppLayout.tsxsrc/components/layout/BottomNav/BottomNav.test.tsxsrc/components/layout/BottomNav/BottomNavTab.tsxsrc/components/layout/BottomNav/index.tsxsrc/components/layout/FABMenu.tsxsrc/components/layout/FloatingActionButton.tsxsrc/components/layout/Header/HeaderLogo.test.tsxsrc/components/layout/Header/HeaderLogo.tsxsrc/components/layout/Header/HeaderSearchBar.tsxsrc/components/layout/Header/HeaderSearchDropdownContent.tsxsrc/components/layout/Header/NavigationMenu.test.tsxsrc/components/layout/Header/index.tsxsrc/components/layout/HomePageCount.test.tsxsrc/components/layout/HomePageCount.tsxsrc/components/layout/MobileHeader.tsxsrc/components/layout/navigationItems.test.tssrc/components/layout/navigationItems.tssrc/components/layout/useFloatingActionButtonHandlers.tssrc/components/note/EditorSkeleton.test.tsxsrc/components/note/EditorSkeleton.tsxsrc/components/note/NotePageCard.tsxsrc/components/note/NotePagePublicView.test.tsxsrc/components/note/NotePagePublicView.tsxsrc/components/note/NoteShareUrlCopyButton.test.tsxsrc/components/note/NoteShareUrlCopyButton.tsxsrc/components/note/NoteTitleSwitcher.test.tsxsrc/components/note/NoteTitleSwitcher.tsxsrc/components/note/PageEditorContent.tsxsrc/components/note/PageTitleBlock.test.tsxsrc/components/note/PageTitleBlock.tsxsrc/components/page/EmptyState.tsxsrc/components/page/LinkGroupRow.tsxsrc/components/page/LinkSection.tsxsrc/components/page/LinkedPagesSection.test.tsxsrc/components/page/LinkedPagesSection.tsxsrc/components/page/PageCard.tsxsrc/components/page/PageGrid.test.tsxsrc/components/page/PageGrid.tsxsrc/components/page/PageLinkCard.test.tsxsrc/components/pdf-reader/HighlightLayer.test.tsxsrc/components/pdf-reader/HighlightLayer.tsxsrc/components/pdf-reader/HighlightSidebar.test.tsxsrc/components/pdf-reader/HighlightSidebar.tsxsrc/components/pdf-reader/HighlightToolbar.test.tsxsrc/components/pdf-reader/HighlightToolbar.tsxsrc/components/pdf-reader/MissingPdfBanner.tsxsrc/components/pdf-reader/OpenPdfButton.tsxsrc/components/pdf-reader/PdfPageCanvas.test.tsxsrc/components/pdf-reader/PdfPageCanvas.tsxsrc/components/pdf-reader/PdfReader.tsxsrc/components/pdf-reader/PdfReaderUnsupported.tsxsrc/components/pdf-reader/pdfTextLayer.csssrc/components/pdf-reader/saveAndDeriveFlow.test.tssrc/components/pdf-reader/saveAndDeriveFlow.tssrc/components/pdf-reader/selectionToPdfRects.test.tssrc/components/pdf-reader/selectionToPdfRects.tssrc/components/pdf-reader/useOpenPdfFlow.test.tssrc/components/pdf-reader/useOpenPdfFlow.tssrc/components/pdf-reader/usePdfDocument.tssrc/components/search/SearchResultCard.tsxsrc/components/tagFilterBar/TagChip.tsxsrc/components/tagFilterBar/TagFilterBar.tsxsrc/components/tagFilterBar/index.tssrc/contexts/GlobalSearchContext.test.tssrc/contexts/GlobalSearchContext.tsxsrc/hooks/runAIChatAction.test.ts
💤 Files with no reviewable changes (4)
- e2e/page-editor.spec.ts
- server/api/src/tests/services/snapshotService.test.ts
- e2e/wikilink-create-dialog.spec.ts
- extension/popup.js
There was a problem hiding this comment.
Code Review
This pull request implements a comprehensive API error tracking and AI analysis system (Epic #616), introducing an api_errors table, a Claude-based analysis workflow, and an admin dashboard. It also completes the migration to a note-centric page model (Issue #823) and adds real-time updates via Server-Sent Events. Feedback identifies critical issues including an invalid version for the setup-node action, an incorrect Claude model identifier in the analysis script, and a potential regression in the browser extension where logged-out users can no longer initiate clips from the popup.
I am having trouble creating individual review comments. Click here to see my feedback.
.github/actions/claude-analyze/action.yml (81)
actions/setup-node のバージョン v6 は存在しないようです。最新の安定版は v4 です。このままだとワークフローが失敗する可能性があるため、v4 に修正することを推奨します。
uses: actions/setup-node@v4.github/actions/claude-analyze/analyze.mjs (55)
claude-sonnet-4-6 というモデル名は存在しないようです。おそらく claude-3.5-sonnet-20240620 または claude-3-sonnet-20240229 のタイポだと思われます。このままだとAnthropic API呼び出しが失敗するため、正しいモデル名に修正してください。
const DEFAULT_MODEL = "claude-3.5-sonnet-20240620";
extension/popup.js (181-184)
この openZediWithClipUrl 関数と、関連するクリックハンドラの else ブロックが削除されたことで、ログアウト状態のユーザーがポップアップの「保存」ボタンをクリックしても何も起きなくなってしまいます。これは意図した動作でしょうか?もし意図しない場合は、background.js のコンテキストメニューと同様に、クリップ対象URLを渡してZediを開く処理を維持することを検討してください。
* feat(error): add ErrorScreen as ErrorBoundary fallback 予期せぬ runtime/render エラー発生時に表示する全画面フォールバック `ErrorScreen` を追加し、`main.tsx` の ErrorBoundary に注入する。 NotFound と同トーンの中央寄せレイアウトで、再読み込みとホームへの 2 つのアクションを提供する。Sentry の eventId 表示は運用開始時の 追加タスクとして #906 で追跡する。 Add ErrorScreen — a full-page fallback rendered by the root ErrorBoundary when an unexpected error is caught. Mirrors the NotFound layout and offers reload / back-to-home actions. Surfacing the Sentry eventId is deferred to #906. Refs: #906 * fix(error): use native anchor in ErrorScreen to avoid Router crash Root ErrorBoundary は BrowserRouter の外側で fallback を描画するため、 react-router の <Link> は `useHref` で実行時エラーになる。素の <a> に 置き換えてフルリロード経路にし、boundary 状態とアプリ状態をまとめて リセットできるようにする。 The root ErrorBoundary renders its fallback outside the BrowserRouter context, which made react-router's <Link> crash on `useHref`. Use a plain <a> so the fallback also performs a full page reload, clearing boundary and app state in one step. PR feedback: gemini-code-assist on #907. --------- Co-authored-by: Claude <noreply@anthropic.com>
* fix: address PR #905 review comments Implement CodeRabbit/Codex feedback for admin errors UX, Sentry scrubbing, GitHub Actions analyze-error hardening, Claude JSON parsing, and related tests. Co-authored-by: Cursor <cursoragent@cursor.com> * fix: address PR #908 review comments Fix API typecheck for scrubRequestUrl, scrub relative request URLs with a synthetic base, and only unwrap json-labeled markdown fences in Claude analyze parsing. Co-authored-by: Cursor <cursoragent@cursor.com> * fix: address PR #908 CodeRabbit review comments Detect analysis root payload by own keys for fast schema failures; reset pending status on Cancel in ErrorDetailDialog. Co-authored-by: Cursor <cursoragent@cursor.com> --------- Co-authored-by: Cursor <cursoragent@cursor.com>
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@admin/src/pages/errors/ErrorDetailDialog.tsx`:
- Line 62: The inline comment in the ErrorDetailDialog component currently
contains only Japanese; add an English translation immediately after (or before)
the Japanese sentence to make the doc bilingual. Locate the comment inside the
ErrorDetailDialog (the comment starting "オーバーレイ・ESC と同様に、キャンセルボタンでも未保存選択を破棄する。")
and append a concise English equivalent such as "Discard unsaved selection when
the cancel button is used, same as overlay/ESC." to match repository bilingual
comment standards.
In `@server/api/src/__tests__/services/pageAccessService.test.ts`:
- Around line 44-45: Update the block comment above the helper function
noRoleChains to include a Japanese translation of the existing English note
("Private note, caller not owner: member + domain rule lookups run then deny.")
so the comment contains both English and Japanese bilingual text; locate the
comment immediately above function noRoleChains and add a concise Japanese
sentence conveying the same meaning (e.g.
「非公開メモ。呼び出し元がオーナーでない場合:メンバーとドメインのルール照会を行い、その後拒否されます。」) so the comment block
includes both languages.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: dc2ef73c-fedc-41bc-aef1-2d9882727b3f
📒 Files selected for processing (24)
.github/actions/claude-analyze/__tests__/schema.test.mjs.github/actions/claude-analyze/action.yml.github/actions/claude-analyze/autoIssueRunner.mjs.github/actions/claude-analyze/escapeGithubAnnotation.mjs.github/actions/claude-analyze/schema.mjs.github/workflows/analyze-error.ymladmin/src/api/admin.test.tsadmin/src/i18n/locales/en/nav.jsonadmin/src/i18n/locales/ja/nav.jsonadmin/src/pages/errors/ErrorDetailDialog.test.tsxadmin/src/pages/errors/ErrorDetailDialog.tsxadmin/src/pages/errors/ErrorsContent.tsxadmin/src/pages/errors/useApiErrors.tsserver/api/src/__tests__/routes/admin/errors.test.tsserver/api/src/__tests__/routes/pageSnapshots.test.tsserver/api/src/__tests__/routes/pages.test.tsserver/api/src/__tests__/routes/syncPages.test.tsserver/api/src/__tests__/routes/webhooks/sentry.test.tsserver/api/src/__tests__/services/pageAccessService.test.tsserver/api/src/lib/githubAppAuth.tsserver/api/src/lib/sentry.test.tsserver/api/src/lib/sentry.tssrc/components/ErrorScreen.test.tsxsrc/components/ErrorScreen.tsx
✅ Files skipped from review due to trivial changes (3)
- .github/actions/claude-analyze/escapeGithubAnnotation.mjs
- admin/src/i18n/locales/ja/nav.json
- admin/src/i18n/locales/en/nav.json
🚧 Files skipped from review as they are similar to previous changes (13)
- admin/src/api/admin.test.ts
- server/api/src/tests/routes/admin/errors.test.ts
- admin/src/pages/errors/ErrorsContent.tsx
- server/api/src/tests/routes/webhooks/sentry.test.ts
- server/api/src/lib/sentry.test.ts
- .github/actions/claude-analyze/action.yml
- admin/src/pages/errors/useApiErrors.ts
- server/api/src/tests/routes/syncPages.test.ts
- server/api/src/lib/sentry.ts
- .github/workflows/analyze-error.yml
- .github/actions/claude-analyze/autoIssueRunner.mjs
- server/api/src/tests/routes/pages.test.ts
- server/api/src/lib/githubAppAuth.ts
| setPendingFor({ id: row.id, status: next }); | ||
| }; | ||
|
|
||
| /** オーバーレイ・ESC と同様に、キャンセルボタンでも未保存選択を破棄する。 */ |
There was a problem hiding this comment.
Make this inline doc comment bilingual.
Line 62 currently has only Japanese text; add an English counterpart to match repository standards.
Proposed fix
- /** オーバーレイ・ESC と同様に、キャンセルボタンでも未保存選択を破棄する。 */
+ /**
+ * オーバーレイ・ESC と同様に、キャンセルボタンでも未保存選択を破棄する。
+ * Discard unsaved selection on Cancel as well as overlay/ESC close.
+ */
const handleClose = () => {As per coding guidelines **/*.{ts,tsx,js,md}: Include both Japanese and English comments/documentation in code and documentation files.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| /** オーバーレイ・ESC と同様に、キャンセルボタンでも未保存選択を破棄する。 */ | |
| /** | |
| * オーバーレイ・ESC と同様に、キャンセルボタンでも未保存選択を破棄する。 | |
| * Discard unsaved selection on Cancel as well as overlay/ESC close. | |
| */ |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@admin/src/pages/errors/ErrorDetailDialog.tsx` at line 62, The inline comment
in the ErrorDetailDialog component currently contains only Japanese; add an
English translation immediately after (or before) the Japanese sentence to make
the doc bilingual. Locate the comment inside the ErrorDetailDialog (the comment
starting "オーバーレイ・ESC と同様に、キャンセルボタンでも未保存選択を破棄する。") and append a concise English
equivalent such as "Discard unsaved selection when the cancel button is used,
same as overlay/ESC." to match repository bilingual comment standards.
| /** Private note, caller not owner: member + domain rule lookups run then deny. */ | ||
| function noRoleChains(pageOwnerId: string) { |
There was a problem hiding this comment.
Add Japanese text to the helper comment for bilingual consistency.
Line 44 is English-only. Please add a Japanese counterpart in the same block comment.
Proposed fix
-/** Private note, caller not owner: member + domain rule lookups run then deny. */
+/**
+ * 非公開ノートで呼び出し元がオーナーでない場合: member/domain の参照後に拒否する。
+ * Private note, caller not owner: member + domain rule lookups run then deny.
+ */
function noRoleChains(pageOwnerId: string) {As per coding guidelines **/*.{ts,tsx,js,md}: Include both Japanese and English comments/documentation in code and documentation files.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| /** Private note, caller not owner: member + domain rule lookups run then deny. */ | |
| function noRoleChains(pageOwnerId: string) { | |
| /** | |
| * 非公開ノートで呼び出し元がオーナーでない場合: member/domain の参照後に拒否する。 | |
| * Private note, caller not owner: member + domain rule lookups run then deny. | |
| */ | |
| function noRoleChains(pageOwnerId: string) { |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@server/api/src/__tests__/services/pageAccessService.test.ts` around lines 44
- 45, Update the block comment above the helper function noRoleChains to include
a Japanese translation of the existing English note ("Private note, caller not
owner: member + domain rule lookups run then deny.") so the comment contains
both English and Japanese bilingual text; locate the comment immediately above
function noRoleChains and add a concise Japanese sentence conveying the same
meaning (e.g. 「非公開メモ。呼び出し元がオーナーでない場合:メンバーとドメインのルール照会を行い、その後拒否されます。」) so the
comment block includes both languages.
…op/cloudflare/wrangler-action-4 chore(deps): bump cloudflare/wrangler-action from 3 to 4
…/minor-and-patch-c77b26d34a chore(deps): bump the minor-and-patch group with 2 updates
…/pdfjs-dist-5.7.284 chore(deps): bump pdfjs-dist from 4.10.38 to 5.7.284
Co-authored-by: Cursor <cursoragent@cursor.com>
概要
developの累積変更を本番ラインmainに取り込むリリース用 PR です(integration → production)。変更点
本 PR はブランチ間マージのため差分が大きくなっています。主な領域は次のとおりです。
src/NotePageView/NotePagePublicView、タグフィルタバー、PDF ビューア・ハイライト連携、協調編集・FAB の整理 など)server/api/snapshotService廃止、/health、マイグレーション0030_add_notes_tag_filter_bar.sqlなど)server/hocuspocus//health、WikiLink 正規化、認証・同期まわりの修正server/mcp/e2e/.github/変更の種類
※ API・ルーティングに変更があるため、マージ前にステージングでの動作確認とマイグレーション適用を推奨します。
テスト方法
チェックリスト
server/api/drizzle/)を確認したスクリーンショット(UI 変更がある場合)
ノート UI・タグフィルタ・PDF・スケルトン表示など変更あり。必要に応じてリリースノート用にキャプチャを添付してください。
関連 Issue
複数の機能 Issue / PR が
developに集約されています。代表的なもの: #389, #675, #847, #860, #863, #864, #880, #889, #890, #896, #897 など。Made with Cursor
Summary by CodeRabbit
New Features
Bug Fixes
Documentation