The `Youtube` brand icon from lucide-react is deprecated and slated for
removal. The YouTube section header and per-row icon already sit next to
the "YouTube" label or full URL, so the brand mark is redundant — collapse
both web and youtube items to the existing `Globe` icon.
Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
First wave of eliminating global `app` references (per Obsidian's
`obsidianmd/no-restricted-globals` rule). Adds a `useApp()` hook that
reads from `AppContext` and throws if no provider is in scope, then
converts the React layer to use it. Modals that render React content
now wrap their tree with `<AppContext.Provider value={this.app}>` so
descendants pick up the modal-owned app rather than the global one,
which matters in popout windows.
Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* chore(types): tighten any types and fix langchain getType deprecation
- Replace `message.getType()` with `message.type` in BedrockChatModel
- Type `docToSave` as `OramaDocument` and `explanation` as `SearchExplanation`
- Extract a typed `fallbackMetadata` to avoid `any` union warnings
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* fix(lint): drop unnecessary `as unknown` cast on upsert return
Now that `docToSave` is typed as `OramaDocument`, it's already assignable
to the `Promise<unknown>` return type without an explicit cast.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* fix(mobile): handle desktop-encrypted API keys on mobile (follow-up to #2443)
PR #2443 added the Buffer polyfill to base64.ts, utils.ts, and
BedrockChatModel.ts but missed src/encryptionService.ts:119. My
original write-up incorrectly claimed mobile never enters the
DESKTOP_PREFIX branch of getDecryptedKey — it does, whenever a user
encrypts their API keys on desktop and then syncs the vault to mobile
via Obsidian Sync. Mobile reads the same encrypted blob, enters the
DESKTOP_PREFIX branch, and hits `Buffer.from(...)`, which throws
"Can't find variable: Buffer" on iOS WebKit.
User reproducer: "Model request failed: Can't find variable: Buffer"
during normal chat send on mobile after PR #2443 merged.
Two-part fix:
1. Add `import { Buffer } from "buffer"` (the npm polyfill, already in
dependencies and used in PR #2443) so the literal ReferenceError is
gone. The legacy ENCRYPTION_PREFIX branch at line 137 also benefits.
2. Even with Buffer available, the next line
`getSafeStorage()!.decryptString(buffer)` cannot succeed on mobile
because there is no Electron safeStorage there. The encrypted blob
uses OS-level encryption (Keychain on macOS, DPAPI on Windows,
libsecret on Linux) that cannot be replayed from mobile. Add a
Platform.isMobile guard that throws a clear, actionable error
asking the user to re-enter their API key on this device instead
of crashing on the unreachable decryption call.
Out of scope:
- The DESKTOP_PREFIX-on-mobile situation requires the user to re-save
their key in mobile settings (which will re-encrypt with
WEBCRYPTO_PREFIX, the cross-platform path that Web Crypto handles).
Surfacing a clear error is the right user experience until the
plugin auto-migrates on first mobile launch — that's a larger
follow-up not appropriate for a same-day hotfix.
- The legacy ENCRYPTION_PREFIX branch at line 137 still references
Buffer, but it's already gated by
`getSafeStorage()?.isEncryptionAvailable()` which returns false on
mobile, so mobile never enters it. The new import covers it
defensively if that gate ever changes.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* fix(mobile): list desktop-encrypted keys on mobile load + block desktop encryption
Two additions on top of the existing fix in this PR:
1. **Guard the encryption side with Platform.isDesktop.** Mobile should
NEVER produce DESKTOP_PREFIX keys, even if `require("electron")`
happens to return a truthy shim. Before this change, the only gate
was `getSafeStorage()?.isEncryptionAvailable()` — fragile, since
any mobile environment that exposes a partial Electron-like shim
could slip through and save a key as DESKTOP_PREFIX that the same
device then can't decrypt.
2. **Surface a Notice on mobile load** listing every key field that's
still DESKTOP_PREFIX-encrypted. Without this, the user only finds
out which key needs re-entry by hitting a chat error and having to
guess. With it, they see the full list at startup:
"Copilot: 3 API key(s) encrypted on desktop and unusable on
mobile. Re-enter in Copilot settings: plusLicenseKey,
openAIApiKey, activeModels[gpt-5.5]"
Notice has timeout 0 so it stays until dismissed.
The new exported `findDesktopEncryptedKeyFields(settings)` walks:
- All top-level string settings (catches plusLicenseKey,
openAIApiKey, anthropicApiKey, githubCopilotToken, etc.)
- settings.activeModels[*].apiKey (per-model custom keys)
- settings.activeEmbeddingModels[*].apiKey
The exact field names returned match the keys in data.json so the
user can grep / inspect / re-enter them confidently.
This addresses the user report after the first commit in this PR:
"despite reentering api key on mobile" — which happened because they
re-entered one key but a *different* key (likely plusLicenseKey or
a model-specific apiKey) was still DESKTOP_PREFIX, causing the chat
to fail on that other key.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* fix(encryption): deprecate encryption-at-rest; make encrypt/save a no-op
The encryption toggle has created repeated incidents:
1. Mobile cannot decrypt DESKTOP_PREFIX (Electron safeStorage) keys
that synced over from desktop, because mobile has no safeStorage.
2. The WebCrypto path is security theater — the AES-GCM key is a
hardcoded "obsidian-copilot-v1" string, so anyone with data.json
can decrypt every "encrypted" key in seconds.
3. PRs #2401 and #2403 (3.3.0) introduced bare `Buffer.from` in the
encryption + base64 paths, which crashed mobile WebView with
"Can't find variable: Buffer" — see #2443 and earlier commits in
this PR.
4. Even with that fixed, the eager dispatch-map builders in
chatModelManager / embeddingManager await getDecryptedKey for
every provider; a throw on the one bad desktop-encrypted key
killed chats that used a different provider with a valid key.
Rather than continue patching, deprecate the toggle and stop adding
encryption to new keys. Existing encrypted blobs in data.json remain
readable via getDecryptedKey (for all three legacy formats:
DESKTOP_PREFIX, WEBCRYPTO_PREFIX, and the original `enc_`); they
migrate organically to plaintext the next time the user re-saves any
given key.
Changes:
- `encryptAllKeys` → no-op (returns settings unchanged). 6+ lines of
per-field encryption gone.
- `getEncryptedKey` → no-op (returns plaintext, strips `dec_` marker
if present). 25 lines of safeStorage / WebCrypto / DESKTOP_PREFIX
branching gone.
- `main.ts` settings-save → unconditionally `saveData(next)` instead
of branching on `enableEncryption`. The setting field stays in the
CopilotSettings interface for backwards-compat with existing
data.json files but is no longer read or written.
- `AdvancedSettings.tsx` → "Enable Encryption" toggle removed from
the UI. Users who had it on get no further encryption; users who
had it off see no change.
Mobile-specific path inside `getDecryptedKey`:
- DESKTOP_PREFIX keys on mobile return `DESKTOP_KEY_ON_MOBILE_SENTINEL`
(no longer throw). chatModelManager / embeddingManager's eager
dispatch-map construction completes; the sentinel sits harmlessly
in unused config slots; providers actually being used with valid
plaintext keys work. The startup Notice (already in this PR)
surfaces which fields the user must re-enter.
Tests:
- `getEncryptedKey` test suite rewritten to verify the new no-op
behavior (plaintext passthrough, dec_ stripping, empty-string).
- `encryptAllKeys` test suite shrunk to one identity-return assertion.
- Cross-platform encrypt/decrypt round-trip tests removed (encryption
is gone). Added a legacy WebCrypto decrypt test to confirm we can
still read existing enc_web_* blobs.
- 1974 tests pass (was 1975; -1 for the dropped round-trip test).
Net change: encryptionService.ts down from 207 to 130 lines.
Risk:
- `data.json` now contains plaintext API keys after first re-save.
Worth saying explicitly in release notes. The previous "encryption"
was indistinguishable from plaintext for anyone with filesystem
access (hardcoded WebCrypto key), so this is a documentation
change more than a security change.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* fix(mobile): simplify to a throw; drop sentinel + Notice + helper
User feedback: the sentinel + startup-Notice approach added 70+ lines of
transitional code that we'd have to delete later. Replace with the
lightest viable shape.
- Mobile DESKTOP_PREFIX branch in getDecryptedKey now throws a generic
but actionable error ("re-enter your API keys on this device") and
nothing else.
- Removed DESKTOP_KEY_ON_MOBILE_SENTINEL constant.
- Removed findDesktopEncryptedKeyFields function.
- Removed the startup Notice block in main.ts and its import.
Trade-off accepted: with multiple desktop-encrypted keys present, the
chat fails on the first one the eager dispatch-map encounters; user
re-enters that key, retries, hits the next bad key, repeats. The
generic message tells them what to do; they iterate rather than seeing
the full list up front.
Net: -71 lines vs. previous approach. Future cleanup (when no users
have desktop-encrypted keys anymore) is one small if-branch in
getDecryptedKey instead of a sentinel + assert + helper + Notice
spread across four files.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* fix(settings): restore version chip + reset button hidden by Obsidian h1 CSS
The top-of-settings row (Copilot Settings title, version chip, "(up to
date)" annotation, Reset Settings button) is rendered but invisible in
current Obsidian releases. DOM inspection confirms the <h1> wrapping
the row has `display: none` applied with `height: 0`:
h1Texts: ["Copilot Settingsv3.3.0 (up to date)Reset Settings"]
h1Styles: [{ display: "none", height: 0, visibility: "visible" }]
Cause: Obsidian's settings modal CSS hides plugin-rendered <h1> elements
because Obsidian reserves the top-level heading slot in a settings tab
for itself. This convention was tightened in a recent Obsidian release,
so the same code that worked at <h1>'s introduction (PR #1194) and
update-notification addition (PR #1415) now silently renders zero-height.
Fix: replace the <h1> with a <div role="heading" aria-level={1}> that
carries the same heading-style classes (tw-text-2xl tw-font-bold) plus
the existing flex/layout classes. The accessibility role/level preserves
the heading semantic for screen readers without triggering Obsidian's
display:none on real <h1>. The small-text version chip is now explicitly
tw-font-normal so it doesn't inherit the wrapper's tw-font-bold.
Verified by DOM diagnostic in live Obsidian: <h1> was being hidden;
<div role="heading"> is not affected by the same CSS rule.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* fix(settings): shrink restored heading and add spacing below
Per request: make the restored title row much smaller (tw-text-2xl
tw-font-bold → tw-text-base tw-font-semibold) and add tw-mb-4 spacing
below it so it sits comfortably above the tabs row.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Replaces `any`-typed mock objects in 26 test files with structured
types using `jest.Mock`, casts requireMock results to typed shapes,
and introduces `internals()` helpers for private-method access. No
production code changes; eliminates 487 of 679 no-unsafe-call
violations in preparation for enabling the rule.
Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Mobile Obsidian runs on Capacitor + WebView (no Node.js runtime), so the
bare `Buffer` global is undefined. On iOS WebKit this surfaces as the
runtime error "Can't find variable: Buffer" the moment the image
processing path fires. Desktop is unaffected because Electron exposes
Node globals.
Two PRs in 3.3.0 introduced direct `Buffer.from(...)` calls in
image-handling paths assuming "Buffer is global":
- #2403 rewrote src/utils/base64.ts from btoa/atob to bare Buffer.from
- #2401 did the same in src/utils.ts safeFetch arrayBuffer branch and
src/LLMProviders/BedrockChatModel.ts decodeBase64ToUint8Array
Result: any image attachment on mobile fails with "failed to send
image, please try again". Confirmed via user log:
ERROR Error sending message: Can't find variable: Buffer
The fix is an explicit `import { Buffer } from "buffer"` in each
affected file. The `buffer` npm package (already in dependencies) is a
browser-compatible polyfill that esbuild bundles into main.js, so the
same Buffer code path works on desktop (where it's effectively the
Electron-provided Buffer) and on mobile (where it's the polyfill).
Two lint rules complicate this:
- Obsidian's official scorecard flags btoa/atob as deprecated and
asks for Buffer.from. That rules out reverting to btoa/atob.
- The repo's own `import/no-nodejs-modules` rule blocks `import from
"buffer"`. Suppressed inline at each import site with justification,
because `buffer` here is the npm polyfill (in package.json
dependencies), not Node's built-in being used at runtime.
Files touched:
- src/utils/base64.ts (arrayBufferToBase64 / base64ToArrayBuffer)
This is the call site the image processor hits and the path
encryptionService.ts uses for WEBCRYPTO_PREFIX (mobile encryption),
so mobile encryption is also fixed transitively.
- src/utils.ts (safeFetch arrayBuffer fallback)
- src/LLMProviders/BedrockChatModel.ts (decodeBase64ToUint8Array)
Regression test:
- src/utils/base64.test.ts deletes globalThis.Buffer before exercising
the helpers to confirm the imported polyfill binding is what gets
used (not a bare global reference). Catches any future regression
where someone replaces the import with a bare global usage.
Out of scope:
- src/encryptionService.ts has Buffer.from references but they sit
inside desktop-only branches (`getSafeStorage().isEncryptionAvailable()`
/ DESKTOP_PREFIX) that mobile never enters. Touching them from a
mobile hotfix would expand blast radius unnecessarily.
- src/LLMProviders/BedrockChatModel.ts:835 is already guarded with
`typeof Buffer !== "undefined"` so mobile skips it; left alone.
Bundle size unchanged at 3.3 MB (buffer polyfill is small).
Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Adds a test-only override enforcing @typescript-eslint/no-unsafe-assignment
on **/*.test.{ts,tsx} and fixes all 378 violations across 44 test files.
Production code (~499 violations) remains a follow-up PR — the rule stays
"off" globally, with the violation count comment updated to reflect that.
Common fix patterns applied:
- Typed accessor helpers (asInternal) for private-method access via
`as unknown as Shape`, replacing `(instance as any).method(...)`.
- Inline `as <Type>` on mock indexing (mock.calls[i][j]), JSON.parse,
and jest.requireMock results.
- mockTFile()/mockTFolder() from @/__tests__/mockObsidian replacing
`Object.create(TFile.prototype)` (also avoids obsidianmd/no-tfile-tfolder-cast).
- `as unknown as RealType` for mock-object casts where `as any` previously
masked the assignment.
Enables @typescript-eslint/no-unsafe-member-access globally. Tests and 41
heavy source files (>5 violations each) are exempted via per-file overrides
with current violation counts annotated for follow-up PRs. Fixes 124
violations across 51 source files using narrow type assertions, instanceof
Error guards, and proper structural types instead of blanket any casts.
Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* fix(quick-command): restore Cmd+Enter shortcut and shortcut hint icons
The Quick Ask/Command revamp (#2139) replaced an Obsidian Modal with a
standalone DraggableModal. Two regressions slipped through:
- The new modal didn't push a Scope, so any user-bound global Cmd+Enter
hotkey would fire instead of Replace. Push a Scope in
`CustomCommandChatModal.open()` that registers Mod+Enter and
Mod+Shift+Enter as no-op consumers (the React onKeyDown does the real
work) and pop it in close(), matching what `Modal` did automatically.
- The Insert/Replace buttons lost the inline ⌘↩ / ⌘⇧↩ glyph hints that
existed in the pre-revamp modal. Re-add them with Lucide icons
(`Command`, `ArrowBigUp`, `CornerDownLeft`) in `action-buttons.tsx`,
with a Ctrl-text fallback on non-mac.
Also move the shortcut handler from a document-level keydown listener
back to a React `onKeyDown` on the modal subtree — same pattern as the
original implementation — so it can't be pre-empted by capture-phase
listeners.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* fix(quick-command): widen dialog so footer fits Note checkbox + shortcut glyphs
Quick Command mode renders an extra "Note" context checkbox alongside the
model selector and action buttons. With the restored ⌘⇧↩ / ⌘↩ glyphs on
Insert/Replace, the default 500px footer overflows. Bump the initial
width to `min(620px, 92vw)` only when `hideContentAreaOnIdle` (the
Quick Command signal) is set; Custom Commands keep the original width.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* fix(quick-command): widen modal unconditionally instead of gating on mode
Apply the wider footer to Custom Commands too — same overflow potential
once the Insert/Replace glyph hints are present.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Enabled in the TS-only override block. Disabled for test files since
jest assertion patterns (`expect(mock.method).toHaveBeenCalled()`)
reference methods unbound by design and have no clean workaround.
Fixed the single real source violation in colorOpacityPlugin by
calling addUtilities through the plugin API object instead of
destructuring it.
Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Moves the rule into the TS-only config block (the plugin is only registered
for .ts/.tsx). Fixes 187 violations across 74 files using narrow `as` casts,
explicit return-type annotations on callbacks, typed intermediate variables,
and tightening parameter types from `any` to `unknown` where it didn't break
callers. Also replaces the `@ts-ignore`'d `Electron.SafeStorage` reference
with a local `SafeStorage` interface in `encryptionService.ts`. No prompt
content modified; no `eslint-disable` comments added.
Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The four obsidianmd typed rules (no-view-references-in-plugin,
no-unsupported-api, prefer-file-manager-trash-file, prefer-instanceof)
are already gated to **/*.ts(x) by the plugin's hybridRecommendedConfig,
so disabling them on non-TS files is a no-op. Only no-plugin-as-component
and @typescript-eslint/no-deprecated actually leak onto non-TS files.
Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Turns on no-unsafe-enum-comparison, no-base-to-string,
no-redundant-type-constituents, and restrict-template-expressions.
Fixes all 24 violations across 13 source files.
Removes the `off` override and fixes all 368 violations via type casts,
widened parameter types (unknown / string|null|undefined for functions
that defensively handle non-string inputs), and concrete types in place
of `any` for Lexical nodes, mock objects, and option handlers.
Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* chore(eslint): enable no-unnecessary-type-assertion and no-misused-promises
Auto-fixed all 40 no-unnecessary-type-assertion violations via `eslint --fix`.
For no-misused-promises, configured `checksVoidReturn` with two relaxations
(documented inline):
- `attributes: false` — `onClick={async () => ...}` is the standard React pattern
- `inheritedMethods: false` — Obsidian's Plugin.onload/onunload are async
Fixed the remaining 10 violations by hand:
- Wrapped async subscribers/listeners with `void` (projectManager, useChatFileDrop,
main.ts, webViewerServiceSelection)
- Widened ConfirmModal subclass callback types to `() => void | Promise<void>`
- Widened MobileCardDropdownAction.onClick to match parent prop types
- Made SettingsMainV2 handleReset sync (it never awaited anything)
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* fix(eslint): move no-misused-promises to TS-only config block
The @typescript-eslint plugin is registered only for .ts/.tsx files via
obsidianmd's recommendedTypeChecked. Enabling the rule in the
JS+TS-shared block caused CI to fail with "could not find plugin
@typescript-eslint" because the plugin isn't loaded for .js/.mjs files.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Add a step that signs main.js, manifest.json, and styles.css with the
workflow's OIDC identity and publishes the attestation to Sigstore's
public transparency log via actions/attest-build-provenance@v2.
Why: Obsidian's plugin review tooling flags release assets without an
attestation as a soft warning. Attestations cryptographically prove the
asset was built by this workflow on this commit, defending against an
attacker who gains release-asset-replace permissions and silently swaps
in a compromised main.js. Verification post-release:
gh attestation verify main.js --owner logancyang --repo logancyang/obsidian-copilot
Placement: the new step runs AFTER the prerelease manifest swap and
BEFORE gh release create. That way the attested manifest.json is
exactly the bytes that get uploaded — for stable releases that's
the committed manifest.json; for prereleases it's the in-runner
copy of manifest-beta.json.
Permissions: adds attestations: write and id-token: write to the job.
The id-token: write permission is what allows the workflow to request
an OIDC token from GitHub; the attest action uses that token as proof
of identity when signing. No secrets to manage.
Free for public repos.
Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* chore(release): make prerelease tooling stop poisoning master's manifest.json
Background: Obsidian's community plugin store reads manifest.json from master
to decide which GitHub Release to serve installers. The prerelease agent's
npm version bump was rewriting manifest.json with a prerelease version, which
broke plugin installs until hotfix #2428 reverted master's manifest.json to
the stable version.
This PR fixes the underlying tooling so it can't recur.
Changes:
- version-bump.mjs now branches on whether npm_package_version is a prerelease
(contains a hyphen). Stable: writes to manifest.json + versions.json. Prerelease:
writes to manifest-beta.json + versions.json, leaving manifest.json untouched.
Seeds manifest-beta.json from manifest.json when it doesn't exist yet.
On a stable bump that finds a stale manifest-beta.json, removes it (git rm)
because the new stable supersedes the in-flight beta. Stages the right files
itself; package.json no longer needs the explicit git-add step.
- package.json: simplifies the "version" lifecycle script to just
"node version-bump.mjs" since the script now stages files itself.
- release.yml:
* "Verify version matches manifest" step now selects the file based on
is_prerelease (manifest.json for stable, manifest-beta.json for prerelease).
For stable runs nothing changes.
* Adds a "Prepare release-asset manifest" step before the release upload that
copies manifest-beta.json over manifest.json IN THE RUNNER only when the
release is a prerelease. This makes the uploaded manifest.json asset carry
the prerelease version so testers sideloading the assets get a consistent
manifest. The committed master manifest.json is never touched by the
workflow (the workflow doesn't push back).
- .claude/agents/release.md: adds a Step 0 pre-flight assertion that master's
manifest.json.version equals the latest non-prerelease GitHub Release tag.
Catches drift loudly before doing any work. Notes the manifest-beta.json
auto-removal in the rules.
- .claude/agents/prerelease.md: documents the manifest.json / manifest-beta.json
split. Adds the same Step 0 drift assertion. Updates the git add step to
stage manifest-beta.json instead of manifest.json. Adds an explicit guard
rule that manifest.json must not change during a prerelease bump.
Verified locally by exercising version-bump.mjs in a throwaway repo:
- Stable 3.2.8 -> 3.2.9: writes manifest.json + versions.json, no beta side
- Prerelease 3.2.8 -> 3.2.9-beta.0: creates manifest-beta.json, manifest.json untouched
- Prerelease iteration 3.2.9-beta.0 -> 3.2.9-beta.1: updates manifest-beta.json,
manifest.json untouched
- Stable supersedes beta 3.2.9-beta.1 -> 3.2.9: writes manifest.json,
git-rm's manifest-beta.json
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* fix(release): address Codex review on PR #2429
1. release.yml: when verifying a prerelease, also assert that the committed
manifest.json on the merge commit still equals the latest non-prerelease
GitHub Release tag. This catches an old-style version bump or hand edit
that accidentally modified master's manifest.json — without this guard,
the verify step only inspected manifest-beta.json so a poisoned
manifest.json could merge unnoticed and break the Obsidian plugin store.
2. version-bump.mjs: don't fail stable bumps when manifest-beta.json exists
on disk but is untracked (or has local modifications). Check git
trackedness first; use `git rm -f` only when tracked, fall back to a
plain unlink for working-tree-only files. Logs which path it took.
Verified both scenarios in a throwaway repo:
- Untracked manifest-beta.json + stable bump: unlinked, no git rm error
- Tracked manifest-beta.json + stable bump: staged deletion via git rm -f
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* fix(release): use /releases/latest API for stable-drift guard
gh release list defaults to 30 results, so a cluster of 30+ prereleases
since the last stable would make the prerelease drift guard return empty
and fail valid prerelease publishes. Switch to GitHub's /releases/latest
endpoint instead, which by design returns only the most-recent
non-prerelease, non-draft release in a single call regardless of how
many prereleases have accumulated.
Applied the same fix to the Step 0 drift check in both the release agent
and prerelease agent doc for consistency.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
URGENT FIX. The prerelease agent's npm version run on PR #2427 left master's
manifest.json with version "3.2.9-beta.0", but Obsidian's community plugin
store reads manifest.json on master to determine which GitHub Release to
serve to new installers. With the stable release at 3.2.8 and master's
manifest pointing at a prerelease tag, plugin installs and updates were
breaking for users.
Fix:
- manifest.json on master: version back to "3.2.8" (the latest stable
release). minAppVersion 1.7.2 and isDesktopOnly:false are preserved.
- manifest-beta.json (new): full manifest for 3.2.9-beta.0 so BRAT and
similar beta-installer tools can locate the prerelease without us
poisoning master's stable manifest.
The longer-term fix (version-bump.mjs + prerelease agent updates so
this can't recur) follows in a separate PR.
Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* chore(release): add prerelease agent + workflow support, harden release agent pre-flight
- release.yml: accept prerelease semver in PR titles (X.Y.Z-tag.N) and
pass --prerelease to gh release create so prereleases are not offered
as stable updates via Obsidian's plugin browser.
- .claude/agents/release.md: add a Step 0 pre-flight (clean tree, full
project check, bundle size guard, manifest integrity, non-empty diff
since last tag) and explicit rules against silently changing
minAppVersion/isDesktopOnly or shipping with broken checks.
- .claude/agents/prerelease.md: new agent. Mirrors the stable release
flow but bumps via npm version prepatch/prerelease with --preid, emits
prerelease-shaped semver titles, and produces testing-focused release
notes with explicit "What to Test" and "How to Install" sections.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* fix(release): make prerelease regex match full SemVer 2.0.0 grammar
Previous pattern '^[0-9]+\.[0-9]+\.[0-9]+-[a-zA-Z0-9.]+$' rejected hyphens
inside prerelease identifiers, so valid SemVer titles like '3.2.9-beta-hotfix.1'
would fall through to the reject branch and the merged PR would silently skip
publishing a GitHub Release.
New pattern '^[0-9]+\.[0-9]+\.[0-9]+-[0-9A-Za-z-]+(\.[0-9A-Za-z-]+)*$' allows:
- hyphens within identifiers (e.g. beta-hotfix)
- dot-separated multiple identifiers
- single identifier (e.g. 3.2.9-alpha)
And still rejects malformed cases: '3.2.9-', '3.2.9-.1', '3.2.9-x.', '3.2.9-x..y'.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* fix(release): tighten prerelease title to require numeric counter
Previous pattern accepted any SemVer prerelease suffix, including bare
labels like '3.2.9-beta' or '3.2.9-feature' with no counter. That broadened
the workflow's release-trigger surface beyond the documented agent
contract of 'X.Y.Z-<tag>.<N>'.
New pattern '^[0-9]+\.[0-9]+\.[0-9]+-[0-9A-Za-z-]+\.[0-9]+$' requires
exactly one alphanumeric (hyphen-allowed) identifier followed by a
numeric counter, which is what 'npm version prepatch/prerelease --preid=...'
actually emits.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
- minAppVersion 1.4.0 -> 1.7.2: matches the actual Obsidian APIs the code
already calls (Workspace.revealLeaf needs 1.7.2; FileManager.processFrontMatter
needs 1.4.4). Users on Obsidian < 1.7.2 were already hitting runtime errors
on those code paths; stating an honest minimum prevents the install.
- Add isDesktopOnly: false: Chat, Vault QA, and Plus modes work on mobile
(extensive Platform.isMobile guards already in place across dbOperations,
vectorStoreManager, indexEventHandler, CopilotView, etc.).
Addresses two warnings from the Obsidian plugin preview check:
* "Manifest is missing optional but recommended field: isDesktopOnly"
* "'Workspace.revealLeaf' requires Obsidian v1.7.2, but minAppVersion is 1.4.0"
* "'FileManager.processFrontMatter' requires Obsidian v1.4.4, but minAppVersion is 1.4.0"
Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Removes the `no-restricted-imports: off` override so the obsidianmd recommended
rule applies (bans axios/node-fetch/moment/etc.). Replaces the two `moment(...)`
call sites in `src/utils.ts` with `luxon`'s `DateTime` (already a dep) and adds
unit tests covering local/UTC formatting, zero-padding, immutability, and the
invalid-input fallback. Integration tests get a per-file override since they
need `node-fetch` to polyfill jsdom fetch. `no-restricted-globals` stays
disabled — banning the global `app` will land in a separate PR.
Removes the "off" overrides for depend/ban-dependencies, configures the
package.json check with an allowed-list for deps we deliberately keep
(crypto-js, lodash.debounce, eslint-plugin-react, lint-staged, npm-run-all),
and drops three unused direct deps: axios, builtin-modules, and node-fetch
(plus @types/node-fetch). The integration-test fetch polyfills are no longer
needed in Node 18+ jsdom.
Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Removes the deferred "off" overrides from eslint.config.mjs so the
obsidianmd recommended config's "error" severity applies. Scopes
import/no-nodejs-modules off for test setup, mocks, and Node-context
build scripts (e.g. jest.setup.js polyfills TextEncoder/TextDecoder
from node:util). import/no-extraneous-dependencies is now enabled
everywhere; declared js-yaml, dotenv, and @jest/globals as explicit
devDependencies so transitive uses pass.
Fixes the one real violation in src/utils.ts by migrating two moment
call sites (formatDateTime, stringToFormattedDateTime) to Luxon, which
is already in dependencies. Adds 10 unit tests covering UTC/local
formatting, zero-padding, round-tripping, and invalid-input fallback.
Remove the explicit "off" overrides in eslint.config.mjs so the rules
from obsidianmd.configs.recommended apply. Add a scoped disable for the
sole violation in scripts/printPromptDebug.js, where the dynamic import
targets a controlled path under os.tmpdir() produced by esbuild.
Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Group the 14 disabled @typescript-eslint rules into Heavy / Medium / Quick-win
buckets with inline violation counts measured against src/**/*.{ts,tsx}. Makes
it obvious which rules (e.g. restrict-template-expressions at 1, no-base-to-string
at 7) are cheap follow-ups vs. the no-unsafe-* family that dominates the block.
Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Drops the `off` override so the rule (set to `error` by obsidianmd's
recommended config) is enforced. Fixes the one production violation in
markdown-preview by using `replaceChildren()` instead of clearing via
`innerHTML`. Rewrites test fixtures to use `DOMParser` and mock impls
to use `textContent` so the rule is on for tests too.
Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* chore(lint): enable obsidianmd/rule-custom-message and fix violations
Turn on the obsidianmd/rule-custom-message ESLint rule (which wraps no-console
to enforce logInfo/logWarn/logError over console.log per AGENTS.md). Swap
console.log → logInfo across LLMProviders/ and search/, and delete console.log
noise from test files.
src/logger.ts, src/chainFactory.ts (circular import with constants), and
scripts/** are exempted via narrow file overrides.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* chore(lint): enable obsidianmd/no-unsupported-api (#2414)
Removes the deferred "off" override so the rule runs at the recommended
severity on .ts/.tsx. With manifest.minAppVersion=1.4.0 and obsidian@1.2.5
(no @since tags) the rule fires on nothing today — it acts as a guard for
future obsidian type-stub bumps.
Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* chore(lint): enable obsidianmd/object-assign rule (#2415)
Follow-up to #2410. The rule only flags `Object.assign(<ident-containing-default>, <non-object-literal>)` — the Obsidian anti-pattern of mutating DEFAULT_SETTINGS. No call sites in this repo match, so enabling produces zero new errors.
Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* refactor: extract ChainType into src/chainType.ts to break import cycle
constants.ts → chainFactory.ts → logger.ts → settings/model.ts → constants.ts
formed a runtime cycle because chainFactory.ts (a heavy LangChain module) re-
exported the ChainType enum that constants.ts needed at module load. Move the
enum into a tiny standalone file, update all 26 importers, and restore
logInfo in chainFactory.ts. The eslint override for chainFactory.ts is no
longer needed.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Follow-up to #2410. The rule only flags `Object.assign(<ident-containing-default>, <non-object-literal>)` — the Obsidian anti-pattern of mutating DEFAULT_SETTINGS. No call sites in this repo match, so enabling produces zero new errors.
Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Removes the deferred "off" override so the rule runs at the recommended
severity on .ts/.tsx. With manifest.minAppVersion=1.4.0 and obsidian@1.2.5
(no @since tags) the rule fires on nothing today — it acts as a guard for
future obsidian type-stub bumps.
Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Removes the deferred `"off"` override and rewrites the 55 violations
(all in test/mocks/Node-script files) to use `window.*` instead of
`global.*`. Production code was already cleaned up in #2401.
- 18 test files + __mocks__/obsidian.js + jest.setup.js: mechanical
`global.X` → `window.X` (Jest runs under jsdom, so window === globalThis)
- scripts/printPromptDebugEntry.ts: inline eslint-disable — Node-only
debug script with no window available
Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Flip the rule from "off" to "error" to catch accidental references to
the global `document` (which always points at the main window even when
the user is interacting with a popout).
The only flagged site was a `document` field on the `RelevantNoteEntry`
type — not a DOM reference, but the rule pattern-matches the identifier.
Rename it to `note` so `entry.note.path/title` reads naturally next to
`entry.metadata.*`, and remove the suppression.
Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Replace `app.vault.getFiles().find(f => f.path === id)` lookups in the
Project Context Manage modal with `getAbstractFileByPath` + `instanceof
TFile` narrowing — direct hash lookup vs full-vault scan.
Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
- CONTRIBUTING.md: new "Commit Signing" section with the 4-step SSH
signing setup so future protection rules requiring signed commits
can be turned on with team-ready instructions in place.
- README.md: remove the abbreviated ToC, promote the comprehensive
one to the top, drop the self-referencing entry.
Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* fix(vault): respect user trash preference via FileManager.trashFile (W7/9)
Eighth of nine workspaces splitting #2397. Intentional behavior change:
file deletions now honor the user's Settings -> Files and links ->
Deleted files preference (system trash / .trash / permanent), instead
of unconditionally calling vault.delete(file, true).
- src/utils/vaultAdapterUtils.ts: add trashFile(app, file) helper that
casts past the missing app.fileManager.trashFile typedef.
createSyntheticTFile now uses Object.create(TFile.prototype) so
instanceof TFile returns true.
- src/projects/ProjectFileManager.ts: constructor takes App instead
of Vault; all vault.delete(file, true) -> trashFile(app, file).
Project create rollback now trashes partial files (was permanent-
delete before).
- src/projects/projectRegister.ts: constructor takes App.
- src/projects/projectMigration.ts: rollbackCreatedFile(app, ...),
ensureProjectsMigratedIfNeeded(app), all delete sites use trashFile.
- src/main.ts: deleteChatHistory uses trashFile; ProjectRegister
constructed with this.app.
- src/system-prompts/systemPromptManager.ts: prompt delete via trashFile.
- src/commands/customCommandManager.ts: command delete via trashFile.
- src/LLMProviders/projectManager.ts, src/components/Chat.tsx,
src/components/chat-components/ProjectList.tsx: pass app (not vault)
to ProjectFileManager.getInstance.
- __mocks__/obsidian.js: app.fileManager.trashFile jest mock.
- Tests updated to expect trashFile + App constructor arg.
W0, W1, and #2402 already merged. Behavior change requires manual
verification (test plan section 1 + 6).
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
* chore(manifest): bump minAppVersion to 1.4.0
FileManager.trashFile (used by the vault deletion helper introduced in
W7/9) requires Obsidian 1.4+. Bumping minAppVersion ensures the plugin
store only offers this and future releases to compatible app versions;
users on older Obsidian builds stay on their current Copilot version
instead of receiving an update that would crash on delete.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 4.7 <noreply@anthropic.com>
Seventh of nine workspaces splitting #2397. Picks up provider-file
changes W1 deferred — primarily LangChain public-API renames that
aren't pure type changes.
- src/LLMProviders/BedrockChatModel.ts: _getType() -> getType()
(non-deprecated method, same behavior)
- src/LLMProviders/BedrockChatModel.test.ts: matching mock-message
rename across all streaming-decode tests
- src/LLMProviders/chainRunner/CopilotPlusChainRunner.ts: stringify
non-string response.content via JSON.stringify instead of String()
(avoids [object Object] when content is structured)
No semantic message-handling changes. All behavior preserved.
W0, W1, and #2402 already merged.
Co-authored-by: Claude Opus 4.7 <noreply@anthropic.com>
* chore(styles): replace element.style with Tailwind + CSS vars (W5/9)
Sixth of nine workspaces splitting #2397. Replaces direct DOM
inline-style writes with Tailwind utility classes (addClass) and
CSS custom properties for dynamic values.
- src/components/modals/SourcesModal.tsx — addClass + tw-* classes
- src/components/modals/AddImageModal.tsx — input.style.display -> tw-hidden
- src/components/modals/CachePreviewModal.ts — modalEl.addClass tw-w/tw-max-w
- src/components/modals/project/AddProjectModal.tsx — !tw-max-h-[85vh]
- src/components/modals/project/context-manage-modal.tsx — tw-min-w-[50vw]
- src/components/quick-ask/QuickAskOverlay.tsx — body cursor/select via tw + CSS var
- src/components/ui/textarea.tsx — --copilot-autosize-height + tw-h-[var(...)]
- src/components/chat-components/ChatViewLayout.ts — removeProperty fix
- src/components/command-ui/draggable-modal.tsx — tw-left/top-[var(--copilot-drag-x/y,0px)]
- src/settings/SettingsPage.tsx — tw-select-text
- src/settings/v2/components/ModelEditDialog.tsx — tw-h-4/5
- src/system-prompts/SystemPromptAddModal.tsx — tw-h-4/5
- src/hooks/use-draggable.ts and use-resizable.ts: write
--copilot-drag-x/y and --copilot-resize-cursor CSS variables instead
of inline left/top/cursor; consumers reference them via Tailwind
arbitrary-value classes
Behavior: visual-only changes; no logic changes. Reviewer should
walk through SCORECARD_WARNINGS_TEST_PLAN.md §3 (drag/resize, QuickAsk
positioning, draggable modal, typeahead positioning, inline pills).
W0 already merged.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
* chore(styles): use Obsidian setCssProps API for CSS variable writes
Follow-up to the W5 commit. The Obsidian scorecard warning is
"Use the setCssProps function if the CSS properties need to change
dynamically", and the prior pass still wrote them via
element.style.setProperty / removeProperty. Routes the remaining 14
sites across 6 files through HTMLElement#setCssProps, which is the
Obsidian-sanctioned API for writing CSS custom properties.
- src/hooks/use-draggable.ts: --copilot-drag-x/y batched into one call
- src/hooks/use-resizable.ts: --copilot-resize-cursor set/clear
- src/components/quick-ask/QuickAskOverlay.tsx: same cursor pattern
- src/components/chat-components/ChatViewLayout.ts: --copilot-status-bar-clearance
- src/components/ui/textarea.tsx: --copilot-autosize-height
- src/utils/dom/dynamicStyleManager.ts: batches every set+clear into a single
setCssProps call per invocation (CSSOM defines setProperty(name, "") to
call removeProperty(name), so empty-string entries clear)
The single getPropertyValue read at src/components/CopilotView.tsx:112 is
a read, not a write, and is not flagged by the scorecard rule.
Verification: grep -rn "\.style\.setProperty\|\.style\.removeProperty" src
returns zero hits. npm run lint, npm run build, npm test (1962 tests) all
pass.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 4.7 <noreply@anthropic.com>
* fix(popout): document ownership and selection listener fixes (W4/9)
Fifth of nine workspaces splitting #2397. Real popout-window
correctness fixes — semantic choices about which window owns a DOM
operation, plus selection-listener captured-document fixes.
- document → activeDocument where a generic active-window lookup is
correct (SourcesModal, ChatViewLayout, ItemView/Notice mocks,
TabContext, ChatInput keydown listener, CopilotView keyboard
observer, TypeaheadMenuPortal, AddImageModal, and ?? document
fallbacks across draggable-modal, menu-command-modal,
CustomCommandChatModal, use-draggable, use-resizable, and
QuickAskOverlay).
- document → root.ownerDocument in ChatSingleMessage's
linkInlineCitations so DOM nodes end up in the citation's window;
same pattern in the processMessage streaming block via
contentRef.current.ownerDocument.
- New getEditorDocument(editor) helper in BasePillNode. createDOM and
exportDOM now accept the LexicalEditor; pills create DOM in the
editor's window (correct popout behavior). All 6 pill subclasses
updated to match the new signature.
- main.ts: selectionListenerDocument captured at listener registration;
remove uses the same document (fixes leak when activeDocument has
drifted before unload).
- chatSelectionHighlightController.ts: getActiveViewOfType(MarkdownView)
replaces the brittle activeLeaf type comparison.
- Test setup: (window as any).activeDocument = window.document in
ChatSingleMessage.test; window.document.createElement in
AgentPrompt.test Notice mock.
Behavior fix: chat in a popout window now creates DOM nodes in the
popout's document, not the focused window's document.
W0, W1, and #2402 already merged.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
* fix(popout): prefer element.doc/.win over ownerDocument and activeDocument
Switch popout-window-sensitive call sites to Obsidian's `.doc` / `.win`
augmentations so DOM ops, listeners, portals, and positioning follow the
element's actual owner window. Recreate Lexical root on view migration to
rebind input handling after a leaf moves between windows. Document the
decision order in AGENTS.md and polyfill `Node.doc` / `Node.win` in
jest.setup.js so tests still run under jsdom.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* improve image upload implementation
---------
Co-authored-by: Claude Opus 4.7 <noreply@anthropic.com>
Third of nine workspaces splitting #2397. Surfaces previously
swallowed promise rejections via .catch(logError) or `void` prefix.
No logic changes - only error-handling explicitness.
- ~67 floating promises now use void / .catch(logError)
- Promise-returning callbacks where void was expected are wrapped
- ConfirmModal accepts onConfirm/onCancel returning void | Promise<void>;
rejections logged via logError
- Custom command runners wrap `result instanceof Promise` paths
- ChatUIState.replaceMessages is now async to match its delegate
- ChainManager initialize / createChainWithNewModel are explicitly
voided with .catch(logError) at call sites
- dbOperations subscribeToSettingsChange uses void IIFE with try/catch
Behavior change: errors previously hidden by floating promises now
log via @/logger. No code paths changed.
W0, W1, and #2402 already merged.
Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Replace ChatCohere / ChatMistralAI / CohereEmbeddings with ChatOpenAI /
OpenAIEmbeddings pointed at each provider's OpenAI-compatible endpoint:
- Cohere: https://api.cohere.ai/compatibility/v1
- Mistral: https://api.mistral.ai/v1 (already OpenAI-shaped)
Dropping @langchain/cohere, @langchain/mistralai, cohere-ai, and
@mistralai/mistralai also eliminates the @aws-sdk/* and @smithy/* chain
that cohere-ai dragged in for Bedrock auth (a path this plugin doesn't use).
Production bundle: 5.21MB -> 3.39MB.
Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
First of nine workspaces splitting #2397 into focused PRs (see
designdocs/todo/SCORECARD_WARNINGS.md, landing in a later workspace).
This PR is deps-only so the source-touching workspaces (types,
promise safety, popout compat, styles, trash semantics, brevilabs
rewrite, eslint rules) can each land independently.
Dependencies:
- @codemirror/state, @codemirror/view: peer deps for newer @lexical/* usage
- @langchain/classic: required by langchain ^1 split layout
- openai: direct consumer in upcoming provider work
- uuid + @types/uuid: replaces ad-hoc id generation
- zod: schema validation used in upcoming type tightening
DevDependencies:
- eslint-plugin-obsidianmd: installed now but not wired into .eslintrc
until the final workspace (W9) once all flagged sites are cleaned up
No source changes; no behavior changes. npm run lint / build / test
all pass on this branch.
