feat: report a QuickAdd failure once, to the right person, with the message that helps - #1617
Merged
Merged
Conversation
…s the user `toError(err)` with no context returns the CALLER's Error unchanged, and both suggesters then assigned to `err.message`. `renderSuggestion` runs once per visible row and re-runs on every keystroke, so a `renderItem` that rethrows one cached Error grew the prefix without bound - and because `log.logWarning` raises a Notice, it did so once per row per keypress. Measured live in the isolated vault: three items over three renders put NINE stacked notices on screen, the last carrying nine copies of "Custom renderItem threw an error; falling back to default rendering. " (#1604). Both halves fixed at one seam, `createRenderFallbackWarner`: the context goes to `toError`, which builds a fresh Error preserving name and stack - exactly what `toError`'s own docstring says it stopped doing for this reason - and the warner fires once per call site. One broken callback is one defect, and the fallback rendering it triggers is identical for every row. Also deletes the second `toError` in logManager. Its existence is why this happened: the two suggesters imported THAT one, a context-less twin, so the safe helper's "do NOT mutate the caller's Error" contract did not apply to the call sites that most needed it. One helper now. Refs #1604
One user-script failure raised TWO 15-second notices for one bug (#1601): QuickAdd: (ERROR) Failed to run user script probe.js: Cannot read properties… QuickAdd: (ERROR) Error executing choice probe-1601: Cannot read properties… MacroChoiceEngine reports and re-throws, and the command-palette handler reports again. Both layers are right to report - neither can know whether anything above it will - so "report once" now belongs to the function they both call. `reportError` remembers the values it has shown, keyed on identity rather than text, so the same failure through five layers reports once while two independent failures that happen to read alike still both report. It walks the `cause` chain, because not every layer re-throws the same instance: the AI request path reports the provider error and then throws a wrapper carrying it as `cause` (both its sites report through `reportError` now, so that pair collapses too). The user-script LOAD path had the same doubling and reached neither reporting layer - `getUserScript` throws straight past MacroChoiceEngine's catch - so a typo'd `require` stacked two notices carrying the same 300-character message. It reports through `reportError` now. Also fixes what the outermost handlers did with a dismissal. Escape on the one-page input modal raised, live: QuickAdd: (ERROR) Error executing choice onepage-s2: One-page input cancelled by user - a red 15-second notice for a deliberate Escape, which is exactly what PR #1606's first rule exists to prevent. `reportUnlessCancelled` is that rule at the three sites that catch a whole run: the command callback, the legacy URI path, and the picker. Those handlers now name the choice instead of its UUID, and the picker's two template-stringified `log.logError` calls became real reports - interpolating an error throws its stack away, and only a value passed through `reportError` can take part in report-once. Refs #1601
The reporter claimed a rejection if `plugin:quickadd:` appeared ANYWHERE in the stack. `Error.stack` is captured at construction with the whole live call stack, so a foreign caller's bug that merely ran through QuickAdd was claimed as ours - and `preventDefault` took away the console line naming the real culprit (#1602). Reproduced with a second plugin calling `quickadd.api.suggester(v => v.nope.trim(), items)` and floating it: TypeError: Cannot read properties of undefined (reading 'trim') at eval (plugin:probe-foreign:19:43) <- construction site: THEIRS at eval (plugin:quickadd:88:3004) at r.suggester (plugin:quickadd:88:2988) at Object.callback (plugin:probe-foreign:19:19) The topmost frame naming ANY plugin bundle now decides. The issue proposed requiring the first frame to be ours and expected worse false negatives; measured, there are none. An Error constructed inside Obsidian's own async plumbing (`vault.create` into a missing folder, awaited from a plugin) carries no plugin frame at all - not even the caller's - so "any frame" was never catching that class either: Error: ENOENT: no such file or directory, open '…/definitely/missing/folder/x.md' at async open (node:internal/original-fs/promises:636:25) The only thing "any frame" caught and this does not is a foreign frame above ours, which is the false positive itself. QuickAdd's own bug reached through another plugin still reports, verified live. Two details the shape depends on: - The `Name: message` header is stripped instead of filtering for `at ` frames. QuickAdd is `isDesktopOnly: false` and Obsidian mobile runs JavaScriptCore, whose frames are `fn@url:line:col` with no `at ` and no header - an `at `-keyed filter would have left this whole reporter dead on iOS with every desktop-shaped test still green. Stripping the header still stops an Error naming a plugin in its own message from dictating the blame. Both shapes are pinned by tests. - The plugin id is read without requiring a trailing `:line:col`, because an eval'd frame names the bundle as an origin with no position: `eval at exports.load (plugin:quickadd)` is a user script's own code and `eval at <anonymous> (plugin:dataview)` is a dataviewjs snippet. Those are exactly the frames attribution must not skip past. `plugin:quickadd-beta` still reads as a different plugin. `preventDefault` is now gated on a report actually going out, since `reportError` can drop a failure a lower layer already showed the user - claiming the event anyway would kill the browser's async trace and put nothing in its place, the opposite of the guarantee the repeat-window branch keeps. Refs #1602
On an interactive run - the seam whose entire premise is that nobody is at the
desktop - a genuine failure reached the client as:
{"kind":"error","error":"Choice execution failed; no file was created."}
while the actionable sentence (`Template file not found at path "templates/x.md".`)
went to a notice on a screen nobody was watching (#1603). The Template and Capture
engines report a failure and return WITHOUT recording an outcome, so
`executeWithOutcome` produced a reason-less `{status:"error"}` and the CLI
substituted a fixed sentence. The Macro branch already forwarded `error.message`;
only Template and Capture lost it.
`ChoiceOutcome`'s error variant now carries `reason`, exactly as the abort path
already does for `cancelKind:"aborted"`, and both CLI sites forward it - the
interactive bridge and `runResolvedChoice`, which serves `quickadd:run-template` and
`quickadd:run verify=…` and had the identical bug through a different command. The
URI x-callback handler keeps ignoring it on both variants, so no vault detail leaks
to an external callback URL.
The reason is recorded at EVERY failure exit, not just the top-level catch. Five of
the six were silent returns - a missing append-link destination, a target path
occupied by a non-markdown file, an unresolvable file-exists behaviour, a failed
create - each producing the same reason-less outcome without any exception, so
fixing only the catch would have left #1603 reproducible on the more common paths.
The fixed sentence is now a fallback for an outcome that carries nothing, which
nothing in the engines produces.
Two details worth naming:
- Success is recorded at each engine's commit point precisely so a post-commit link
or open-file failure cannot make an automation retry and duplicate the side effect.
That terminality lives in the recorder, not in `run()`: Capture commits from two
methods, and the canvas path keeps going into steps whose throws unwind into
`run()`'s catch.
- `createFileWithTemplate` reports the real cause and returns null, so its caller
knew only THAT creation failed. It now hands the cause up, and the caller records
it without logging a second, vaguer line.
Verified live in the isolated vault - a Template choice pointing at a missing
template, over real loopback HTTP and via the verified CLI path - both now answer
`Template file not found at path "templates/does-not-exist.md".`, and the desktop
shows one notice saying the same thing.
Refs #1603
…abort does
`promptProvider.ts` opens by claiming each method "returns exactly what its in-app
counterpart returns, so a script cannot tell it was driven remotely". `infoDialog`
broke that. `GenericInfoDialog` resolves on EVERY close path and has no reject path
at all, so it can never abort anything - but remotely, replying `{"cancelled":true}`
to an `info` prompt rejected with `UserCancelError` and ended the run. Escape is the
only gesture an info panel affords, and a client maps it to a cancel, so the same
choice that finishes in the app died remotely (#1605). Measured both ways in the
isolated vault before the fix: in-app Escape ran the macro to completion; the remote
cancel returned `{"kind":"error","error":"Input cancelled by user"}` with the
remaining steps never run.
#1574 documented this instead of changing it, because cancelling was a client's only
explicit way out mid-run. So the protocol gets a real one first:
POST /abort?session=…&token=… -> {"ok":true,"interrupted":<n>}
It rejects every pending prompt with `UserCancelError` and marks the session, so a
prompt raised afterwards rejects immediately rather than parking - a run that was
mid-work unwinds at its next prompt. It pushes no final event: the run unwinds
through the ordinary cancellation path and delivers its REAL outcome, which is more
truthful than a fabricated one. `interrupted` is there because the abort is not
omnipotent: a Template/Capture run still opens some prompts in Obsidian itself (the
file-exists chooser, the folder picker, the capture-target picker) which never travel
over this bridge, so `0` tells a client it stopped nothing. That limit is documented
at the wire rather than glossed.
The info exception is decided in `submitReply`, which already tracks each prompt's
type for reply validation - not in `RemotePromptProvider.infoDialog`. A provider-side
swallow cannot tell a per-prompt cancel from a session abort without a second error
class, and would leave `/abort` unable to end a run blocked on an info panel, the one
case it exists for.
The handshake now carries `"capabilities":["abort"]`. Both halves of this are
observable behaviour changes to a documented wire, and without a marker a client
could only feature-detect by string-matching a 404 body - an unknown path and an
unauthed session answer the same shape.
Verified over real loopback HTTP: info cancel -> `200`, panel closes, run finishes
`done`; `/abort` -> `200 {"interrupted":1}` then `error: Input cancelled by user`;
`/abort` on an ended session -> `409`; `GET /abort` -> `404`; bad token -> `404`;
browser `Origin` -> `403`.
Closes #1605
Adversarial review of the four commits above found three ways the report-once rule
picked the wrong survivor, and one way `/abort` misled its client.
**The AI request path lost its remedy.** `OpenAIRequest` reported the raw provider
error and then threw a wrapper carrying the provider name and QuickAdd's own
context-window guidance ("shorten it, choose a model with a larger context, or use the
chunked AI prompt API"). Report-once then dropped the wrapper as an already-reported
cause, so a context-overflow failure showed only the bare provider string - the least
informative half of a pair that used to show both. It now reports the WRAPPER, whose
message is a strict superset, and the test asserts the logged message rather than a
call count.
**A macro failure did not say which macro.** The surviving report is the innermost
one, so it has to carry enough context on its own. `MacroChoiceEngine` now names the
choice alongside the script. That matters most where the outer handler is not there to
help: a run-on-startup macro fails with no user action to correlate it with, and
"Failed to run user script sync.js" alone does not say which of two startup macros
broke.
**#1603 was only fixed for one file-exists branch.** `lastTemplateFileFailure` was set
by `createFileWithTemplate` alone, so Overwrite and Append - the paths taken whenever
the target note already exists - still answered `Could not resolve file exists
behavior for 'x.md'.` and still raised a second, vaguer notice. All three write
helpers record their cause now. Verified live: a Template choice set to Overwrite with
a missing template now answers `Template file not found at path "templates/gone.md".`
with exactly one notice, where it previously gave the vague sentence plus two notices.
**`/abort` could hand back a prompt it had just cancelled.** A prompt raised while no
poll was parked sits in `session.queue`; aborting rejected the pending map but left
the queue, so the next poll delivered a dialog the client had just asked to cancel,
for a dead requestId, in front of the run's real terminal event. Queued prompts are
dropped now; a queued done/error is kept, because that is the outcome the client is
waiting for.
Also from the review:
- `/abort` is now driven through the real router in tests - POST 200 with
`interrupted`, 409 on an ended session, 404 for GET and for a bad token - instead of
calling `abortSession` directly, so the routing and status mapping this PR
documents are actually pinned. The handshake's `capabilities` marker is asserted too.
- The docs no longer promise `/abort` produces an `error` event: a run with nothing
left to interrupt finishes normally and delivers `done`, side effects committed.
`/abort`'s 409 and 404 responses are documented, and the `409` sentence further down
is scoped to `/reply`.
- `promptProvider`'s opening paragraph still claimed every method rejects on a client
cancel, contradicting the paragraph below it.
- The interactive server's tests leaked sessions (they live for 60s after `finish`),
so the file was three tests away from failing on MAX_SESSIONS as a capacity error
attributed to whichever test came 33rd.
- Dead surface removed: Capture's `failRun` level parameter no caller passes, and
`dedupeKey`'s message fallback, unreachable since attribution requires a frame.
Refs #1601, #1602, #1603, #1605
Deploying quickadd with
|
| Latest commit: |
380f915
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://a4a0f4df.quickadd.pages.dev |
| Branch Preview URL: | https://chhoumann-1601-error-reporti.quickadd.pages.dev |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughWalkthroughThe PR centralizes error reporting, propagates choice failure reasons, adds committed outcome recording, introduces interactive session abort semantics, and de-duplicates suggester render warnings. Documentation and tests cover the updated protocols and failure paths. ChangesError reporting and attribution
Choice execution outcomes
Interactive prompt abort protocol
Render fallback warnings
Estimated code review effort: 4 (Complex) | ~60 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 767a15cbe5
ℹ️ 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".
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
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 `@src/engine/TemplateChoiceEngine.ts`:
- Around line 217-230: Update appendToExistingFileWithTemplate’s canvas/base
guard to record the same specific failure it logs by calling
noteTemplateFileFailure with the constructed error and message before returning
null. Preserve the existing execution-scope failure marking and error log so
run() can use lastTemplateFileFailure via its existing fallback without emitting
a generic duplicate notice.
In `@src/interactive/interactivePromptServer.ts`:
- Around line 645-661: Drain the POST /abort request body before sending any
response to avoid leaving bytes on a keep-alive connection. In the /abort
handler near abortSession, consume the request using req.resume() with a small
byte limit or the existing capped readBody() mechanism, then preserve the
current interrupted status and response behavior.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 7a5ee0c4-885d-4d97-acb4-a756a9b74040
📒 Files selected for processing (33)
docs/src/content/docs/docs/Advanced/CLI.mdsrc/ai/OpenAIRequest.test.tssrc/ai/OpenAIRequest.tssrc/choiceExecutor.tssrc/cli/registerQuickAddCliHandlers.test.tssrc/cli/registerQuickAddCliHandlers.tssrc/engine/CaptureChoiceEngine.notice.test.tssrc/engine/CaptureChoiceEngine.selection.test.tssrc/engine/CaptureChoiceEngine.tssrc/engine/MacroChoiceEngine.tssrc/engine/TemplateChoiceEngine.notice.test.tssrc/engine/TemplateChoiceEngine.tssrc/engine/TemplateEngine.tssrc/engine/choiceOutcomeRecorder.test.tssrc/engine/choiceOutcomeRecorder.tssrc/gui/GenericSuggester/genericSuggester.tssrc/gui/InputSuggester/inputSuggester.tssrc/gui/suggesters/choiceSuggester.tssrc/gui/suggesters/utils.test.tssrc/gui/suggesters/utils.tssrc/interactive/interactivePromptServer.test.tssrc/interactive/interactivePromptServer.tssrc/interactive/promptProvider.test.tssrc/interactive/promptProvider.tssrc/logger/logManager.tssrc/main.tssrc/quickAddApi.tssrc/types/ChoiceOutcome.tssrc/utils/errorUtils.test.tssrc/utils/errorUtils.tssrc/utils/unhandledRejectionReporter.test.tssrc/utils/unhandledRejectionReporter.tssrc/utils/userScript.ts
Two review findings from Codex on the PR, both real. **Report-once must not be forever.** A long-lived user-script module that re-throws one cached `Error` on every invocation was reported the first time and then silently suppressed on every subsequent run - no notice, no console entry, a command that simply does nothing. That is the exact failure the reporting seam exists to remove, so suppression now expires after 10 seconds: the same window, for the same reason, as the unhandled-rejection reporter's. One propagation unwinds in microseconds, so the stacked notices in #1601 still collapse while separate runs stay separate. **The canvas/base append guard lost its reason.** Appending a template to a `.canvas` or `.base` file would splice raw text into structured JSON, so it is refused - with the most actionable sentence any of these exits produces, since it names the fix ("Use the Overwrite file-exists option instead"). It was the last report-and-return-null exit still handing a CLI or interactive caller "Could not resolve file exists behavior". Both are pinned by tests verified to fail against the unfixed source. Refs #1601, #1603
/abort takes no body, but a client may still send one, and it answered without
reading - the only route that did. Unread bytes left on a keep-alive socket become
the next request's problem. Node drains an unconsumed request itself when the
response finishes, so this was not reachable in practice; reading it here makes the
route independent of that internal and symmetric with /reply.
Verified over a real keep-alive connection with one socket: `/abort` carrying a 2 KB
body answers `200 {"ok":true,"interrupted":1}`, the next request on the SAME socket
parses correctly, and the final poll still delivers the run's real outcome.
The router test's request stand-in is now async-iterable, which is what a real
IncomingMessage is - it 400'd against the drain otherwise, so the test was quietly
looser than production.
Refs #1605
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #1601. Fixes #1602. Fixes #1603. Fixes #1604. Closes #1605.
Successor to #1606, built on its contract rather than forking it. That PR established three
rules - a dismissal is a typed error, a failure propagates, nothing floats - and filed the
five places where they are still not true. This is those five, plus the sibling defects that
turned up while proving each one.
The problem
QuickAdd could tell the user that something broke. It could not reliably tell them once,
whose fault it was, or what actually went wrong.
MacroChoiceEnginereports and re-throws; the command handler reports again.
plugin:quickadd:appeared anywhere inthe stack.
Error.stackis captured at construction with the whole live call stack, soanother plugin's bug that merely ran through QuickAdd was claimed - and
preventDefault()removed the console line naming the real culprit.
is at the desktop - a genuine failure reached the client as
"Choice execution failed; no file was created."while the actionable sentence went to anotice on a screen nobody was watching.
renderItemfallback assigned to the caught Error's.message.renderSuggestionruns per row per keystroke, so a cached Error grew its prefix without bound - and since
logWarningraises a Notice, it did so once per row per keypress.infoprompt aborted the run.GenericInfoDialogresolves onevery close path and has no reject path at all, so the identical run in the app continues.
Escape is the only gesture an info panel affords.
What a user sees
One failure, one notice - and it names the choice. A Macro whose user script throws.
A broken
renderItemstops burying the user. Three items, three renders - nine stackednotices, each message one prefix longer than the last, overflowing the window.
Escape stops being an error. Dismissing the one-page input modal raised a red 15-second
(ERROR)notice - a direct violation of #1606's own first rule, found while auditing thehandlers #1601 touches.
The design
Report-once belongs to
reportError, not to each layerBoth layers in #1601 are right to report - neither can know whether anything above it will -
so the rule moved into the function they both call.
reportErrorremembers what it has shown,in a
WeakSetkeyed on identity, not text: the same failure through five layers reportsonce, while two independent failures that happen to read alike still both report. It walks the
causechain, because not every layer re-throws the same instance.Two consequences I had to chase down, both found by review:
MacroChoiceEnginenow names thechoice as well as the script. That matters most where the outer handler is not there to help:
a run-on-startup macro has no user action to correlate a notice with.
OpenAIRequestreported the raw provider errorand threw a wrapper carrying the provider name and QuickAdd's context-window guidance -
report-once dropped the wrapper. It reports the wrapper now, whose message is a strict superset.
Two more sites had the same doubling and did not reach either reporting layer: the user-script
load path (
getUserScriptthrows straight pastMacroChoiceEngine's catch, so a typo'drequirestacked two notices with the same 300-character message), and the AI request pair above.Attribution: the topmost plugin frame decides
#1602 proposed requiring the first frame to be ours, and expected worse false negatives.
Measured, there are none. An Error constructed inside Obsidian's own async plumbing carries no
plugin frame at all - not even the caller's:
so "any frame" was never catching that class either. The only thing it caught and this does not
is a foreign frame above ours - which is the false positive. QuickAdd's own bug reached
through another plugin still reports; verified live in both directions.
Two details the shape depends on:
Name: messageheader is stripped, rather than filtering foratframes. QuickAdd isisDesktopOnly: falseand Obsidian mobile runs JavaScriptCore, whose frames arefn@url:line:colwith noatand no header. Anat-keyed filter would have left thisreporter dead on iOS with every desktop-shaped test still green. Both shapes are pinned.
:line:col, because an eval'd frame names thebundle as an origin with no position -
eval at exports.load (plugin:quickadd)is a userscript,
eval at <anonymous> (plugin:dataview)is a dataviewjs snippet. Those are exactly theframes attribution must not skip past.
The outcome carries the reason, at every exit
ChoiceOutcome's error variant gainsreason, exactly as the abort path already does, andboth CLI sites forward it - the interactive bridge and
runResolvedChoice, which servesquickadd:run-templateandquickadd:run verifyand had the identical bug through a differentcommand. The URI x-callback handler keeps ignoring it on both variants, so no vault detail
leaks to an external callback URL.
Recorded at every failure exit, not just the top-level catch: five of the six were silent
returns producing the same reason-less outcome without any exception, so fixing only the catch
would have left #1603 reproducible on the more common paths. Review caught that my first pass
covered only the
createNewbranch, leaving Overwrite/Append - the paths taken whenever thetarget note already exists - on the vague sentence.
Success is recorded at each engine's commit point precisely so a post-commit failure cannot make
an automation retry and duplicate the side effect, so that terminality lives in the recorder, not
in
run(): Capture commits from two methods, and the canvas path continues into steps whosethrows unwind into
run()'s catch.An
infocancel closes the panel;POST /abortends the run#1574 left the
infodivergence alone because cancelling was a client's only explicit way out.So the protocol gets a real one first:
It rejects every pending prompt and marks the session so a later prompt rejects immediately -
a run that was mid-work unwinds at its next prompt. It pushes no final event: the run unwinds
through the ordinary cancellation path and delivers its real outcome, which is more truthful than
a fabricated one.
interruptedis there because the abort is not omnipotent, and I would rather say so at the wirethan gloss it: a Template/Capture run still opens some prompts in Obsidian itself which never
travel over this bridge (filed as #1614, reproduced live), so
0tells a client it stopped nothing.The
infoexception is decided insubmitReply, which already tracks each prompt's type forreply validation - not in
RemotePromptProvider.infoDialog. A provider-side swallow cannot tella per-prompt cancel from a session abort without a second error class, and would leave
/abortunable to end a run blocked on an info panel, the one case it exists for.
The handshake carries
"capabilities":["abort"]. Both halves are observable changes to adocumented wire, and without a marker a client could only feature-detect by string-matching a 404
body. This is why the PR is
feat:- a new public route plus a reversed reply semantic is nota patch, even though everything in it is a bug fix.
One
toError, one warnerlogManagerhad a secondtoError- a context-less twin of the safe one. Its existence is why#1604 happened: the two suggesters imported that one, so the safe helper's "do NOT mutate the
caller's Error" contract did not apply to the call sites that most needed it, and they hand-rolled
the mutation it forbids. Deleted; one helper now.
The notice storm is fixed at the same seam (
createRenderFallbackWarner), which also covers thechoice picker's Markdown-render fallback - the same per-row-per-keystroke shape at
logErrorseverity.
Validation
Everything below ran in this worktree's own isolated vault (Obsidian 1.13.0), comparing a build of
f9f158d0against this branch in the same instance.(ERROR)noticequickadd.api.suggesterrenderItem, 3 items × 3 rendersquickadd:interactiveon a missing templateChoice execution failed; no file was created.Template file not found at path "templates/does-not-exist.md".quickadd … verifyon the sameinfocanceldonePlus, over real loopback HTTP:
/abort→200 {"interrupted":1}thenerror: Input cancelled by user;/aborton an ended session →409;GET /abort→404;bad token →
404; browserOrigin→403.tsc,eslint, and 4502 unit tests green. New ratchets verified to fail on revert: the twoattribution tests fail against the old "any frame" rule, and the JavaScriptCore test fails against
an
at-prefix filter.Review changed the design, twice
Both an adversarial design review (before coding) and an implementation review (before this PR)
found things I had wrong:
/^\s*at /frame gate would have silently killed the reporter on iOS.introduced and this PR removes.
/abortleft queued prompt events in place, so the next poll could hand a client the verydialog it had just cancelled, in front of the run's real outcome.
MAX_SESSIONSas a capacity error blamed on whichever test came 33rd.Known limitations
WeakSetcannot key a primitive,and normalising it at the throw boundary would break the legacy string-sentinel cancellation
contract
isCancellationErrorstill honours for user scripts. Pinned by a test that says so,rather than left to be rediscovered.
StartupMacroEngine's "Startup macro 'X' failed" wrapper is dropped when an inner layerreported first. Mitigated by the inner report now naming the choice; "at startup" is the part
that is lost, and it is inferable from an unprompted notice at launch.
/abortcannot reach prompts QuickAdd opens in Obsidian on a Template/Capture run ([BUG] An interactive run still opens some prompts in Obsidian instead of routing them to the client #1614).unchanged, and not something the old rule caught either.
Filed rather than folded in
#1614 (an interactive run still opens some prompts in Obsidian) · #1615 (a capture that commits
nothing is reported as a verified success).
Summary by CodeRabbit
New Features
Bug Fixes
Documentation
/abortand/replysemantics and response handling.