wiseguru_ReWrite-Voice-Notes/docs/DEVCONFLICTS.md
WiseGuru 42f0a43c62 Resolve the 1.2.1 review-bot no-unsafe-* warnings at the root (type parity)
All ~30 warnings traced to the bot's lint environment resolving types
differently from local, not to unsafe code (DEVCONFLICTS.md finding 10):

- tsconfig lib was ES2016 while the code uses ES2017-ES2019 APIs
  (Object.entries/values/fromEntries, padStart, Promise.finally); the
  test suite's auto-included @types/node masked it locally. lib is now
  ["DOM", "ES2019"] with types: [] so a mismatch fails npm run build.
- moment's typings don't resolve in the bot's environment (obsidian's
  re-export types via the moment package). All calls now go through
  formatMoment in src/time.ts, a narrow structural alias.
- secrets.ts needed as-BufferSource assertions on TS 5.7+ that older TS
  flags as unnecessary; restructured (inferred ArrayBuffer-backed byte
  helpers, BufferSource param, one 32-byte copy) to need none on any
  TS version. Crypto behavior unchanged.
- The two flagged as-Record casts became an isRecord type predicate.

Docs: CLAUDE.md "Review-bot type environment" gotcha, RELEASING.md
parity section (plus the pre-release docs from the workflow change),
DEVCONFLICTS.md finding 10, ROADMAP.md Unreleased entry.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
2026-07-09 20:55:52 -07:00

13 KiB
Raw Permalink Blame History

Developer-guideline conflicts and potential problems

A review of the ReWrite plugin against Obsidian's official developer documentation:

Scope: this document identifies conflicts and potential problems. Severity labels are this reviewer's estimate of how likely each item is to matter during community-plugin review. Each finding now carries a Resolution line recording how it was handled (fixed, or accepted as correct/intentional with rationale).


High: likely to be flagged in submission review

LICENSE reads Copyright (C) 2020-2025 by Dynalist Inc. — this is the verbatim license shipped with obsidian-sample-plugin, never updated to the actual author.

  • Developer policy: plugins must "Include a LICENSE file and clearly indicate the license" and respect copyright / credit incorporated code.
  • The license body (0BSD-style permissive grant) is fine, but the copyright holder is wrong, which is a copyright-attribution problem, not just cosmetic.

