Skip to content

Commit 1581b6b

Browse files
committed
fix: note nudges now work outside fullFeatureMode and only clear state after successful placement
Root cause: note nudge delivery was gated by fullFeatureMode (false for subagent sessions). Triggers set triggerPending, but delivery never ran. Secondary bug: getNoteNudgeText cleared triggerPending before confirming the append succeeded, causing silent drops. Fixes: - Moved note nudge injection outside the fullFeatureMode block - Split getNoteNudgeText into peekNoteNudgeText + markNoteNudgeDelivered - Only mark delivered after appendReminderToLatestUserMessage succeeds - Added logging for trigger/delivery/skip events
1 parent 3205591 commit 1581b6b

2 files changed

Lines changed: 44 additions & 16 deletions

File tree

src/hooks/magic-context/note-nudger.ts

Lines changed: 31 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@
1515

1616
import type { Database } from "bun:sqlite";
1717
import { getSessionNotes } from "../../features/magic-context/storage-notes";
18+
import { sessionLog } from "../../shared/logger";
1819

1920
export type NoteNudgeTrigger = "historian_complete" | "commit_detected" | "todos_complete";
2021

@@ -39,32 +40,53 @@ function getState(sessionId: string): NoteNudgeState {
3940
/**
4041
* Signal that a trigger event occurred. Call from hook layer when any of the 3 triggers fire.
4142
*/
42-
export function onNoteTrigger(sessionId: string, _trigger: NoteNudgeTrigger): void {
43+
export function onNoteTrigger(sessionId: string, trigger: NoteNudgeTrigger): void {
4344
const state = getState(sessionId);
4445
state.triggerPending = true;
46+
sessionLog(sessionId, `note-nudge: trigger fired (${trigger}), triggerPending=true`);
4547
}
4648

4749
/**
48-
* Check whether a note nudge should be injected during this transform pass.
50+
* Peek at whether a note nudge should be injected during this transform pass.
4951
* Returns the nudge text if yes, null if no.
50-
*
51-
* Call from the nudge injection path (transform or nudger).
52+
* Does NOT clear triggerPending — call markNoteNudgeDelivered() after successful placement.
5253
*/
53-
export function getNoteNudgeText(db: Database, sessionId: string): string | null {
54+
export function peekNoteNudgeText(db: Database, sessionId: string): string | null {
5455
const state = getState(sessionId);
5556

5657
if (!state.triggerPending) return null;
5758

5859
// Check if there are actually notes to remind about
5960
const notes = getSessionNotes(db, sessionId);
60-
if (notes.length === 0) return null;
61+
if (notes.length === 0) {
62+
sessionLog(sessionId, "note-nudge: triggerPending but no notes found, skipping");
63+
return null;
64+
}
65+
66+
sessionLog(sessionId, `note-nudge: delivering nudge for ${notes.length} notes`);
67+
const plural = notes.length === 1 ? "note" : "notes";
68+
return `You have ${notes.length} deferred ${plural}. Review with ctx_note read — some may be actionable now.`;
69+
}
6170

62-
// Deliver the nudge
71+
/**
72+
* Mark the note nudge as delivered after successful placement.
73+
* Only call after appendReminderToLatestUserMessage returns a truthy anchor.
74+
*/
75+
export function markNoteNudgeDelivered(sessionId: string): void {
76+
const state = getState(sessionId);
6377
state.triggerPending = false;
6478
state.nudgeDelivered = true;
79+
sessionLog(sessionId, "note-nudge: marked delivered");
80+
}
6581

66-
const plural = notes.length === 1 ? "note" : "notes";
67-
return `You have ${notes.length} deferred ${plural}. Review with ctx_note read — some may be actionable now.`;
82+
/**
83+
* Legacy wrapper — peek + mark in one call.
84+
* Kept for existing tests; prefer peekNoteNudgeText + markNoteNudgeDelivered in production.
85+
*/
86+
export function getNoteNudgeText(db: Database, sessionId: string): string | null {
87+
const text = peekNoteNudgeText(db, sessionId);
88+
if (text) markNoteNudgeDelivered(sessionId);
89+
return text;
6890
}
6991

7092
/**

src/hooks/magic-context/transform-postprocess-phase.ts

Lines changed: 13 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@ import {
1818
type PreparedCompartmentInjection,
1919
renderCompartmentInjection,
2020
} from "./inject-compartments";
21-
import { getNoteNudgeText } from "./note-nudger";
21+
import { markNoteNudgeDelivered, peekNoteNudgeText } from "./note-nudger";
2222
import { reinjectNudgeAtAnchor } from "./nudge-injection";
2323
import type { NudgePlacementStore } from "./nudge-placement-store";
2424
import type { ContextNudge } from "./nudger";
@@ -377,13 +377,19 @@ export function runPostTransformPhase(args: RunPostTransformPhaseArgs): void {
377377
);
378378
}
379379
}
380-
381-
const deferredNoteText = getNoteNudgeText(args.db, args.sessionId);
382-
if (deferredNoteText) {
383-
const noteInstruction = `\n\n<instruction name="deferred_notes">${deferredNoteText}</instruction>`;
384-
appendReminderToLatestUserMessage(args.messages, noteInstruction);
385-
}
386380
} else {
387381
args.nudgePlacements.clear(args.sessionId);
388382
}
383+
384+
// Note nudges run outside fullFeatureMode — they should work in all sessions
385+
// including subagent sessions where fullFeatureMode is false.
386+
const deferredNoteText = peekNoteNudgeText(args.db, args.sessionId);
387+
if (deferredNoteText) {
388+
const noteInstruction = `\n\n<instruction name="deferred_notes">${deferredNoteText}</instruction>`;
389+
const anchored = appendReminderToLatestUserMessage(args.messages, noteInstruction);
390+
if (anchored) {
391+
// Only mark delivered after successful placement
392+
markNoteNudgeDelivered(args.sessionId);
393+
}
394+
}
389395
}

0 commit comments

Comments
 (0)