fix(extension): local content extraction for Chrome Web Store approval#76
Merged
fix(extension): local content extraction for Chrome Web Store approval#76
Conversation
Chrome Web Store rejected the extension because the core "Save page as Skill" feature called https://skillkit.sh/api/save-skill which is unreachable during review. This rewires page saving to work entirely locally using chrome.scripting.executeScript to extract page content (title, meta description, headings, body text) directly in the browser. - Remove API_URL and callSaveApi() server dependency - Add extractPageContent() via chrome.scripting.executeScript - Add detectTags() with weighted keyword scoring (URL/headings/body) - Add buildPageSkill() to generate SKILL.md with frontmatter locally - Add scripting permission to manifest - Pass tabId from popup to background for script injection targeting - Block chrome-extension:// and chromewebstore.google.com URLs - Add console.error logging in catch blocks for debuggability - Guard currentTabId validity in popup click handler - Update store listing to reflect fully local processing - Bump version to 1.20.0
|
The latest updates on your projects. Learn more about Vercel for GitHub. 2 Skipped Deployments
|
📝 WalkthroughWalkthroughThe PR moves page/selection skill extraction and markdown generation into the extension (client-side), adding content extraction, tag detection, description derivation, updated messaging, and download/notification flows; it also bumps versions, adds the Changes
Sequence DiagramsequenceDiagram
participant User as User / Popup
participant BG as Background Script
participant CS as Content Script
participant Page as Page DOM
participant DL as Chrome Downloads API
User->>BG: click Save or send SAVE_PAGE
BG->>CS: executeScript extractPageContent(tabId)
CS->>Page: read title, meta, headings, body
Page-->>CS: page data
CS-->>BG: PageContent
BG->>BG: detectTags, makeDescription, buildPageSkill/Selection
BG->>BG: yamlEscape & slugify metadata
BG->>DL: download skill markdown
DL-->>User: file download starts
BG->>User: notifyTab with badge/result
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 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 |
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
![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
🧹 Nitpick comments (3)
packages/extension/src/background.ts (3)
337-346: Download callback ignoreschrome.runtime.lastError.If the download fails (e.g., invalid filename, disabled downloads), the error is silently swallowed. Checking
lastErrorin the callback would let you surface the failure.Proposed fix
() => { + if (chrome.runtime.lastError) { + console.error("[SkillKit] Download failed:", chrome.runtime.lastError.message); + } URL.revokeObjectURL(blobUrl); },🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/extension/src/background.ts` around lines 337 - 346, The download callback for chrome.downloads.download currently ignores chrome.runtime.lastError so failures are swallowed; update the anonymous callback passed to chrome.downloads.download (the one that calls URL.revokeObjectURL(blobUrl)) to check chrome.runtime.lastError and surface/log/notify the error (including context like the filename `skillkit-skills/${name}/SKILL.md`), then always revoke the blob URL (URL.revokeObjectURL(blobUrl)) in all cases to avoid leaks.
313-323:yamlEscapedoesn't quote YAML special bare values.Strings like
"true","false","null","yes","no", or pure numeric strings pass the character-class test but are interpreted as non-string types by YAML parsers. This mainly matters ifyamlEscapeis used for fields other than URLs/descriptions (e.g., thenamefield as noted above), but hardening it now would prevent future surprises.Example enhancement
function yamlEscape(value: string): string { const singleLine = value.replace(/\r?\n/g, " ").trim(); + const YAML_SPECIAL = /^(true|false|yes|no|on|off|null|~|\d[\d.eE+-]*)$/i; if ( /[:#{}[\],&*?|>!%@`]/.test(singleLine) || singleLine.startsWith("'") || - singleLine.startsWith('"') + singleLine.startsWith('"') || + YAML_SPECIAL.test(singleLine) ) { return `"${singleLine.replace(/\\/g, "\\\\").replace(/"/g, '\\"')}"`; } return singleLine; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/extension/src/background.ts` around lines 313 - 323, The yamlEscape function currently only quotes values based on special characters but misses bare scalars like true/false/null/yes/no and numeric-only strings which YAML can coerce to non-string types; update yamlEscape to also detect these cases (e.g., match /^(-?\d+(\.\d+)?([eE][+-]?\d+)?)$/ for numbers and a case-insensitive set for boolean/null tokens like "true","false","yes","no","null","~") and force-quote them the same way you do when special characters are present; keep existing newline-to-space trimming and backslash/quote escaping logic and apply it when quoting inside yamlEscape.
192-198: Substring matching on short keywords can produce false-positive tags.
String.includes(kw)with 2-letter keywords like"ai","ml","js","ts","ci","cd"matches inside unrelated words (e.g.,"container"→"ai","html"→"ml","adjust"→"js"). The binary scoring limits the blast radius, but tech-unrelated pages could still receive spurious tags.Using word-boundary matching would be more precise:
Suggested improvement
- if (urlLower.includes(kw)) score += 3; - if (headingsLower.includes(kw)) score += 2; - if (bodyLower.includes(kw)) score += 1; + const re = new RegExp(`\\b${kw}\\b`); + if (re.test(urlLower)) score += 3; + if (re.test(headingsLower)) score += 2; + if (re.test(bodyLower)) score += 1;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/extension/src/background.ts` around lines 192 - 198, The current substring matching in the tag-scoring loop over TECH_KEYWORDS (inside the for (const [tag, keywords]...) block) produces false positives for short keywords; change the checks that use urlLower.includes(kw), headingsLower.includes(kw), and bodyLower.includes(kw) to use word-boundary, case-insensitive regex tests instead: build/escape a RegExp for each kw using \b... \b and test against urlLower, headingsLower, and bodyLower so only whole-word matches increment the score (keep the same +3/+2/+1 scoring logic and ensure kw escaping to avoid regex metacharacters).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/extension/src/background.ts`:
- Around line 233-237: The YAML frontmatter construction at skillMd currently
emits an unquoted name value which can be parsed as null/boolean for slugs like
"null" or "yes"; update the template so the name field is produced as a properly
quoted/escaped string (e.g., use yamlEscape(skillName) or wrap ${skillName} in
quotes) the same way description uses yamlEscape, ensuring the name line in
skillMd uses yamlEscape(skillName) (or equivalent) to prevent YAML
misinterpretation.
In `@packages/extension/src/popup.ts`:
- Around line 29-33: The popup's restricted-url check is missing the Chrome Web
Store prefix and is out-of-sync with background.ts; extract the prefixes into a
shared constant (e.g., export RESTRICTED_PREFIXES and an isRestricted(url)
helper in a new/shared module like constants.ts or types.ts), include
"https://chromewebstore.google.com" in that list, then import and use the shared
isRestricted in popup.ts (replace the inline currentUrl check) and in
background.ts so both files use the same logic.
---
Nitpick comments:
In `@packages/extension/src/background.ts`:
- Around line 337-346: The download callback for chrome.downloads.download
currently ignores chrome.runtime.lastError so failures are swallowed; update the
anonymous callback passed to chrome.downloads.download (the one that calls
URL.revokeObjectURL(blobUrl)) to check chrome.runtime.lastError and
surface/log/notify the error (including context like the filename
`skillkit-skills/${name}/SKILL.md`), then always revoke the blob URL
(URL.revokeObjectURL(blobUrl)) in all cases to avoid leaks.
- Around line 313-323: The yamlEscape function currently only quotes values
based on special characters but misses bare scalars like true/false/null/yes/no
and numeric-only strings which YAML can coerce to non-string types; update
yamlEscape to also detect these cases (e.g., match
/^(-?\d+(\.\d+)?([eE][+-]?\d+)?)$/ for numbers and a case-insensitive set for
boolean/null tokens like "true","false","yes","no","null","~") and force-quote
them the same way you do when special characters are present; keep existing
newline-to-space trimming and backslash/quote escaping logic and apply it when
quoting inside yamlEscape.
- Around line 192-198: The current substring matching in the tag-scoring loop
over TECH_KEYWORDS (inside the for (const [tag, keywords]...) block) produces
false positives for short keywords; change the checks that use
urlLower.includes(kw), headingsLower.includes(kw), and bodyLower.includes(kw) to
use word-boundary, case-insensitive regex tests instead: build/escape a RegExp
for each kw using \b... \b and test against urlLower, headingsLower, and
bodyLower so only whole-word matches increment the score (keep the same +3/+2/+1
scoring logic and ensure kw escaping to avoid regex metacharacters).
ℹ️ Review info
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
packages/extension/package.jsonpackages/extension/src/background.tspackages/extension/src/manifest.jsonpackages/extension/src/popup.tspackages/extension/src/types.tspackages/extension/store-assets/STORE_LISTING.md
packages/extension/src/popup.ts
Outdated
Comment on lines
+29
to
+33
| const isRestricted = | ||
| !currentUrl || | ||
| currentUrl.startsWith("chrome://") || | ||
| currentUrl.startsWith("chrome-extension://") || | ||
| currentUrl.startsWith("about:"); |
There was a problem hiding this comment.
Restricted-URL check is out of sync with background.ts.
The popup checks chrome://, chrome-extension://, and about: — but background.ts also blocks https://chromewebstore.google.com. This means the Save button stays enabled on Chrome Web Store pages, and the user will see an error only after clicking it.
Consider extracting the restricted-prefix list into a shared constant in types.ts (or a shared module) so both files stay in sync.
Proposed approach
In a shared module (e.g., constants.ts):
export const RESTRICTED_PREFIXES = [
"chrome://",
"chrome-extension://",
"https://chromewebstore.google.com",
"about:",
];
export function isRestricted(url: string): boolean {
if (!url) return true;
return RESTRICTED_PREFIXES.some((p) => url.startsWith(p));
}Then import and use isRestricted in both popup.ts and background.ts.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/extension/src/popup.ts` around lines 29 - 33, The popup's
restricted-url check is missing the Chrome Web Store prefix and is out-of-sync
with background.ts; extract the prefixes into a shared constant (e.g., export
RESTRICTED_PREFIXES and an isRestricted(url) helper in a new/shared module like
constants.ts or types.ts), include "https://chromewebstore.google.com" in that
list, then import and use the shared isRestricted in popup.ts (replace the
inline currentUrl check) and in background.ts so both files use the same logic.
- Extract RESTRICTED_PREFIXES and isRestricted() into shared constants.ts so popup and background use the same URL blocklist - Fix yamlEscape to quote YAML bare scalars (true/false/null/yes/no/on/off) and numeric strings that parsers would coerce to non-string types - Apply yamlEscape to skill name fields in both buildPageSkill and buildSelectionSkill to prevent YAML misinterpretation - Switch tag detection from substring matching to word-boundary regex to eliminate false positives (e.g. "js" matching "adjustments") - Check chrome.runtime.lastError in download callback and always revoke blob URL to prevent leaks on download failure
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/extension/src/background.ts`:
- Around line 330-348: Change downloadSkill(name, skillMd) to return a Promise
that resolves when the chrome.downloads.download callback completes and rejects
if chrome.runtime.lastError exists; ensure URL.revokeObjectURL(blobUrl) still
runs. Update all callers (the four context menu handlers and message handlers
that currently call downloadSkill and then immediately call notifyTab() or
sendResponse()) to await downloadSkill(...) and only call notifyTab/sendResponse
after the Promise resolves, propagating any rejection so callers can
handle/report failures instead of always showing success.
In `@packages/extension/src/popup.ts`:
- Around line 18-23: The code in init and any other checks treats tab IDs as
falsy (rejecting 0); update the checks to use explicit null/undefined
comparisons: in the init function replace the falsy guard on tab?.id with an
explicit null/undefined check (e.g., test for tab?.id == null) and similarly
replace any checks like if (!currentTabId) or if (!tabId) with explicit
null/undefined checks (currentTabId == null) so a valid tab id of 0 is accepted;
apply this change to the guard in init and the other occurrence referencing
currentTabId/tab id.
- Around line 53-57: Wrap the chrome.runtime.sendMessage call in a
try/catch/finally inside the async function that uses response so the UI is
always recovered: call await chrome.runtime.sendMessage(message) inside try,
handle rejected cases in catch (e.g., treat as ErrorResponse or log/propagate),
and move saveBtn.removeAttribute("disabled") into finally so saveBtn is
re-enabled whether sendMessage resolves or throws; reference the response
variable and SaveResponse/ErrorResponse types and the saveBtn DOM element to
locate and update the code.
ℹ️ Review info
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
packages/extension/src/background.tspackages/extension/src/constants.tspackages/extension/src/popup.ts
Comment on lines
330
to
+348
| function downloadSkill(name: string, skillMd: string): void { | ||
| const blob = new Blob([skillMd], { type: 'text/markdown' }); | ||
| const blob = new Blob([skillMd], { type: "text/markdown" }); | ||
| const blobUrl = URL.createObjectURL(blob); | ||
| chrome.downloads.download({ | ||
| url: blobUrl, | ||
| filename: `skillkit-skills/${name}/SKILL.md`, | ||
| saveAs: false, | ||
| }, () => { | ||
| URL.revokeObjectURL(blobUrl); | ||
| }); | ||
| chrome.downloads.download( | ||
| { | ||
| url: blobUrl, | ||
| filename: `skillkit-skills/${name}/SKILL.md`, | ||
| saveAs: false, | ||
| }, | ||
| () => { | ||
| if (chrome.runtime.lastError) { | ||
| console.error( | ||
| `[SkillKit] Download failed for skillkit-skills/${name}/SKILL.md:`, | ||
| chrome.runtime.lastError.message, | ||
| ); | ||
| } | ||
| URL.revokeObjectURL(blobUrl); | ||
| }, | ||
| ); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, let's find the downloadSkill function and verify it exists
rg "function downloadSkill|const downloadSkill" packages/extension/src/background.ts -A 20Repository: rohitg00/skillkit
Length of output: 630
🏁 Script executed:
# Find all call sites of downloadSkill
rg "downloadSkill\(" packages/extension/src/background.ts -B 3 -A 3Repository: rohitg00/skillkit
Length of output: 1307
🏁 Script executed:
# Check the file structure to understand the context better
wc -l packages/extension/src/background.tsRepository: rohitg00/skillkit
Length of output: 101
🏁 Script executed:
# Get a broader view of the file around the downloadSkill function
sed -n '320,360p' packages/extension/src/background.tsRepository: rohitg00/skillkit
Length of output: 1244
Propagate download failure instead of only logging it.
downloadSkill currently logs lastError asynchronously but returns void, allowing callers to immediately report success via notifyTab() or sendResponse() without knowing if the download succeeded. This causes the UI to show a success checkmark even when the file write fails. Return a Promise from downloadSkill and have all callers await it before notifying success.
All four call sites (context menu handlers and message handlers) have this issue and must be updated to await the result.
Suggested direction
-function downloadSkill(name: string, skillMd: string): void {
+function downloadSkill(name: string, skillMd: string): Promise<void> {
const blob = new Blob([skillMd], { type: "text/markdown" });
const blobUrl = URL.createObjectURL(blob);
- chrome.downloads.download(
- {
- url: blobUrl,
- filename: `skillkit-skills/${name}/SKILL.md`,
- saveAs: false,
- },
- () => {
- if (chrome.runtime.lastError) {
- console.error(
- `[SkillKit] Download failed for skillkit-skills/${name}/SKILL.md:`,
- chrome.runtime.lastError.message,
- );
- }
- URL.revokeObjectURL(blobUrl);
- },
- );
+ return new Promise((resolve, reject) => {
+ chrome.downloads.download(
+ {
+ url: blobUrl,
+ filename: `skillkit-skills/${name}/SKILL.md`,
+ saveAs: false,
+ },
+ (downloadId) => {
+ try {
+ if (chrome.runtime.lastError || downloadId == null) {
+ reject(
+ new Error(
+ chrome.runtime.lastError?.message ?? "Download did not start",
+ ),
+ );
+ return;
+ }
+ resolve();
+ } finally {
+ URL.revokeObjectURL(blobUrl);
+ }
+ },
+ );
+ });
}Then await downloadSkill(...) in all callers before notifyTab(...) or sendResponse(...).
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| function downloadSkill(name: string, skillMd: string): void { | |
| const blob = new Blob([skillMd], { type: 'text/markdown' }); | |
| const blob = new Blob([skillMd], { type: "text/markdown" }); | |
| const blobUrl = URL.createObjectURL(blob); | |
| chrome.downloads.download({ | |
| url: blobUrl, | |
| filename: `skillkit-skills/${name}/SKILL.md`, | |
| saveAs: false, | |
| }, () => { | |
| URL.revokeObjectURL(blobUrl); | |
| }); | |
| chrome.downloads.download( | |
| { | |
| url: blobUrl, | |
| filename: `skillkit-skills/${name}/SKILL.md`, | |
| saveAs: false, | |
| }, | |
| () => { | |
| if (chrome.runtime.lastError) { | |
| console.error( | |
| `[SkillKit] Download failed for skillkit-skills/${name}/SKILL.md:`, | |
| chrome.runtime.lastError.message, | |
| ); | |
| } | |
| URL.revokeObjectURL(blobUrl); | |
| }, | |
| ); | |
| function downloadSkill(name: string, skillMd: string): Promise<void> { | |
| const blob = new Blob([skillMd], { type: "text/markdown" }); | |
| const blobUrl = URL.createObjectURL(blob); | |
| return new Promise((resolve, reject) => { | |
| chrome.downloads.download( | |
| { | |
| url: blobUrl, | |
| filename: `skillkit-skills/${name}/SKILL.md`, | |
| saveAs: false, | |
| }, | |
| (downloadId) => { | |
| try { | |
| if (chrome.runtime.lastError || downloadId == null) { | |
| reject( | |
| new Error( | |
| chrome.runtime.lastError?.message ?? "Download did not start", | |
| ), | |
| ); | |
| return; | |
| } | |
| resolve(); | |
| } finally { | |
| URL.revokeObjectURL(blobUrl); | |
| } | |
| }, | |
| ); | |
| }); | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/extension/src/background.ts` around lines 330 - 348, Change
downloadSkill(name, skillMd) to return a Promise that resolves when the
chrome.downloads.download callback completes and rejects if
chrome.runtime.lastError exists; ensure URL.revokeObjectURL(blobUrl) still runs.
Update all callers (the four context menu handlers and message handlers that
currently call downloadSkill and then immediately call notifyTab() or
sendResponse()) to await downloadSkill(...) and only call notifyTab/sendResponse
after the Promise resolves, propagating any rejection so callers can
handle/report failures instead of always showing success.
Comment on lines
+18
to
23
| let currentTabId = 0; | ||
|
|
||
| async function init() { | ||
| const [tab] = await chrome.tabs.query({ active: true, currentWindow: true }); | ||
| if (!tab?.id) return; | ||
|
|
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cd packages/extension/src && head -50 popup.ts | cat -nRepository: rohitg00/skillkit
Length of output: 1994
🌐 Web query:
Chrome tabs API tab ID value range 0 valid identifier
💡 Result:
In the Chrome Extensions Tabs API, a real tab’s ID is a non‑negative integer (≥ 0)—so 0 is a valid tab ID (don’t treat it as “no tab” just because it’s falsy in JS). (chromium.googlesource.com)
The reserved “no tab” sentinel is chrome.tabs.TAB_ID_NONE = -1 (used to represent the absence of a browser tab, e.g., some devtools/app windows). (developer.chrome.com)
There’s no documented maximum value; tab IDs are just unique within the current browser session. (developer.chrome.com)
Citations:
- 1: https://chromium.googlesource.com/chromium/src/%2B/master/chrome/common/extensions/api/tabs.json?utm_source=openai
- 2: https://developer.chrome.com/docs/extensions/reference/api/tabs?utm_source=openai
- 3: https://developer.chrome.com/docs/extensions/reference/api/tabs?utm_source=openai
Use explicit null/undefined checks for tab IDs instead of falsy checks.
Tab IDs in Chrome's Tabs API are non-negative integers (≥ 0), so 0 is a valid tab ID. Lines 22 and 38 currently reject this value using falsy checks. Replace with explicit null/undefined checks to allow valid numeric IDs.
Suggested fix
-let currentTabId = 0;
+let currentTabId: number | null = null;
- if (!tab?.id) return;
+ if (tab?.id == null) return;
- if (!currentUrl || !currentTabId) return;
+ if (!currentUrl || currentTabId == null) return;Also applies to: 38-38
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/extension/src/popup.ts` around lines 18 - 23, The code in init and
any other checks treats tab IDs as falsy (rejecting 0); update the checks to use
explicit null/undefined comparisons: in the init function replace the falsy
guard on tab?.id with an explicit null/undefined check (e.g., test for tab?.id
== null) and similarly replace any checks like if (!currentTabId) or if (!tabId)
with explicit null/undefined checks (currentTabId == null) so a valid tab id of
0 is accepted; apply this change to the guard in init and the other occurrence
referencing currentTabId/tab id.
Comment on lines
53
to
57
| const response: SaveResponse | ErrorResponse | undefined = | ||
| await chrome.runtime.sendMessage(message); | ||
|
|
||
| saveBtn.removeAttribute('disabled'); | ||
| saveBtn.removeAttribute("disabled"); | ||
|
|
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, let's check the file exists and examine the relevant code
if [ -f "packages/extension/src/popup.ts" ]; then
echo "=== File found, examining lines 50-70 for context ==="
sed -n '50,70p' packages/extension/src/popup.ts | cat -n
else
echo "File not found at packages/extension/src/popup.ts"
# Try to find it
find . -name "popup.ts" -type f 2>/dev/null | head -5
fiRepository: rohitg00/skillkit
Length of output: 733
🏁 Script executed:
# Get more context around the code - read entire function or block
wc -l packages/extension/src/popup.tsRepository: rohitg00/skillkit
Length of output: 95
🏁 Script executed:
# Read the full file to understand the context better
cat -n packages/extension/src/popup.ts | head -100Repository: rohitg00/skillkit
Length of output: 3112
Ensure UI recovery when sendMessage fails.
If chrome.runtime.sendMessage rejects, the button remains disabled and the popup appears frozen. Wrap this in try/catch/finally and always restore button state.
Suggested fix
- const response: SaveResponse | ErrorResponse | undefined =
- await chrome.runtime.sendMessage(message);
-
- saveBtn.removeAttribute("disabled");
+ let response: SaveResponse | ErrorResponse | undefined;
+ try {
+ response = await chrome.runtime.sendMessage(message);
+ } catch {
+ setStatus("error", "Failed to communicate with background script");
+ resultEl.style.display = "none";
+ return;
+ } finally {
+ saveBtn.removeAttribute("disabled");
+ }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| const response: SaveResponse | ErrorResponse | undefined = | |
| await chrome.runtime.sendMessage(message); | |
| saveBtn.removeAttribute('disabled'); | |
| saveBtn.removeAttribute("disabled"); | |
| let response: SaveResponse | ErrorResponse | undefined; | |
| try { | |
| response = await chrome.runtime.sendMessage(message); | |
| } catch { | |
| setStatus("error", "Failed to communicate with background script"); | |
| resultEl.style.display = "none"; | |
| return; | |
| } finally { | |
| saveBtn.removeAttribute("disabled"); | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/extension/src/popup.ts` around lines 53 - 57, Wrap the
chrome.runtime.sendMessage call in a try/catch/finally inside the async function
that uses response so the UI is always recovered: call await
chrome.runtime.sendMessage(message) inside try, handle rejected cases in catch
(e.g., treat as ErrorResponse or log/propagate), and move
saveBtn.removeAttribute("disabled") into finally so saveBtn is re-enabled
whether sendMessage resolves or throws; reference the response variable and
SaveResponse/ErrorResponse types and the saveBtn DOM element to locate and
update the code.
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.
Summary
Save page as Skillcalledhttps://skillkit.sh/api/save-skill— unreachable during reviewchrome.scripting.executeScriptto extract content directly in the browserChanges
background.ts: RemovedAPI_URL/callSaveApi(), addedextractPageContent()(scripting API),detectTags()(weighted keyword scoring across URL/headings/body),buildPageSkill()(local SKILL.md generation), error logging in catch blocksmanifest.json: Addedscriptingpermission, bumped to v1.20.0types.ts: AddedtabIdtoSAVE_PAGEpayload for script injection targetingpopup.ts: PassestabIdto background, guards against invalid tab IDSTORE_LISTING.md: Updated description, features, permissions justification, and data disclosure to reflect fully local processingpackage.json: Bumped to v1.20.0Test plan
pnpm --filter @skillkit/extension buildpassespnpm --filter @skillkit/extension typecheckpassespackages/extension/dist/in Chromechrome://extensions-> shows "Cannot save" errorSummary by CodeRabbit
New Features
Chores
Documentation