Skip to content
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions .github/copilot-instructions.md
Original file line number Diff line number Diff line change
Expand Up @@ -111,7 +111,7 @@ it landed as a shock to the whole team
- ID: 8-character lowercase base36 string.
- Anchor text: the SELECTION capture caps at 80 visible characters, mid-truncated with `…`. `serialize()` enforces a separate 200-character ceiling on the escaped value, and the parser accepts any length so an anchor written by hand or by an older build is read rather than silently dropped. Two limits, two jobs: 80 is the capture budget, 200 is the format's.
- Replies are sorted chronologically by timestamp on read; file order is the tiebreak for equal timestamps.
- The `-->` sequence cannot appear literally in any free-text field, because it closes the HTML comment wrapper. `parser.ts` escapes it as `--\>` on write and restores it on read, for the body, `[anchor=...]`, reply bodies, the addressed and resolved notes, and the `annoteca-original` fence. Anything that writes a marker must go through `serialize()`; do not hand-build one.
- Neither `<!--` nor `-->` can appear literally in any free-text field, because `<!--` opens and `-->` closes the HTML comment wrapper. `parser.ts` escapes them as `\<!--` (`escapeOpener`) and `--\>` (`escapeTerminator`) on write and restores them on read, for the body, `[anchor=...]`, reply bodies, the addressed and resolved notes, and the `annoteca-original` fence. The opener escape is what lets the plugin tell a comment quoting the marker syntax from one that lost its terminator: an unescaped `<!--` inside a marker's text is read as a marker that never closed. Anything that writes a marker must go through `serialize()`; do not hand-build one.
- Every bracketed trailing line is a SINGLE-LINE grammar, so `serialize()` collapses a run of line breaks to one space in `[anchor=...]`, reply bodies, and the addressed and resolved notes. A raw newline in one of those emits a continuation line that matches nothing, which ends the parser's backward walk on the spot and absorbs the `[id=...]` and every other trailing line into the body. The comment body and the `annoteca-original` fence are multi-line by contract and are NOT collapsed. `skill-export.ts` states this rule for assistants; it applies to plugin code too.
- The `annoteca-original` fence is the addressed note's original when it appears anywhere after the last `[addressed ...]` line that is not itself inside a fence. Adjacency is NOT required and must not be reintroduced: `serialize()` writes the fence on the next line, but hand-written and assistant-written markers put a blank line or a `[reply ...]` in between, and requiring adjacency leaves the fence in place to stop the backward walk, which destroys the whole comment.
- Forward-compatibility: a trailing line in a shape the format defines but this version does not recognize is ignored by the parser. That means exactly two shapes, `[key=value]` alone on the line, and `[key <author> <timestamp>]:`, where key is `[a-z][a-z0-9-]*`. **Not** "any line starting with `[`": bodies are markdown, and `[label](url)`, `[ref]: url`, `[^1]: note` and `[[Wikilink]]` all begin with a bracket. Treating those as structured deleted the last line of the body. When a trailing line is ambiguous, keep it in the body: lost prose is unrecoverable, an unparsed structured line is merely visible.
Expand Down Expand Up @@ -230,7 +230,7 @@ Releases are triggered by pushing a version tag. The release workflow:

