Skip to content

Commit f63a6a2

Browse files
committed
feat: suppress lessons in plan mode + drop the per-line blockquote from the card
Two determinism fixes for lesson delivery, both observed failing in a real session (lesson fired during plan mode → couldn't save; card body fell out of the blockquote with code blocks): - The Stop hook now reads permission_mode from the payload (refactor parseStopHookActive -> parseHookPayload, stopHookContinuation -> readHookPayload + shouldSuppressHook) and stays silent when it is 'plan', for both onboard-hook and lesson-ready. No lesson fires while the model can't deliver or save it. The redundant 'Plan-mode suppression' section is dropped from SKILL.md. - The card is now plain markdown between the two ### bands instead of a per-line '>' blockquote, which broke on multi-paragraph bodies and fenced code (even on Sonnet). No code-side body strip (kept the model + simpler format instead).
1 parent 23a29f0 commit f63a6a2

4 files changed

Lines changed: 72 additions & 75 deletions

File tree

assets/SKILL.md

Lines changed: 9 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -22,14 +22,6 @@ by teaching one thing at a time, at the right moment, based on what they actuall
2222

2323
---
2424

25-
## Plan-mode suppression
26-
27-
If you are currently in **plan mode** (plan mode is active and `ExitPlanMode` has not
28-
yet been called in this session), **exit this skill immediately** — do not read any
29-
devcoach MCP resources, do not run onboarding, do not deliver or log any lesson.
30-
31-
---
32-
3325
## Onboarding flow
3426

3527
The onboarding flow runs inline, immediately before delivering a lesson, whenever
@@ -353,21 +345,17 @@ it is central to the task and no applicable profile topic exists.
353345

354346
## 4. Lesson format
355347

356-
Append the lesson **at the bottom of the response**, rendered as a card: a titled
357-
top band, the content **indented as a blockquote**, and a bottom band echoing the
358-
topic and level.
348+
Append the lesson **at the bottom of the response**, rendered as a card: a titled top
349+
band, the lesson as **plain markdown**, and a bottom band echoing the topic and level.
359350

360351
```
361352
### ──────── 🎓 devcoach ────────
362-
> [Category] · Level: [Junior|Mid|Senior]
363-
>
364-
> **[Lesson title]**
365-
>
366-
> [Body: 3–6 paragraphs. Concise, practical, with a code example if useful.
367-
> Explain the WHY, not just the what. Connect it to the task just completed.]
368-
>
369-
> 💡 *Senior tip:* [One sentence a senior would say to a junior on this topic]
353+
**[Lesson title]** · [Category] · [Junior|Mid|Senior]
354+
355+
[Body: 3–6 paragraphs. Concise, practical, with a code example if useful.
356+
Explain the WHY, not just the what. Connect it to the task just completed.]
370357
358+
💡 *Senior tip:* [One sentence a senior would say to a junior on this topic]
371359
### ──────── [topic] · [level] ────────
372360
```
373361

@@ -376,8 +364,8 @@ Card rules:
376364
U+2500) around a centered title — the heading makes Claude Code colour the whole
377365
line. Do **not** use `---` (a real rule can't carry a title). Aim for a band
378366
width of ~34 characters and keep the top and bottom bands the same width.
379-
- Every content line is prefixed with `> ` (blank separators become a bare `>`).
380-
- Leave **one blank line before the bottom band** so the blockquote closes cleanly.
367+
- The body is **plain markdown** between the bands — do **not** prefix lines with `> `
368+
(a per-line blockquote breaks on multi-paragraph bodies and fenced code blocks).
381369

382370
**Display vs storage:** the card above is for chat only. When you call `log_lesson`,
383371
the `body` is **clean markdown** — no bands, no `> ` blockquote, and no title/`Category ·

plugin/skills/devcoach/SKILL.md

Lines changed: 9 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -22,14 +22,6 @@ by teaching one thing at a time, at the right moment, based on what they actuall
2222

2323
---
2424

25-
## Plan-mode suppression
26-
27-
If you are currently in **plan mode** (plan mode is active and `ExitPlanMode` has not
28-
yet been called in this session), **exit this skill immediately** — do not read any
29-
devcoach MCP resources, do not run onboarding, do not deliver or log any lesson.
30-
31-
---
32-
3325
## Onboarding flow
3426

3527
The onboarding flow runs inline, immediately before delivering a lesson, whenever
@@ -353,21 +345,17 @@ it is central to the task and no applicable profile topic exists.
353345

354346
## 4. Lesson format
355347

356-
Append the lesson **at the bottom of the response**, rendered as a card: a titled
357-
top band, the content **indented as a blockquote**, and a bottom band echoing the
358-
topic and level.
348+
Append the lesson **at the bottom of the response**, rendered as a card: a titled top
349+
band, the lesson as **plain markdown**, and a bottom band echoing the topic and level.
359350

360351
```
361352
### ──────── 🎓 devcoach ────────
362-
> [Category] · Level: [Junior|Mid|Senior]
363-
>
364-
> **[Lesson title]**
365-
>
366-
> [Body: 3–6 paragraphs. Concise, practical, with a code example if useful.
367-
> Explain the WHY, not just the what. Connect it to the task just completed.]
368-
>
369-
> 💡 *Senior tip:* [One sentence a senior would say to a junior on this topic]
353+
**[Lesson title]** · [Category] · [Junior|Mid|Senior]
354+
355+
[Body: 3–6 paragraphs. Concise, practical, with a code example if useful.
356+
Explain the WHY, not just the what. Connect it to the task just completed.]
370357
358+
💡 *Senior tip:* [One sentence a senior would say to a junior on this topic]
371359
### ──────── [topic] · [level] ────────
372360
```
373361

@@ -376,8 +364,8 @@ Card rules:
376364
U+2500) around a centered title — the heading makes Claude Code colour the whole
377365
line. Do **not** use `---` (a real rule can't carry a title). Aim for a band
378366
width of ~34 characters and keep the top and bottom bands the same width.
379-
- Every content line is prefixed with `> ` (blank separators become a bare `>`).
380-
- Leave **one blank line before the bottom band** so the blockquote closes cleanly.
367+
- The body is **plain markdown** between the bands — do **not** prefix lines with `> `
368+
(a per-line blockquote breaks on multi-paragraph bodies and fenced code blocks).
381369

382370
**Display vs storage:** the card above is for chat only. When you call `log_lesson`,
383371
the `body` is **clean markdown** — no bands, no `> ` blockquote, and no title/`Category ·

src/cli/commands.ts

Lines changed: 37 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -719,39 +719,50 @@ async function cmdSetup(): Promise<void> {
719719
// ── Stop hooks ───────────────────────────────────────────────────────────────
720720

721721
/**
722-
* Claude Code passes the Stop-hook payload as JSON on stdin. `stop_hook_active`
723-
* is true when the current stop is itself the result of a Stop hook forcing the
724-
* agent to continue — re-blocking then would loop forever. Returns true only for
725-
* a well-formed payload with the flag set; empty/garbage input is treated as a
726-
* fresh stop.
722+
* Claude Code passes the Stop-hook payload as JSON on stdin. We use two fields:
723+
* `stop_hook_active` (true when this stop is itself a hook-forced continuation —
724+
* re-blocking then would loop forever) and `permission_mode` (`"plan"` while the user
725+
* is planning, when the model cannot deliver or save a lesson). Empty/garbage input is
726+
* treated as a fresh, non-plan stop.
727727
*/
728-
export function parseStopHookActive(raw: string): boolean {
728+
export interface HookPayload {
729+
stop_hook_active: boolean;
730+
permission_mode: string | null;
731+
}
732+
733+
export function parseHookPayload(raw: string): HookPayload {
729734
try {
730-
const parsed = JSON.parse(raw);
731-
return parsed?.stop_hook_active === true;
735+
const p = JSON.parse(raw);
736+
return {
737+
stop_hook_active: p?.stop_hook_active === true,
738+
permission_mode: typeof p?.permission_mode === "string" ? p.permission_mode : null,
739+
};
732740
} catch {
733-
return false;
741+
return { stop_hook_active: false, permission_mode: null };
734742
}
735743
}
736744

737745
/**
738-
* Read the Stop-hook payload from stdin, but only when it is actually piped
739-
* (a FIFO or a redirected file). On a TTY or any other source we skip the read
740-
* entirely so an interactive run — or a test — never blocks waiting for EOF.
746+
* Read the Stop-hook payload from stdin, but only when it is actually piped (Claude
747+
* Code delivers it over a SOCKET; a FIFO or a redirected file carry it too). Skip a
748+
* TTY/character device, where a blocking read would hang an interactive run or a test.
741749
*/
742-
function stopHookContinuation(): boolean {
750+
function readHookPayload(): HookPayload {
751+
const empty: HookPayload = { stop_hook_active: false, permission_mode: null };
743752
try {
744753
const st = fstatSync(0);
745-
// Claude Code delivers the Stop-hook payload over a SOCKET; a piped FIFO or a
746-
// redirected file carry it too. Read all three. Skip only a TTY/character device,
747-
// where a blocking read would hang an interactive run or a test.
748-
if (!st.isFIFO() && !st.isFile() && !st.isSocket()) return false;
749-
return parseStopHookActive(readFileSync(0, "utf8"));
754+
if (!st.isFIFO() && !st.isFile() && !st.isSocket()) return empty;
755+
return parseHookPayload(readFileSync(0, "utf8"));
750756
} catch {
751-
return false;
757+
return empty;
752758
}
753759
}
754760

761+
/** A Stop hook stays silent on a hook-forced continuation, or while in plan mode. */
762+
function shouldSuppressHook(p: HookPayload): boolean {
763+
return p.stop_hook_active || p.permission_mode === "plan";
764+
}
765+
755766
/**
756767
* Block the stop without the non-zero "Stop hook error" framing. Claude Code reads
757768
* `{decision:"block", reason}` from stdout (exit 0), blocks the stop, and feeds
@@ -764,7 +775,7 @@ function emitBlock(reason: string): never {
764775
}
765776

766777
function cmdOnboardHook(): void {
767-
if (stopHookContinuation()) process.exit(0);
778+
if (shouldSuppressHook(readHookPayload())) process.exit(0);
768779
// No DB yet → onboarding has not run. Cue WITHOUT creating coaching.db or any
769780
// marker file, so an interrupted onboarding leaves nothing behind and re-cues on
770781
// the next task. The artifacts appear only when complete_onboarding actually runs.
@@ -793,7 +804,7 @@ function cmdOnboardHook(): void {
793804
const NOTEBOOK_UPDATE_EVERY = 10;
794805

795806
function cmdLessonReady(): void {
796-
if (stopHookContinuation()) process.exit(0);
807+
if (shouldSuppressHook(readHookPayload())) process.exit(0);
797808
// No DB → no profile yet. Stay silent without creating coaching.db.
798809
if (!existsSync(db.DB_PATH)) process.exit(0);
799810
let result: { allowed: boolean; nextLessonNumber: number };
@@ -837,12 +848,12 @@ function cmdLessonReady(): void {
837848
"its 'Recommended focus', calibrate depth from 'Recurring patterns', and watch its 'Open " +
838849
"hypotheses'. Pick ONE devcoach://profile topic NOT already in devcoach://taught-topics, " +
839850
"pitched at or above the user's confidence band.\n" +
840-
"4. FIRST write the lesson as your visible reply — a card in EXACTLY this format:\n\n" +
851+
"4. FIRST write the lesson as your visible reply — in EXACTLY this format. It is plain " +
852+
"markdown between two band headings; do NOT prefix lines with '>' (no blockquote):\n\n" +
841853
"### ──────── 🎓 devcoach ────────\n" +
842-
"> [Category] · Level: [Junior|Mid|Senior]\n" +
843-
"> **[Title]**\n" +
844-
"> [3–6 short paragraphs: explain the why, tie it to the task; fenced code if useful]\n" +
845-
"> 💡 *Senior tip:* [one line]\n" +
854+
"**[Title]** · [Category] · [Junior|Mid|Senior]\n\n" +
855+
"[3–6 short paragraphs: explain the why, tie it to the task; fenced code if useful]\n\n" +
856+
"💡 *Senior tip:* [one line]\n" +
846857
"### ──────── [topic] · [level] ────────\n\n" +
847858
"5. ONLY AFTER the card is written, call log_lesson. Put title/topic_id/categories/level/" +
848859
"summary in their own fields; body = ONLY the prose + the 💡 tip as CLEAN markdown (no " +

tests/cli.test.ts

Lines changed: 17 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@ import { existsSync, mkdirSync, mkdtempSync, readFileSync, rmSync, writeFileSync
22
import { tmpdir } from "node:os";
33
import { dirname, join } from "node:path";
44
import { describe, expect, it, vi } from "vitest";
5-
import { parseStopHookActive, runCli } from "../src/cli/commands";
5+
import { parseHookPayload, runCli } from "../src/cli/commands";
66
import * as db from "../src/core/db";
77
import { parseLesson } from "../src/core/models";
88
import { VERSION } from "../src/version";
@@ -116,12 +116,22 @@ describe("cli", () => {
116116
expect(due.out).toContain("call update_notebook");
117117
});
118118

119-
it("parseStopHookActive: only a payload with stop_hook_active=true short-circuits", () => {
120-
expect(parseStopHookActive(JSON.stringify({ stop_hook_active: true }))).toBe(true);
121-
expect(parseStopHookActive(JSON.stringify({ stop_hook_active: false }))).toBe(false);
122-
expect(parseStopHookActive(JSON.stringify({ hook_event_name: "Stop" }))).toBe(false);
123-
expect(parseStopHookActive("")).toBe(false);
124-
expect(parseStopHookActive("not json")).toBe(false);
119+
it("parseHookPayload: extracts stop_hook_active and permission_mode", () => {
120+
expect(parseHookPayload(JSON.stringify({ stop_hook_active: true })).stop_hook_active).toBe(
121+
true,
122+
);
123+
expect(parseHookPayload(JSON.stringify({ stop_hook_active: false })).stop_hook_active).toBe(
124+
false,
125+
);
126+
expect(parseHookPayload(JSON.stringify({ permission_mode: "plan" })).permission_mode).toBe(
127+
"plan",
128+
);
129+
expect(parseHookPayload(JSON.stringify({ permission_mode: "default" })).permission_mode).toBe(
130+
"default",
131+
);
132+
const empty = parseHookPayload("not json");
133+
expect(empty.stop_hook_active).toBe(false);
134+
expect(empty.permission_mode).toBe(null);
125135
});
126136

127137
it("knowledge + group + settings commands", async () => {

0 commit comments

Comments
 (0)