Repository navigation
fix: audit bug fixes — capture cleanup, collection isolation, MCP error handling, Vue/React reactivity - #281
Merged
Merged
Conversation
…eardown - core: push() now applies sanitizeText/sanitizeMeta to ancestor segments and calls sanitizeText even when text is undefined (fixes #247, #248) - core: validate maxHistory is a non-negative integer in constructor (fixes #254) - core: guard a11yTextExtractor aria-labelledby lookup against missing document in non-browser environments (fixes #257) - react-native: useAskableVisibility and useAskableScrollView now capture context ownership at creation time so cleanup always destroys the right context regardless of later prop changes (fixes #250) - svelte: useAskable.svelte.ts registers focus/clear listeners inside $effect so they are removed when the component unmounts (fixes #252) - svelte, vue: remove wildcard @askable-ui/core from devDependencies (fixes #262)
Moves removeOverlay / state reset before the onCapture call so that any error thrown inside the callback cannot leave a stale overlay in the DOM. Adds a regression test that asserts the overlay is already absent from the DOM by the time onCapture fires.
Replaces Promise.all with Promise.allSettled (plus an async wrapper to capture synchronous throws) so a single failing sanitizeItem does not reject the entire collection. Failed items are omitted from the result while all healthy items still resolve. Adds tests for both synchronous throw and async rejection isolation.
- Wrap get_current_context and format_context_for_prompt handlers in try/catch so a provider error returns isError:true instead of crashing the MCP server process. - Remove .passthrough() from the sources array item schema so unrecognised fields are stripped rather than forwarded to the provider without validation.
The previous implementation evaluated enabled once at setup time so
toggling it later had no effect. Now accepts MaybeRef<boolean> and
uses watch({ immediate: true }) to register or unregister the source
whenever the value changes. Adds a test that toggles a ref from false
to true and back.
…Inspector useAskableRegionCapture: onCapture and onCancel callbacks captured the options object at start() call time. Any callback change between start() and the pointer-up event would be silently ignored. Now always reads from optionsRef.current so the most-recent callback fires. AskableInspector: inspectorOptions was used inside useEffect but only individual scalar fields were listed in the deps array. Stabilise the options object with useMemo keyed on the same scalars so the effect dep is a single stable reference and exhaustive-deps is satisfied.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
This branch was successfully deployed
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
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.
Summary
Fixes for all bugs identified during the fresh codebase audit (#267–#279).
Fixes included
fix(core): clean up capture overlay before invoking onCapture— closes #276Moves
removeOverlay()before theonCapturecallback so any error thrown inside it cannot leave a stale overlay mounted in the DOM.fix(core): isolate per-item sanitizeItem failures in collection sources— closes #275Replaces
Promise.allwithPromise.allSettled(+ async wrapper for sync throws) incollectionItemsResult. A single failingsanitizeItemno longer rejects the entire collection — failing items are omitted, healthy items still resolve.fix(mcp): error handling in tool handlers and tighten Zod schema— closes #272, #273get_current_contextandformat_context_for_prompthandlers now returnisError: trueon provider failure instead of crashing the server process..passthrough()from the sources array item schema so unrecognised fields are stripped before reaching the provider.fix(vue): make useAskableSource enabled option reactive— closes #268enablednow acceptsMaybeRef<boolean>and useswatch({ immediate: true })to register/unregister the source whenever the value changes.fix(react): stale closures in useAskableRegionCapture and AskableInspector— closes #269, #270useAskableRegionCapture:onCapture/onCancelnow always read fromoptionsRef.currentat call time.AskableInspector: inspector options stabilised withuseMemokeyed on the individual scalar fields, satisfying exhaustive-deps.Previously committed on this branch:
fix(core):maxHistoryvalidation +push()sanitizer leaks — closes bug(core): Promise.all in collectionItemsResult — one failing sanitizeItem rejects the entire collection #275 (earlier commit)fix(core/a11y): SSR guard foraria-labelledbyfix(react-native): context ownership tracking inuseAskableVisibility/useAskableScrollViewfix(svelte):$effect-based listener cleanup inuseAskablefix(deps): wildcard@askable-ui/coreremoved from svelte + vue devDependencies — closes fix(deps): remove wildcard @askable-ui/core devDependency from svelte and vue packages #271Test plan
npm test --workspace=packages/core— 181 tests passnpm test --workspace=packages/vue— 39 tests pass (includes new reactiveenabledref test)npm test --workspace=packages/react— 52 tests passnpm test --workspace=packages/mcp— 5 tests passnpm test --workspace=packages/svelte— 35 tests passGenerated by Claude Code