- **Marker format changes must be reflected in `parser.ts`** (the source of truth). Updating the format in one place and not the other will cause silent data loss or parse failures.
- **Category name rules are strict.** An invalid category name silently fails to match the parser regex and markers with that category will not be indexed.
- **`-->` is escaped by `serialize()`, not forbidden.** Every free-text field round-trips through `escapeTerminator` / `unescapeTerminator` in `parser.ts`. Do not add a caller-side check that rejects `-->`, and do not write a marker without going through `serialize()`.
- **`<!--` and `-->` are escaped by `serialize()`, not forbidden.** Every free-text field round-trips through `escapeOpener` and `escapeTerminator` (and their unescape twins) in `parser.ts`. Do not add a caller-side check that rejects either sequence, and do not write a marker without going through `serialize()`. An unescaped `<!--` in a body is read as evidence of a marker that never closed, not as prose, so a comment written before the opener escape existed whose text quotes the format reads back under the quoted category; the plugin reports that as a broken marker rather than rewriting the note.
- **Stale marker snapshots, not just stale positions.** A panel card holds the `Comment` captured when it was drawn. Re-resolving the marker's `id` for its OFFSETS is necessary but not sufficient: building the write from the cached object serializes its replies, author, timestamps, and addressed state as of render time, silently discarding anything that landed in between. `replaceMarker` in `comment-service.ts` therefore takes a TRANSITION (`current => next`) applied to the freshly re-read comment, and returns whether it wrote so the caller can pick the right Notice. Add new lifecycle verbs that way; do not reintroduce a `next`-shaped parameter.
- **An `Editor` does not belong to a file, so re-check the file at WRITE time.** Obsidian reuses the same `Editor` object when a markdown leaf switches file, and the stale reference then reads and writes the NEW file's buffer. The default composer is the side panel, whose whole point is that the editor stays usable behind the form, so a wikilink click, a file-explorer click or Make a copy while it is open is ordinary rather than exotic. Any surface holding an `Editor` across user interaction therefore carries the view it came from (`ComposerRequest.view`, a required field on purpose) and compares `view.file?.path` to the captured path immediately before writing, refusing rather than redirecting. Open-time checks do not count; the one this replaced ran when the form opened and Save still wrote into whatever the tab had moved on to. Take the path from that view too, never from `getActiveFile()`, or the check has nothing it can agree with.
- **An id-less marker is identified by its own text, plus the card that asked for it.** The format allows a marker with no `[id=...]`, and `convert comments` emits them with no id, no date and no author, so a run of them differs only in body and two of equal body length have equal extent. Offset plus category plus extent is therefore not an identity: deleting a line above slides the next marker onto the remembered offset. Compare the marker's exact source text as captured when the form opened, AND the category and body of the snapshot that was clicked. Both, not either: the fingerprint is read using the snapshot's offsets, so on its own it agrees with itself whenever the snapshot was already stale. Two byte-identical markers stay indistinguishable, which is a placement cost and never a content one.
Expand All @@ -242,6 +242,6 @@ Releases are triggered by pushing a version tag. The release workflow:
- **Map a marker offset with `assoc: 1`.** `mapPos` defaults to `-1`, which keeps a position before text inserted exactly at it. A marker is not a cursor: typing at the end of the prose immediately in front of one puts those characters ahead of the `<!--`, so the marker starts after them and a default-association lookup misses by what was typed and reads as "deleted".
- **Anything the editor decorations read must be a declared CodeMirror dependency, or it will not repaint.** `EditorView.decorations.compute(deps, ...)` only re-runs when one of `deps` changes. A module-level flag or a value reached through `ctx.getSettings()` is invisible to that machinery, so changing it appears to do nothing until an unrelated edit or click recomputes the facet for some other reason. Focusing the editor is not enough. This has bitten three times: hide-all, inline comment bodies, and every editor-indicator setting. The pattern is in `decorations.ts`: mirror the value into a `StateField`, list that field in `deps`, and dispatch an effect into every view in `liveViews`. Settings are already covered wholesale by `refreshDecorationsEverywhere()`, which `saveSettings` calls.
- **A command must not re-derive the drawing code's conditions.** If a command needs to know whether pressing it would produce anything visible, ask a predicate that lives beside the drawing code (see `inlineBodiesBlockedBy` in `decorations.ts`). Enumerating the gates at the call site drifts the moment a new early return is added to `decorationsCompute`.
- **`npm run lint` runs `scripts/check-submission.mjs`** in addition to ESLint. That script checks manifest description constraints, `!important` in CSS, and ESLint directive hygiene. Read it before adding new lint suppressions.
- **`npm run lint` runs `scripts/check-submission.mjs`** in addition to ESLint. That script checks manifest description constraints, `!important` in CSS, ESLint directive hygiene, and regex lookbehind (`(?<=` / `(?<!`), which is a parse error in JavaScriptCore before iOS 16.4 and stops the plugin loading at all on those devices. It scans sources, not the built bundle. Read it before adding new lint suppressions or a lookbehind pattern.
- **`npm run lint` also runs `prettier --check "**/*.ts"`.** Formatting is a hard gate, so hand-formatted or scripted edits that Prettier would rewrite fail CI. Fix with `npx prettier --write`, never by editing `.prettierrc.json`. The scope is TypeScript only; `styles.css`, `manifest.json`, and the `.mjs` scripts are deliberately outside it.
- **The test vault auto-copy** in `esbuild.config.mjs` looks for `../../obs-test-vault`. It silently skips if absent — this is expected on CI and in most dev environments.
26 changes: 16 additions & 10 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -18,10 +18,10 @@ jobs:

