Skip to content

Commit 5483a04

Browse files
committed
fix(hooks): emit correct PostToolUse schema as context notification
PostToolUse hook was emitting PreToolUse-style permissionDecision/deny payloads, which Claude Code silently ignores for post-hoc events. Three call sites were broken: - CONFLICT after agent edit - DRIFT_ONLY after agent edit - auto-synced confirmation Fix: replace _print_decision with _emit_decision that dispatches to typed dataclasses. PostToolUse paths now emit hookSpecificOutput.additionalContext via PostToolUseContext; PreToolUse paths use PreToolUseDecision. Remove old HookDecision/HookEvent enums from hooks_cmd.py (moved to hook_decision.py). Add _post_drift_notice() to reframe drift messages as notifications. Update TestPairDriftGuardPost and add TestPostToolUseSchema to assert the correct wire schema. Delete TestHookEnums (covered by test_hook_decision.py).
1 parent 8d8d39b commit 5483a04

3 files changed

Lines changed: 193 additions & 159 deletions

File tree

jupyter_jcli/commands/hooks_cmd.py

Lines changed: 111 additions & 114 deletions
Original file line numberDiff line numberDiff line change
@@ -5,33 +5,18 @@
55
import re
66
import subprocess
77
import sys
8-
from enum import Enum
98
from pathlib import Path
109

1110
import click
1211

1312
from jupyter_jcli._enums import DriftStatus
1413
from jupyter_jcli.hook_debug import HookDebugLogger, read_hook_stdin
15-
16-
17-
class HookDecision(str, Enum):
18-
"""Permission decision values for Claude Code hook payloads.
19-
20-
Values are constrained by the Claude Code PreToolUse hook protocol.
21-
Changing them requires synchronising with the Claude Code harness.
22-
"""
23-
DENY = "deny"
24-
ASK = "ask"
25-
ALLOW = "allow"
26-
27-
28-
class HookEvent(str, Enum):
29-
"""Hook event names emitted in hook payloads.
30-
31-
Values are constrained by the Claude Code hook protocol.
32-
"""
33-
PRE_TOOL_USE = "PreToolUse"
34-
POST_TOOL_USE = "PostToolUse"
14+
from jupyter_jcli.hook_decision import (
15+
HookDecision,
16+
PostToolUseContext,
17+
PreToolUseDecision,
18+
PreToolUseOutcome,
19+
)
3520

3621
# ---------------------------------------------------------------------------
3722
# Guard patterns — each entry is (label, compiled_regex).
@@ -125,7 +110,7 @@ def nbconvert_guard(debug: bool):
125110
inner = unwrap_runner(sc)
126111
label = _check_exec_guard(inner)
127112
if label is not None:
128-
_print_decision(HookDecision.DENY, _HINT.format(label=label), logger=log)
113+
_emit_decision(PreToolUseDecision(PreToolUseOutcome.DENY, _HINT.format(label=label)), logger=log)
129114
sys.exit(0)
130115