Resolution: Fixed. LICENSE now reads Copyright (C) 2026 WiseGuru; the 0BSD grant body is kept verbatim (license choice confirmed: 0BSD's only trade-off vs MIT is no mandatory attribution, which is acceptable here).

2. Node.js APIs used while isDesktopOnly: false

manifest.json sets isDesktopOnly: false, but the plugin imports Node built-ins (child_process, net, fs, process) in src/whisper-host.ts.

  • Submission requirement: "If using Node.js packages like fs, crypto, or os, the plugin must be marked as desktop-only."
  • The plugin does this deliberately and defensibly: the Node modules are lazy-required only inside Platform.isDesktop guards (the local whisper.cpp host is a desktop-only feature), so mobile never touches them. This is the accepted pattern for a mixed desktop/mobile plugin.
  • Still flag-worthy: automated review and human reviewers frequently catch Node imports in non-desktop-only manifests. Expect to have to justify it. The casts in getNodeApi() (src/whisper-host.ts, window.require / window.process) are part of the same pattern.

Resolution: Accepted as-is. isDesktopOnly stays false so mobile keeps the cloud-provider features; Node modules are lazy-required only inside Platform.isDesktop guards (the local whisper.cpp host), so mobile never loads them. The README already discloses the desktop-only host. This is the documented pattern for a mixed desktop/mobile plugin; be ready to explain it in review.

3. package.json license identifier is not a valid SPDX id

package.json line 14 declares "license": "0-BSD". The SPDX identifier is 0BSD (no hyphen). Tooling that validates SPDX will not recognize 0-BSD.

Resolution: Fixed. package.json now declares "license": "0BSD".


Medium: code-guideline deviations

4. Vault.modify() used instead of Vault.process() for read-modify-write

Guideline: "Use Vault.process instead of Vault.modify" for background edits (atomic), and "Use the Editor interface instead of Vault.modify() to the active file."

  • src/insert.ts:59-62insertAppend reads the file then vault.modifys it. This is a non-atomic read-modify-write, and the target may be the active file (it falls back to the active view's file), where the guideline prefers the Editor interface to preserve cursor/selection.
  • src/templates-folder.ts:255vault.modify during the template-update reconcile.
  • src/template-guide.ts:249vault.modify when rewriting the guide.

The latter two are background, non-active-file writes; Vault.process would be the recommended atomic form.

Resolution: Fixed. All three sites now use Vault.process(file, (data) => ...): insert.ts insertAppend computes the separator from the callback's data (and no longer does a separate vault.read), templates-folder.ts uses process(child, () => rendered), and template-guide.ts uses process(existing, () => content). Behavior is unchanged.

5. app.vault.adapter used instead of app.vault

Guideline: prefer app.vault over app.vault.adapter (caching + serialized, safer operations).

Both touch config/sidecar files that are intentionally not regular vault notes (e.g. the .nosync secrets envelope), so direct adapter access is arguably justified — but it is still a documented deviation a reviewer may question.

Resolution: Accepted as correct (reclassified). Both paths resolve under plugin.manifest.dir (.obsidian/plugins/rewrite-voice-notes/), which is the plugin config directory, not vault note content. The app.vault TFile API does not address files there; app.vault.adapter is the appropriate API for plugin-config files. No change.

6. Use of undocumented / private Obsidian internals

The guidelines steer away from relying on internals not in the public API (they can change without notice). The plugin reaches several via as unknown as casts:

  • src/ui/quick-record.ts:189app.hotkeyManager (not in public typings).
  • src/ui/modal.ts:507 — cast on this.app to reach an internal.
  • src/secrets.ts:111app.secretStorage. This one is now a GA public API (1.11.4) but predates the installed typings, so it is reached via a cast; lower risk than the others but worth listing.

These are localized behind narrow interfaces (the documented pattern for when a cast is unavoidable), but they remain private-API dependencies that can break on an Obsidian update.

Resolution: Accepted as-is. Each access is already isolated behind a narrow local interface + cast, which is the recommended mitigation when a needed API is absent from the public typings. secretStorage is a GA public API the bundled typings simply predate. Left unchanged; revisit if a future typings update makes any of these first-class.

7. Editor-dependent commands use callback instead of editorCheckCallback

Guideline: "Use editorCallback or editorCheckCallback for commands requiring an active Markdown editor."

  • src/main.ts:80-86process-text requires an active Markdown editor/selection but is registered with a plain callback and checks for an editor internally (showing a Notice when absent). Using editorCheckCallback would let Obsidian hide the command when no editor is active, which is the recommended behavior. (The whisper start/stop commands already use checkCallback correctly.)

Resolution: Accepted as intentional (reclassified). processTextWithTemplate does not strictly require an editor: when none is active it shows a guiding Notice ("Open a Markdown note or select text"). Switching to editorCheckCallback would hide the command entirely in that state and remove that fallback. Kept as callback by design.


Low: minor / conventional

8. manifest.json missing author / authorUrl

manifest.json has no author or authorUrl fields. Not strictly required, but conventional and present in the sample manifest; pairs with finding #1 (the stale LICENSE copyright).

Resolution: Fixed. manifest.json now sets author (WiseGuru), authorUrl (https://github.com/WiseGuru), and fundingUrl (https://wyz.guru/buy-someone-else-coffee/).

9. app.vault.getFiles() full-vault scan

src/ui/audio-source.ts:18 iterates all vault files to filter audio by extension. The guideline's "don't iterate all files" advice is specifically about finding a file by path (use getFileByPath), which this is not — there is no extension index in the API, so a scan is reasonable. Listed only for completeness; not a real conflict.

10. 1.2.1 automated-review no-unsafe-* warnings (type-environment mismatch)

The 1.2.1 submission drew ~30 warnings (no-unsafe-assignment/call/member-access/argument/return, no-unnecessary-type-assertion) that local npm run lint did not reproduce even after the rule mirror (eslint.config.mts) was in place. Investigation showed none of the flagged code was genuinely unsafe; every warning came from the bot's type environment differing from local. The bot's typed lint uses the repo's own tsconfig.json but not the repo's full node_modules, so a value that types cleanly locally can be error-typed there (and an error-typed value trips no-unsafe-* at every use). Three sub-causes:

  1. Declared lib was lower than the APIs the code uses. tsconfig.json declared lib ES2016 while the source uses Object.entries/values/fromEntries, String.padStart, and Promise.finally (ES2017ES2019). Locally, tsc auto-included the test suite's @types/node, which silently supplied those types; in the bot's environment they were error-typed (~20 of the warnings).
  2. moment's types don't resolve in the bot's environment. obsidian itself types fine there, but its moment re-export's type comes from the moment package (a transitive dependency), which the bot doesn't have — every direct moment(...) call was error-typed (5 warnings across src/audio-persist.ts, src/insert.ts, src/template-guide.ts).
  3. TS-version-dependent assertions. On TS 5.7+ (local, 5.8) a value declared Uint8Array is ArrayBufferLike-backed and needs as BufferSource to satisfy WebCrypto; on the bot's older TS the same assertion is "unnecessary" (src/secrets.ts).

Resolution: Fixed (all three). tsconfig.json now declares lib: ["DOM", "ES2019"] with types: [] (so the @types/node masking can never recur — a lib/API mismatch now fails npm run build locally) and skipLibCheck. All moment calls go through formatMoment in src/time.ts, a narrow structural alias over Obsidian's bundled moment (same pattern as ScriptProcessorNodeLike). The secrets byte helpers were restructured to need no BufferSource assertion under any TS version (inferred ArrayBuffer-backed return types, a BufferSource param, one 32-byte defensive copy of the hash-wasm output), and the two as Record<string, unknown> narrows the bot flagged were replaced with an isRecord type predicate.


Checked and clean (no conflict found)

For the record, these commonly-flagged items were checked and are compliant:

  • DOM safety: no innerHTML / outerHTML / insertAdjacentHTML anywhere; DOM is built with createEl/createDiv/createSpan.
  • No hardcoded styling in JS: no el.style / inline-style assignments; all styling lives in styles.css and uses Obsidian CSS variables (--text-muted, --background-modifier-border, etc.).
  • App instance: uses this.app throughout; no window.app / global app.
  • No var; const/let only. async/await used over raw Promises.
  • No default hotkeys set on any command.
  • normalizePath is used consistently for user-supplied paths.
  • Resource cleanup: registerEvent / registerInterval used; the one manual DOM element (Quick Record floater) is torn down in onunload.
  • Settings headings: routed through a setHeading() helper, no manual <h2>, no top-level "Settings"/plugin-name heading.
  • Sentence case in UI text and command names; the codebase carries no eslint-disable directives (the former sentence-case exemption for a random-string example in src/ui/passphrase-modal.ts was replaced by passing the string through a variable, which the rule does not inspect).
  • Manifest description: action-focused, ends with a period, under 250 characters, no emoji.
  • Network-use disclosure (developer policy): the README states keys are user-supplied, "Nothing is sent to a ReWrite server," and lists every provider endpoint.
  • No telemetry, no ads, no self-update mechanism, no code obfuscation (developer policy prohibitions).
  • Frontmatter modified via FileManager.processFrontMatter (src/insert.ts:94), not manual YAML editing of the note.