-
Notifications
You must be signed in to change notification settings - Fork 1.6k
Fix duplicate post-turn idle blocked alerts for unchanged sessions #1175
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from 1 commit
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -68,6 +68,79 @@ const RALPH_ACTIVE_PROGRESS_PHASES = new Set([ | |
| 'fixing', | ||
| ]); | ||
|
|
||
| const IDLE_NOTIFICATION_SUMMARY_MAX_LENGTH = 240; | ||
|
|
||
| function summarizeIdleNotificationMessage(message: unknown): string { | ||
| const source = safeString(message) | ||
| .split('\n') | ||
| .map((line) => line.trim()) | ||
| .filter(Boolean); | ||
| const preferred = source.at(-1) || ''; | ||
| const normalized = preferred.replace(/\s+/g, ' ').trim(); | ||
|
Comment on lines
+75
to
+79
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
The fingerprint summary is taken from the last non-empty line of the assistant message. For multi-line replies that end with stable boilerplate (for example, a repeated closing prompt), different idle/blocker updates can collapse to the same summary and be treated as unchanged, so Useful? React with 👍 / 👎. |
||
| if (!normalized) return ''; | ||
| return normalized.length > IDLE_NOTIFICATION_SUMMARY_MAX_LENGTH | ||
| ? `${normalized.slice(0, IDLE_NOTIFICATION_SUMMARY_MAX_LENGTH - 1)}…` | ||
| : normalized; | ||
| } | ||
|
|
||
| function classifyIdleNotificationPhase(message: unknown): 'idle' | 'progress' | 'finished' | 'failed' { | ||
| const lower = safeString(message).toLowerCase(); | ||
| if (!lower) return 'idle'; | ||
|
|
||
| if (/(error|failed|exception|invalid|timed out|timeout)/i.test(lower)) { | ||
| return 'failed'; | ||
| } | ||
|
|
||
| if ([ | ||
| 'all tests pass', | ||
| 'build succeeded', | ||
| 'completed', | ||
| 'complete', | ||
| 'done', | ||
| 'final summary', | ||
| 'summary', | ||
| ].some((pattern) => lower.includes(pattern))) { | ||
| return 'finished'; | ||
| } | ||
|
|
||
| if ([ | ||
| 'verify', | ||
| 'verified', | ||
| 'verification', | ||
| 'review', | ||
| 'reviewed', | ||
| 'diagnostic', | ||
| 'typecheck', | ||
| 'test', | ||
| 'implement', | ||
| 'implemented', | ||
| 'apply patch', | ||
| 'change', | ||
| 'fix', | ||
| 'update', | ||
| 'refactor', | ||
| 'resume', | ||
| 'resumed', | ||
| 'progress', | ||
| 'continue', | ||
| 'continued', | ||
| ].some((pattern) => lower.includes(pattern))) { | ||
| return 'progress'; | ||
| } | ||
|
|
||
| return 'idle'; | ||
| } | ||
|
|
||
| function buildIdleNotificationFingerprint(payload: Record<string, unknown>): string { | ||
| const lastAssistantMessage = safeString(payload['last-assistant-message'] || payload.last_assistant_message || ''); | ||
| const summary = summarizeIdleNotificationMessage(lastAssistantMessage); | ||
| const phase = classifyIdleNotificationPhase(lastAssistantMessage); | ||
| return JSON.stringify({ | ||
| phase, | ||
| ...(summary ? { summary } : {}), | ||
| }); | ||
| } | ||
|
|
||
| async function main() { | ||
| const rawPayload = process.argv[process.argv.length - 1]; | ||
| if (!rawPayload || rawPayload.startsWith('-')) { | ||
|
|
@@ -443,19 +516,20 @@ async function main() { | |
| const { notifyLifecycle } = await import('../notifications/index.js'); | ||
| const { shouldSendIdleNotification, recordIdleNotificationSent } = await import('../notifications/idle-cooldown.js'); | ||
| const sessionJsonPath = join(stateDir, 'session.json'); | ||
| const idleFingerprint = buildIdleNotificationFingerprint(payload); | ||
| let notifySessionId = ''; | ||
| try { | ||
| const sessionData = JSON.parse(await readFile(sessionJsonPath, 'utf-8')); | ||
| notifySessionId = safeString(sessionData && sessionData.session_id ? sessionData.session_id : ''); | ||
| } catch { /* no session file */ } | ||
|
|
||
| if (notifySessionId && shouldSendIdleNotification(stateDir, notifySessionId)) { | ||
| if (notifySessionId && shouldSendIdleNotification(stateDir, notifySessionId, idleFingerprint)) { | ||
| const idleResult = await notifyLifecycle('session-idle', { | ||
| sessionId: notifySessionId, | ||
| projectPath: cwd, | ||
| }); | ||
| if (idleResult && idleResult.anySuccess) { | ||
| recordIdleNotificationSent(stateDir, notifySessionId); | ||
| recordIdleNotificationSent(stateDir, notifySessionId, idleFingerprint); | ||
| } | ||
| try { | ||
| const { buildNativeHookEvent } = await import('../hooks/extensibility/events.js'); | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
shouldSendIdleNotificationnow treatsOMX_IDLE_COOLDOWN_SECONDS=0as “always send” only when no fingerprint is passed, butnotify-hookalways passes a fingerprint. In that common path, unchanged fingerprints are still suppressed, which regresses the documented behavior that cooldown0disables throttling entirely and can silently drop repeated idle notifications/hooks for users who explicitly opted out of throttling.Useful? React with 👍 / 👎.