131116
sys.exit(0)
@@ -197,12 +182,14 @@ def python_run_guard(debug: bool):
197182
log.record_exception(exc)
198183
sys.exit(0)
199184
if ipynb is not None:
200-
_print_decision(
201-
HookDecision.DENY,
202-
_PYTHON_HINT.format(
203-
label="python script",
204-
file=file_str,
205-
ipynb=ipynb.name,
185+
_emit_decision(
186+
PreToolUseDecision(
187+
PreToolUseOutcome.DENY,
188+
_PYTHON_HINT.format(
189+
label="python script",
190+
file=file_str,
191+
ipynb=ipynb.name,
192+
),
206193
),
207194
logger=log,
208195
)
@@ -239,14 +226,16 @@ def pair_drift_guard_pre(debug: bool) -> None:
239226
path = Path(file_path)
240227

241228
if path.suffix == ".ipynb":
242-
_print_decision(
243-
HookDecision.DENY,
244-
f"Direct Edit/Write of `{path.name}` is not supported — edit notebooks "
245-
"via the py:percent round-trip instead:\n"
246-
f" 1. j-cli convert ipynb-to-py {path.name} {path.stem}.py\n"
247-
f" 2. Edit {path.stem}.py with Edit/Write\n"
248-
f" 3. j-cli convert py-to-ipynb {path.stem}.py {path.name}\n"
249-
"(Outputs in the `.ipynb` are preserved through the round-trip.)",
229+
_emit_decision(
230+
PreToolUseDecision(
231+
PreToolUseOutcome.DENY,
232+
f"Direct Edit/Write of `{path.name}` is not supported — edit notebooks "
233+
"via the py:percent round-trip instead:\n"
234+
f" 1. j-cli convert ipynb-to-py {path.name} {path.stem}.py\n"
235+
f" 2. Edit {path.stem}.py with Edit/Write\n"
236+
f" 3. j-cli convert py-to-ipynb {path.stem}.py {path.name}\n"
237+
"(Outputs in the `.ipynb` are preserved through the round-trip.)",
238+
),
250239
logger=log,
251240
)
252241
sys.exit(0)
@@ -294,46 +283,50 @@ def _run_pre_drift_check(path: Path, logger=None) -> None:
294283

295284
if result.status == DriftStatus.CONFLICT:
296285
idx_str = ", ".join(str(i) for i in result.conflict_indices)
297-
_print_decision(
298-
HookDecision.DENY,
299-
f"Pre-existing conflict between `{py_path.name}` and `{ipynb_path.name}` "
300-
f"at cell(s) [{idx_str}] — both sides have been edited (e.g. by a human "
301-
"user in JupyterLab and via py:percent) since the last commit of `.py`, "
302-
"and the edits collide on the same cell(s). This drift existed before "
303-
"your tool call.\n\n"
304-
f"Before resolving, run `git diff -- {py_path.name}` to see what changed "
305-
f"on the `.py` side, and open `{ipynb_path.name}` (or jupyter-lab) to "
306-
"inspect the other side. Then pick a direction:\n"
307-
f" j-cli convert ipynb-to-py {ipynb_path.name} {py_path.name}"
308-
" # takes ipynb's cells; discards .py's edits\n"
309-
f" j-cli convert py-to-ipynb {py_path.name} {ipynb_path.name}"
310-
" # takes .py's cells; discards ipynb's edits"
311-
+ _diff_section(result.diff_text, py_path.name),
286+
_emit_decision(
287+
PreToolUseDecision(
288+
PreToolUseOutcome.DENY,
289+
f"Pre-existing conflict between `{py_path.name}` and `{ipynb_path.name}` "
290+
f"at cell(s) [{idx_str}] — both sides have been edited (e.g. by a human "
291+
"user in JupyterLab and via py:percent) since the last commit of `.py`, "
292+
"and the edits collide on the same cell(s). This drift existed before "
293+
"your tool call.\n\n"
294+
f"Before resolving, run `git diff -- {py_path.name}` to see what changed "
295+
f"on the `.py` side, and open `{ipynb_path.name}` (or jupyter-lab) to "
296+
"inspect the other side. Then pick a direction:\n"
297+
f" j-cli convert ipynb-to-py {ipynb_path.name} {py_path.name}"
298+
" # takes ipynb's cells; discards .py's edits\n"
299+
f" j-cli convert py-to-ipynb {py_path.name} {ipynb_path.name}"
300+
" # takes .py's cells; discards ipynb's edits"
301+
+ _diff_section(result.diff_text, py_path.name),
302+
),
312303
logger=logger,
313304
)
314305
return
315306

316307
if result.status == DriftStatus.DRIFT_ONLY:
317-
_print_decision(
318-
HookDecision.DENY,
319-
f"`{py_path.name}` is not yet committed, so jcli has no baseline to "
320-
f"auto-merge the pair. Current sources of `{py_path.name}` and "
321-
f"`{ipynb_path.name}` differ. This state existed before your tool call.\n\n"
322-
"This usually happens right after creating a new notebook (common "
323-
"`j-cli exec` flow: create `.py`, exec to generate `.ipynb` with outputs; "
324-
"the two can drift in whitespace/cell count before the first commit).\n\n"
325-
"Before picking a side:\n"
326-
f" 1. Run `git log --oneline -- {py_path.name}` to confirm `.py` really "
327-
"is new (no HEAD).\n"
328-
" 2. Run `git status` and check who/what wrote each side most recently.\n"
329-
f" 3. If `{ipynb_path.name}` has exec outputs you want to keep, take "
330-
f"`{ipynb_path.name}` as truth; otherwise take `{py_path.name}`.\n\n"
331-
"Then, once you've decided:\n"
332-
f" j-cli convert ipynb-to-py {ipynb_path.name} {py_path.name}"
333-
" # overwrites .py\n"
334-
f" j-cli convert py-to-ipynb {py_path.name} {ipynb_path.name}"
335-
" # overwrites .ipynb sources (outputs preserved)"
336-
+ _diff_section(result.diff_text, py_path.name),
308+
_emit_decision(
309+
PreToolUseDecision(
310+
PreToolUseOutcome.DENY,
311+
f"`{py_path.name}` is not yet committed, so jcli has no baseline to "
312+
f"auto-merge the pair. Current sources of `{py_path.name}` and "
313+
f"`{ipynb_path.name}` differ. This state existed before your tool call.\n\n"
314+
"This usually happens right after creating a new notebook (common "
315+
"`j-cli exec` flow: create `.py`, exec to generate `.ipynb` with outputs; "
316+
"the two can drift in whitespace/cell count before the first commit).\n\n"
317+
"Before picking a side:\n"
318+
f" 1. Run `git log --oneline -- {py_path.name}` to confirm `.py` really "
319+
"is new (no HEAD).\n"
320+
" 2. Run `git status` and check who/what wrote each side most recently.\n"
321+
f" 3. If `{ipynb_path.name}` has exec outputs you want to keep, take "
322+
f"`{ipynb_path.name}` as truth; otherwise take `{py_path.name}`.\n\n"
323+
"Then, once you've decided:\n"
324+
f" j-cli convert ipynb-to-py {ipynb_path.name} {py_path.name}"
325+
" # overwrites .py\n"
326+
f" j-cli convert py-to-ipynb {py_path.name} {ipynb_path.name}"
327+
" # overwrites .ipynb sources (outputs preserved)"
328+
+ _diff_section(result.diff_text, py_path.name),
329+
),
337330
logger=logger,
338331
)
339332
return
@@ -403,35 +396,41 @@ def _apply_merge_and_decide(
403396

404397
if wrote_target:
405398
other = ipynb_path if target == py_path else py_path
406-
_print_decision(
407-
HookDecision.DENY,
408-
f"Someone else edited the paired `{other.name}` before your edit — the "
409-
f"changes have been auto-merged into `{target.name}`. Re-read `{target.name}` "
410-
"so your next Edit sees the updated content. "
411-
"(This drift existed before your tool call; you did not cause it.)",
399+
_emit_decision(
400+
PreToolUseDecision(
401+
PreToolUseOutcome.DENY,
402+
f"Someone else edited the paired `{other.name}` before your edit — the "
403+
f"changes have been auto-merged into `{target.name}`. Re-read `{target.name}` "
404+
"so your next Edit sees the updated content. "
405+
"(This drift existed before your tool call; you did not cause it.)",
406+
),
412407
logger=logger,
413408
)
414409

415410

416-
def _print_decision(
417-
decision: HookDecision,
418-
reason: str,
419-
event: HookEvent = HookEvent.PRE_TOOL_USE,
420-
logger=None,
421-
) -> None:
422-
payload = {
423-
"hookSpecificOutput": {
424-
"hookEventName": event,
425-
"permissionDecision": decision,
426-
"permissionDecisionReason": reason,
427-
}
428-
}
411+
def _emit_decision(decision: HookDecision, *, logger=None) -> None:
412+
payload = decision.to_payload()
429413
raw = json.dumps(payload)
430414
if logger is not None:
431415
logger.set_stdout(raw, payload)
432416
print(raw)
433417

434418

419+
def _post_drift_notice(drift_reason: str) -> str:
420+
"""Rewrap a drift reason as a post-hoc notification to Claude.
421+
422+
The edit has already been applied; we can only inform Claude that
423+
the paired file is now out of sync because someone changed it
424+
behind our back.
425+
"""
426+
return (
427+
"Paired notebook drift detected after edit — the other side may "
428+
"have been modified by a human or another agent.\n\n"
429+
f"{drift_reason}\n\n"
430+
"Run `j-cli convert` to reconcile before further edits."
431+
)
432+
433+
435434
_MAX_DIFF_CHARS = 6000
436435

437436

@@ -469,15 +468,17 @@ def notebook_edit_guard(debug: bool) -> None:
469468
if tool_name != "NotebookEdit":
470469
sys.exit(0)
471470

472-
_print_decision(
473-
HookDecision.DENY,
474-
"NotebookEdit is disabled in this project — edit notebooks via the "
475-
"py:percent round-trip instead:\n"
476-
" 1. j-cli convert ipynb-to-py <nb.ipynb> <nb.py>\n"
477-
" 2. Edit <nb.py> with Edit/Write\n"
478-
" 3. j-cli convert py-to-ipynb <nb.py> <nb.ipynb>\n"
479-
"(The paired `.py` round-trip preserves outputs and keeps the pair "
480-
"in sync via `pair-drift-guard-pre`.)",
471+
_emit_decision(
472+
PreToolUseDecision(
473+
PreToolUseOutcome.DENY,
474+
"NotebookEdit is disabled in this project — edit notebooks via the "
475+
"py:percent round-trip instead:\n"
476+
" 1. j-cli convert ipynb-to-py <nb.ipynb> <nb.py>\n"
477+
" 2. Edit <nb.py> with Edit/Write\n"
478+
" 3. j-cli convert py-to-ipynb <nb.py> <nb.ipynb>\n"
479+
"(The paired `.py` round-trip preserves outputs and keeps the pair "
480+
"in sync via `pair-drift-guard-pre`.)",
481+
),
481482
logger=log,
482483
)
483484
sys.exit(0)
@@ -561,8 +562,7 @@ def _run_post_drift_check(path: Path, logger=None) -> None:
561562
if result.status == DriftStatus.CONFLICT:
562563
idx_str = ", ".join(str(i) for i in result.conflict_indices)
563564
other = ipynb_path if path == py_path else py_path
564-
_print_decision(
565-
HookDecision.DENY,
565+
drift_reason = (
566566
f"Your edit to `{path.name}` and an independent edit to `{other.name}` "
567567
f"both changed cell(s) [{idx_str}] — the changes collide and cannot be "
568568
"auto-merged. (The edit to `"
@@ -574,10 +574,9 @@ def _run_post_drift_check(path: Path, logger=None) -> None:
574574
" # take ipynb; discard .py edits on those cells\n"
575575
f" j-cli convert py-to-ipynb {py_path.name} {ipynb_path.name}"
576576
" # take .py; discard ipynb edits on those cells"
577-
+ _diff_section(result.diff_text, py_path.name),
578-
event=HookEvent.POST_TOOL_USE,
579-
logger=logger,
577+
+ _diff_section(result.diff_text, py_path.name)
580578
)
579+
_emit_decision(PostToolUseContext(_post_drift_notice(drift_reason)), logger=logger)
581580
return
582581

583582
if result.status == DriftStatus.DRIFT_ONLY:
@@ -589,17 +588,15 @@ def _run_post_drift_check(path: Path, logger=None) -> None:
589588
convert_hint = (
590589
f" j-cli convert ipynb-to-py {ipynb_path.name} {py_path.name}"
591590
)
592-
_print_decision(
593-
HookDecision.DENY,
591+
drift_reason = (
594592
f"Pair is drifted and `{py_path.name}` has no git baseline, so jcli "
595593
"can't auto-merge. Since you just edited "
596594
f"`{path.name}`, if that represents your current intent run:\n"
597595
f"{convert_hint}\n"
598596
"Be aware this overwrites the other file's independent content."
599-
+ _diff_section(result.diff_text, py_path.name),
600-
event=HookEvent.POST_TOOL_USE,
601-
logger=logger,
597+
+ _diff_section(result.diff_text, py_path.name)
602598
)
599+
_emit_decision(PostToolUseContext(_post_drift_notice(drift_reason)), logger=logger)
603600

604601

605602
def _sync_pair_after_edit(
@@ -648,11 +645,11 @@ def _sync_pair_after_edit(
648645

649646
if synced:
650647
other = ipynb_path if edited == py_path else py_path
651-
_print_decision(
652-
HookDecision.ALLOW,
653-
f"Auto-synced your edit in `{edited.name}` to `{other.name}`. "
654-
"Pair is now in sync.",
655-
event=HookEvent.POST_TOOL_USE,
648+
_emit_decision(
649+
PostToolUseContext(
650+
f"Auto-synced your edit in `{edited.name}` to `{other.name}`. "
651+
"Pair is now in sync."
652+
),
656653
logger=logger,
657654
)
658655

tests/test_hooks_cmd.py

Lines changed: 0 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -7,33 +7,6 @@
77
from click.testing import CliRunner
88

99
from jupyter_jcli.cli import main
10-
from jupyter_jcli.commands.hooks_cmd import HookDecision, HookEvent
11-
12-
13-
# ---------------------------------------------------------------------------
14-
# HookDecision / HookEvent enum behaviour
15-
# ---------------------------------------------------------------------------
16-
17-
class TestHookEnums:
18-
def test_hook_decision_members(self):
19-
assert HookDecision.DENY == "deny"
20-
assert HookDecision.ASK == "ask"
21-
assert HookDecision.ALLOW == "allow"
22-
assert isinstance(HookDecision.DENY, str)
23-
24-
def test_hook_event_members(self):
25-
assert HookEvent.PRE_TOOL_USE == "PreToolUse"
26-
assert HookEvent.POST_TOOL_USE == "PostToolUse"
27-
assert isinstance(HookEvent.PRE_TOOL_USE, str)
28-
assert isinstance(HookEvent.POST_TOOL_USE, str)
29-
30-
def test_invalid_decision_raises(self):
31-
with pytest.raises(ValueError):
32-
HookDecision("bogus")
33-
34-
def test_invalid_event_raises(self):
35-
with pytest.raises(ValueError):
36-
HookEvent("bogus")
3710

3811

3912
# ---------------------------------------------------------------------------

0 commit comments

Comments
 (0)