feat: add conditional /watch polling and beep alerts#447
feat: add conditional /watch polling and beep alerts#447alectimison-maker wants to merge 4 commits into
Conversation
|
@alectimison-maker is attempting to deploy a commit to the esokullu's projects Team on Vercel. A member of the Team first needs to authorize it. |
| }, | ||
| scheduledAt: createdAt, | ||
| nextRunAt: iso(this.now() + 1000), | ||
| immediate: true, |
There was a problem hiding this comment.
[bug] Watch jobs are created with immediate: true and nextRunAt ≈ now+1s, then _completeWatch sets immediate: false and advances nextRunAt by the watch interval. sameScheduledIntent still requires matching scheduledJobIsImmediate and near scheduled times, so after the first poll an identical /watch is no longer treated as a duplicate live job. Users can stack many concurrent Act-mode watches for the same page/condition (amplified by 30–120s intervals and LLM cost).
Suggestion: For source === 'watch', dedupe live jobs by target + watch payload only (ignore immediate / near-time). Extend the existing “identical live watch should be deduped” test to re-create after one successful partial poll.
| } else if (event === 'completed') { | ||
| ensureScheduledTerminalMessage(job); | ||
| await settleScheduledRun(event, job, runTabId); | ||
| } else if (event === 'polled' || event === 'triggered') { |
There was a problem hiding this comment.
[bug] Keep watches emit running on every poll, and the existing handler always does addMessage('assistant', '') with the same dataset.scheduledJobId. The new polled/triggered path only settles the first matching assistant node, and only fills text when that node is still empty. Result: each 30–120s poll leaves an additional empty assistant bubble in chat, while later observations never replace the first filled one.
Suggestion: For job.source === 'watch' on running, reuse findScheduledAssistantMessageForJob(jobId) (or a single sticky run row) instead of creating a new message; on polled/triggered, update that message’s text to the latest lastResult rather than gating on !textEl.textContent.trim().
| target: { | ||
| type: 'url', | ||
| url: parsed.url, | ||
| ...(tabId != null ? { tabId } : {}), |
There was a problem hiding this comment.
[bug] createWatchJob stores the initiating tabId on a type: 'url' target. Existing _resolveTab will tabs.update that same tab back to the watched URL whenever the tab has navigated away. At 30–120s this repeatedly hijacks the user’s active browsing session (and can wipe SPA state) instead of following the page or using a background tab.
Suggestion: Prefer a dedicated inactive tab for watches: omit initiating tabId from the URL target (always tabs.create({ active: false })), or if URL diverges create/reuse a background tab rather than forcing tabs.update on the original tab. Document the chosen binding semantics in README/architecture.
| && !!eventKey | ||
| && !duplicateAlert | ||
| && eventKey !== job.watch?.lastTriggeredEventKey; | ||
| const alertContractFailed = lastOutcome === 'success' |
There was a problem hiding this comment.
[suggestion] For /beep watches, any done(outcome="success") without a fresh armed event_key permanently fails the entire job (alertContractFailed). That is intentionally fail-closed for audio, but local/small models frequently omit optional tools; a single miss destroys a long-running --keep watch after it may already have performed the user-requested action.
Suggestion: Consider treating missing arm as a non-fatal “success without alert” (complete/continue without sound, record a warning), or only fail closed when beep was armed with a key that fails the contract. Keep the stronger fail-closed path if product intent is strict, but call it out in the slash help / created message.
| @@ -2464,7 +2488,10 @@ async function settleScheduledRun(event, job, tabId = currentTabId) { | |||
| if (assistantEl) { | |||
There was a problem hiding this comment.
[suggestion] Settlement still only injects job.lastResult when the assistant text is empty. Even after Issue 2 is fixed to reuse one bubble, keep-watch users will not see refreshed observations in chat unless this condition is relaxed for polled/triggered.
Suggestion: For watch poll events, always replace/update the message text with the latest observation (still via formatMarkdown), and keep the empty-only gate for true terminal schedule completions if desired.
| @@ -297,6 +298,13 @@ export default { | |||
| "sp.slash.record_transcribe": "Record and save a Whisper transcript after stop", | |||
There was a problem hiding this comment.
[nit] /watch is outOfBand: true and works while busy, but sp.slash.busy_only_oob still lists only the older OOB commands and does not mention /watch.
Suggestion: Add /watch to the busy-only OOB toast string in all locales (Chrome and Firefox).
2 participants
Summary
/watch [--keep] [--secs <30-120>] [--long | --short] <condition and action> [/beep]for immediate, page-bound conditional pollingpartialpolls, one-shot/keep outcome handling, safe untrusted observation replay, and duplicate coalescingbeep({event_key, message?})tool only to/beepwatches; persist and dedupe event keys before background audio, and play only after verifieddone(outcome="success")notifySoundwithout a second side-panel chimeSafety properties
beep; one poll can arm only one fresh event keyTests
node test/run.js: 1142 passed, with only 2 pre-existingmainfailures (changelog25.4.0vs package25.4.2; locale-dependent WebBrain promotion title)npm run test:security: 60/60 passednpm test: reaches the same two pre-existingnode test/run.jsfailures, so its&&does not start the security command; the security command was run separately aboveCloses #410