Skip to content

Conversation

@Tokisaki-Galaxy
Copy link

@Tokisaki-Galaxy Tokisaki-Galaxy commented Jan 5, 2026

https://github.com/Tokisaki-Galaxy/aw-watcher-web/actions
Passed the action compilation and grammar check, and has been tested to be normal.
resolve #199


Important

Implements Offscreen API in Chrome to keep the service worker alive by adding offscreen document and periodic messaging.

  • Behavior:
    • Implements setupOffscreen() in main.ts to keep the service worker alive using the Offscreen API for Chrome.
    • Sends periodic KEEP_ALIVE messages from offscreen.ts to prevent service worker termination.
    • Listens for KEEP_ALIVE messages in main.ts and responds with status 'ok'.
  • Files:
    • Adds offscreen.html to load offscreen.ts.
    • Adds offscreen.ts to send periodic messages to the background script.
  • Configuration:
    • Updates manifest.json to include offscreen permission for Chrome.
    • Updates vite.config.ts to include src/offscreen.html in additionalInputs.

This description was created by Ellipsis for 712cbab. You can customize this summary. It will automatically update as commits are pushed.

Copilot AI review requested due to automatic review settings January 5, 2026 01:49
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR implements Chrome's Offscreen API to keep the service worker alive, replacing the previous keep-alive mechanism that used periodic alarm checks. The new approach creates an offscreen document that sends periodic heartbeat messages to the background service worker.

Key Changes:

  • Replaced interval-based alarm polling with Chrome Offscreen API for service worker lifecycle management
  • Added offscreen document that sends keep-alive messages every 20 seconds
  • Added "offscreen" permission to the manifest

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
src/manifest.json Added "offscreen" permission required for Chrome Offscreen API
src/offscreen.html Created minimal HTML file to host the offscreen script
src/offscreen.ts Implemented keep-alive logic that sends messages every 20 seconds to background
src/background/main.ts Replaced old keep-alive mechanism with offscreen document creation and message handler
vite.config.ts Added offscreen.html to build inputs for bundling

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copy link
Contributor

@ellipsis-dev ellipsis-dev bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Important

Looks good to me! 👍

Reviewed everything up to 712cbab in 1 minute and 47 seconds. Click for details.
  • Reviewed 122 lines of code in 5 files
  • Skipped 0 files when reviewing.
  • Skipped posting 2 draft comments. View those below.
  • Modify your settings and rules to customize what types of comments Ellipsis leaves. And don't forget to react with 👍 or 👎 to teach Ellipsis.
1. src/background/main.ts:110
  • Draft comment:
    Duplicate invocation of setupOffscreen: It's added as listeners on onStartup and onInstalled, then called directly. If intentional, add a clarifying comment; otherwise, consider removing the direct call.
  • Reason this comment was not posted:
    Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 10% vs. threshold = 50% The comment is asking the author to either clarify or change the code. However, this pattern seems intentional: the direct call ensures immediate setup when the background script loads, while the event listeners handle browser restarts. The function has proper guards to prevent duplicate document creation. This comment is essentially asking the author to "confirm their intention" or "add clarifying comments," which violates the rules. The comment doesn't identify a bug or required code change - it's more of a "this might be confusing" observation. The pattern is actually quite common in browser extensions where you want something to run both immediately and on specific events. The pattern could genuinely be confusing to future maintainers, and a clarifying comment might improve code readability. The comment does suggest a concrete action (adding a comment or removing the call), which could be considered actionable rather than purely speculative. While the pattern might benefit from clarification, the rules explicitly state not to ask the author to confirm their intention or add clarifying comments. The comment uses the phrase "If intentional, add a clarifying comment; otherwise, consider removing" which is exactly the type of "verify/ensure/confirm" language the rules say to avoid. There's no actual bug here - the code works correctly with the guards in place. This comment should be deleted. It's asking the author to either confirm their intention or add a clarifying comment, which violates the rules. The code has proper guards and the pattern is intentional and functional. No actual code change is required.
2. src/offscreen.ts:3
  • Draft comment:
    The comment is in Chinese; consider using English (or a consistent language) for clarity across the codebase.
  • Reason this comment was not posted:
    Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 20% vs. threshold = 50% This is a style/consistency comment about using English instead of Chinese. While language consistency can be valuable, the rules state "Do NOT comment unless there is clearly a code change required" and "Do NOT make comments that are obvious or unimportant." This is more of a stylistic preference rather than a functional issue. The code works fine with Chinese comments. Without seeing the rest of the codebase, I can't verify if there's actually an established pattern of English-only comments. The comment is also somewhat presumptuous about what language should be used - maybe this is a Chinese-language project. I might be missing context about whether this codebase has an established English-only policy. If the entire codebase uses English comments and this is the only Chinese comment, it could be a legitimate consistency issue worth addressing. Even if there is an English-only policy, this falls under "obvious or unimportant" comments. The author can see the comment is in Chinese. This is a stylistic preference that doesn't affect functionality, and without strong evidence of a codebase-wide policy visible in this diff, I should err on the side of deletion per the rules. This comment is about style/language preference rather than a functional code issue. It falls under "obvious or unimportant" comments that don't clearly require a code change. Without strong evidence of a codebase policy in the diff itself, this should be deleted.

Workflow ID: wflow_BI7DONsYuOb4zSim

You can customize Ellipsis by changing your verbosity settings, reacting with 👍 or 👎, replying to comments, or adding code review rules.

@0xbrayo
Copy link
Member

0xbrayo commented Jan 5, 2026

Have you tested this locally? I keep getting error creating bucket for some reason. Properly set up cors and everything in the server. @BelKed Maybe you can give this a go.

@Tokisaki-Galaxy
Copy link
Author

Tokisaki-Galaxy commented Jan 5, 2026

Have you tested this locally? I keep getting error creating bucket for some reason. Properly set up cors and everything in the server. @BelKed Maybe you can give this a go.

I have used it normally, and after modification, it works well at least at windows 11 edge(143.0.3650.96).
If there is an error, maybe there is a problem with code compatibility. If possible, please put it forward so that I can revise it.

image 636254b9-a0cb-412b-a239-e098cf955f3d

@0xbrayo
Copy link
Member

0xbrayo commented Jan 5, 2026

Got it working on the python server. Some cors issues on the rust server, I presume. Seems to be working perfectly.🚀

@Tokisaki-Galaxy
Copy link
Author

Got it working on the python server. Some cors issues on the rust server, I presume. Seems to be working perfectly.🚀

The issues on the Rust server appear to be CORS-related; likely because its security policy doesn't yet permit the chrome-extension:// origin/scheme.

Since this has been a long-standing issue affecting many users, I’d appreciate it if we could expedite the merge and release of this fix. Thanks!

@BelKed
Copy link
Contributor

BelKed commented Jan 8, 2026

The Rust server has hard-coded UUID which only extensions installed from the Chrome Web Store have
If an extension is installed manually, it gets a different UUID, so seeing a CORS issue is normal in this case

I'll manually test it in the upcoming days, thank you for the fix 💙

Copy link
Contributor

@BelKed BelKed left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Manually tested, seems to be working correctly :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

aw-watcher-web-chrome reports only beginning and end of tab use

4 participants