steps:
- name: Checkout repository
uses: actions/checkout@v6
uses: actions/checkout@d23441a48e516b6c34aea4fa41551a30e30af803 # v6

- name: Setup Node.js
uses: actions/setup-node@v6
uses: actions/setup-node@249970729cb0ef3589644e2896645e5dc5ba9c38 # v6
with:
node-version: "22"
cache: "npm"
Expand Down Expand Up @@ -68,28 +68,34 @@ jobs:
- name: Check for deprecated Obsidian API usage
run: |
echo "Scanning for known deprecated Obsidian APIs..."
# Scans every source .ts file, not the main.ts entry point alone: a
# deprecated call in any of the other ~30 modules was previously
# invisible here. Sources rather than the built bundle, because the
# production bundle is minified and rewrites getLeaf(true) to
# getLeaf(!0), which the boolean-argument pattern below would miss.
SOURCES=$(ls *.ts | grep -v '\.d\.ts$')
FOUND=0

# workspace.activeLeaf — deprecated, use getActiveViewOfType()
if grep -n "\.activeLeaf\b" main.ts; then
if grep -n "\.activeLeaf\b" $SOURCES; then
echo "::error::Deprecated API: 'activeLeaf'. Use getActiveViewOfType() or getLeaf() instead."
FOUND=1
fi

# MarkdownRenderer.renderMarkdown — deprecated, use MarkdownRenderer.render()
if grep -n "renderMarkdown(" main.ts; then
if grep -n "renderMarkdown(" $SOURCES; then
echo "::error::Deprecated API: 'renderMarkdown'. Use MarkdownRenderer.render() instead."
FOUND=1
fi

# getLeaf() with a boolean argument — deprecated, use getLeaf('tab') etc.
if grep -n "getLeaf(true)\|getLeaf(false)" main.ts; then
if grep -n "getLeaf(true)\|getLeaf(false)" $SOURCES; then
echo "::error::Deprecated API: 'getLeaf(bool)'. Use getLeaf('tab') or getLeaf('split') instead."
FOUND=1
fi

# Notice.noticeEl — deprecated, use Notice.messageEl
if grep -n "\.noticeEl\b" main.ts; then
if grep -n "\.noticeEl\b" $SOURCES; then
echo "::error::Deprecated API: 'noticeEl'. Use 'messageEl' instead."
FOUND=1
fi
Expand All @@ -115,9 +121,9 @@ jobs:
contents: read
security-events: write
steps:
- uses: actions/checkout@v6
- uses: actions/checkout@d23441a48e516b6c34aea4fa41551a30e30af803 # v6
- name: Run OSV-Scanner
uses: google/osv-scanner-action/osv-scanner-action@v2.0.3
uses: google/osv-scanner-action/osv-scanner-action@40a8940a65eab1544a6af759e43d936201a131a2 # v2.0.3
with:
scan-args: |-
--lockfile=package-lock.json
Expand All @@ -131,8 +137,8 @@ jobs:
contents: read
pull-requests: write
steps:
- uses: actions/checkout@v6
- uses: actions/dependency-review-action@v4
- uses: actions/checkout@d23441a48e516b6c34aea4fa41551a30e30af803 # v6
- uses: actions/dependency-review-action@2031cfc080254a8a887f58cffee85186f0e49e48 # v4
with:
fail-on-severity: high
comment-summary-in-pr: on-failure
38 changes: 34 additions & 4 deletions .github/workflows/release.yml
Original file line number Diff line number Diff line change
Expand Up @@ -18,38 +18,47 @@ jobs:
runs-on: ubuntu-latest

steps:
- uses: actions/checkout@v6
- uses: actions/checkout@d23441a48e516b6c34aea4fa41551a30e30af803 # v6
Comment thread
coderabbitai[bot] marked this conversation as resolved.

- name: Use Node.js
uses: actions/setup-node@v6
uses: actions/setup-node@249970729cb0ef3589644e2896645e5dc5ba9c38 # v6
with:
node-version: "22"
cache: "npm"
Comment thread
coderabbitai[bot] marked this conversation as resolved.
Outdated

