feat: workflow worker thread + paste unit tests - #23
Conversation
Send-to-AI and insert-only run on a worker thread so the tray message loop stays live during page-ready / clipboard hold. COM/UIA init is per-thread. Hotkey busy is cleared via WM_QP_WORK_DONE (WM_APP+2). Extract shared paste/URL helpers to extension/paste_logic.js and cover them with node:test (duplicate detect, HTML escape, tab origin rules). Co-authored-by: Hermes/grok-4.6/m3rcur1al <hermes-m3rcur1al@local>
There was a problem hiding this comment.
Pull request overview
Moves hotkey workflows off the UI thread and adds shared, tested extension paste helpers.
Changes:
- Adds workflow worker lifecycle and per-thread COM initialization.
- Centralizes extension paste and URL logic.
- Adds Node tests and CI coverage.
Reviewed changes
Copilot reviewed 11 out of 11 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
src/app.cpp |
Implements worker execution and completion handling. |
include/app.hpp |
Declares worker state and APIs. |
src/page_ready.cpp |
Makes COM initialization thread-local. |
include/page_ready.hpp |
Updates COM initialization documentation. |
extension/paste_logic.js |
Adds shared paste and URL helpers. |
extension/content.js |
Uses shared paste helpers. |
extension/background.js |
Uses shared URL helpers. |
extension/manifest.json |
Loads the shared content script. |
tests/paste_logic.test.cjs |
Tests shared helper behavior. |
.github/workflows/ci.yml |
Runs extension unit tests. |
README.md |
Documents worker behavior and tests. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| expect(ac.hotkeyTrigger == HotkeyTriggerMode::OnRelease, L"default OnRelease"); | ||
| expect(ac.hotkeyReleaseTimeoutMs > 0, L"release timeout default"); | ||
| expect(ac.logLevel == LogLevel::Info, L"default log level Info"); | ||
| expect(App::WM_QP_WORK_DONE >= WM_APP, L"WM_QP_WORK_DONE in WM_APP range"); |
There was a problem hiding this comment.
Fixed in 5c9f63f — --self-test now starts the worker, queues a no-op job, pumps WM_QP_WORK_DONE / busy, then stops idle and mid-job. No clipboard/SendInput in that path.
| { | ||
| const DWORD wr = WaitForSingleObject(workThread_, 20000); | ||
| if (wr == WAIT_TIMEOUT || wr == WAIT_FAILED) | ||
| TerminateThread(workThread_, 1); |
There was a problem hiding this comment.
Fixed in 5c9f63f — removed TerminateThread. StopWorkThread waits INFINITE for the current job to finish, then the worker sees workStop_ and exits.
Ping the loaded MV3 manifest version and compare to PE X.Y.Z. Mismatch shows a one-shot MessageBox (Reload unpacked) plus About/log. Timer retries while the host is not connected; first hotkey also checks. Co-authored-by: Hermes/grok-4.6/m3rcur1al <hermes-m3rcur1al@local>
StopWorkThread waits for the worker to finish the current job. Never TerminateThread (mutex/clipboard/COM). --self-test starts the worker, queues a no-op job, pumps WM_QP_WORK_DONE / busy, then stops idle and mid-job. Co-authored-by: Hermes/grok-4.6/m3rcur1al <hermes-m3rcur1al@local>
|
Addressed Copilot in 5c9f63f (2/2):
Left open. |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 14 out of 14 changed files in this pull request and generated no new comments.
Suppressed comments (2)
src/app.cpp:1078
- This does not ensure the job is active before shutdown starts.
StopWorkThreadsignals the stop event immediately, and because that event is index 0 inWaitForMultipleObjects, the worker can exit without dequeuing or entering the 200 ms job. Synchronize on a worker-started event/latch before callingStopWorkThreadso the active-job shutdown path is actually exercised.
expect(app.QueueHotkeyWork(job), L"work test queue active");
app.StopWorkThread();
src/app.cpp:355
- This runs on the window thread (from both
WM_TIMERandOnWorkDone), butPingblocks inExtBridge::Callfor up to 800 ms. If the native host remains connected while the service worker is stalled, the tray/menu will freeze on every two-second retry, up to 30 times, undermining the responsiveness this PR is intended to provide. Perform the version request asynchronously and post its result back to the window thread instead.
if (!ExtBridge::Instance().Ping(800, &verU))
Summary
The larger leftover from the v1.0.5 review:
Workflow off the UI thread
WM_QP_WORK_DONE(WM_APP+2, not the tray message).EnsureComInitializedis thread_local so UIA fallback works on the worker.WM_CLOSEjoins the worker (up to 20s) before destroy.JS unit tests around paste
extension/paste_logic.jsshared by content script, background, and Node.node --test tests/paste_logic.test.cjs— duplicate detect, HTML escape,isAlreadyOnApp, HTTPS origin allowlist.needs: [format, js].After merge: Reload unpacked companion (
paste_logic.jsis a new content-script file).Test plan
node --test tests/paste_logic.test.cjs— 9 pass--self-testexit 0