- name: Install dependencies
run: npm ci

- name: Lint
# The scorecard gate (eslint-plugin-obsidianmd, prettier, the submission
# pre-check). CI runs it on every PR, but a tag can be pushed at a commit
# CI never covered, so the release must not skip it and cut an unlinted
# build. Runs before Build because check-submission scans sources, and
# main.js does not exist yet.
run: npm run lint

- name: Test
run: npm test

- name: Build plugin
run: npm run build

- name: Generate artifact attestation
uses: actions/attest-build-provenance@v4
uses: actions/attest-build-provenance@4d101475d8b20a2381f78447822ac1eab6504dd8 # v4
with:
subject-path: |
main.js
manifest.json
styles.css

- name: VirusTotal scan of release artifacts
id: virustotal
# Best-effort: VirusTotal returns HTTP 409 ("resource already exists")
# when a byte-identical artifact was scanned before, and the action
# treats that as fatal. continue-on-error keeps a 409 (or any VirusTotal
# hiccup) from blocking the release; the scan still runs and reports.
continue-on-error: true
uses: crazy-max/ghaction-virustotal@v5
uses: crazy-max/ghaction-virustotal@936d8c5c00afe97d3d9a1af26d017cfdf26800a2 # v5
with:
vt_api_key: ${{ secrets.VT_API_KEY }}
files: |
Expand All @@ -69,6 +78,27 @@ jobs:
echo "Extracted release notes:"
cat release-notes.md

- name: Append VirusTotal links to release notes
if: steps.virustotal.outputs.analysis != ''
# Via env, not inline ${{ }} in the script, so the action's output cannot
# break out of the shell.
env:
VT_ANALYSIS: ${{ steps.virustotal.outputs.analysis }}
run: |
# The scan runs before the release is created, so without capturing its
# output the analysis URLs lived only in the workflow log. Fold them into
# the notes so the release itself records where each artifact was
# scanned. Only when the CHANGELOG produced notes: an empty file means
# Create release falls back to --generate-notes, and appending here would
# both defeat that and leave a notes-only-VirusTotal release.
[ -s release-notes.md ] || exit 0
Comment thread
coderabbitai[bot] marked this conversation as resolved.
Outdated
{
printf '\n## VirusTotal\n\n'
printf '%s' "$VT_ANALYSIS" | tr ',' '\n' | while IFS='=' read -r name url; do
[ -n "$url" ] && printf -- '- [%s](%s)\n' "$name" "$url"
done
} >> release-notes.md

- name: Create release
env:
GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}
Expand Down
4 changes: 3 additions & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,9 @@ Why HTML comments?
4. When an assistant has addressed a comment, the hover popup offers **Accept**, **Revise**, or **Reject** (reject restores the original text). Otherwise resolve when done and reopen if it comes back, or use "Resolve and remove" to drop the marker instead of keeping it as history.
5. Navigate with "Next comment" / "Previous comment", which follow your current scope across files. The active comment is highlighted in the editor, and opening the panel keeps your place in the document.

Diagnostics commands cover the edge cases: find orphaned comments whose surrounding prose was deleted, detect markers that drifted, validate malformed markers, and back up or restore plugin settings.
Diagnostics commands cover the edge cases: find orphaned comments whose surrounding prose was deleted, clear orphaned stars whose comment no longer exists anywhere in the vault, detect markers that drifted, validate malformed markers, and back up or restore plugin settings.

Annoteca also warns you when a note has a broken comment marker, once per note as you open or save it. A marker that lost its closing tag hides the text after it without anything looking wrong, so the check runs on its own rather than waiting for you to go looking for it, and the notice says how to repair the marker.

## Working with AI assistants

Expand Down
4 changes: 2 additions & 2 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -40,8 +40,8 @@
"picomatch": "^2.3.2",
"flatted": "^3.4.0",
"ajv": "^6.14.0",
"brace-expansion@1": "^1.1.13",
"brace-expansion@2": "^2.0.3",
"brace-expansion@1": "^1.1.18",
"brace-expansion@2": "^2.1.4",
"js-yaml": "^4.3.1",
"@babel/core": "^7.29.6"
}
Expand Down
Loading