Skip to content

Commit 3817633

Browse files
tumbergerclaude
andcommitted
fix(cowork): report ground-truth health; don't claim unverifiable hooks work
Claude Code's settings watcher only watches dirs that already had a settings file when the session started. Cowork does not pre-create one, so writing the first settings.json into a session whose CLI is already running never takes effect — yet health marked it hooked. That is the silent miss relocated into the health log. Split the signal: - written — wrote (or found current) our hook AND have reason to believe it loads: a new session we seeded before its CLI started, or a re-merge of a hook already present (that dir was watched at session start, so the change hot-reloads). - unverified — wrote a first-time hook onto a possibly-already-running session (the startup pass). The watcher may never load it, so it is best-effort and must not read as working. The heartbeat warns about these explicitly. - confirmed — a spool arrived: ground truth the hook fires. A spool promotes a session from unverified to written. reinjectExisting now trusts only sessions that already carried our hook (the safe mode-switch re-merge); a first-time hook on a pre-existing session is written best-effort but recorded as unverified. Steady-state inject still trusts its writes (it wins the race before the CLI starts). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
1 parent 14267a1 commit 3817633

2 files changed

Lines changed: 135 additions & 25 deletions

File tree

internal/coworkobserve/coworkobserve.go

Lines changed: 89 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -327,9 +327,21 @@ const heartbeatInterval = 5 * time.Minute
327327
// The heartbeat makes "no Cowork activity" distinguishable from "observation
328328
// broken" in the daemon diagnostics.
329329
type health struct {
330-
sessionsSeen map[string]bool // recent .claude dirs discovered by the injector
331-
hooked map[string]bool // .claude dirs carrying our hook
332-
spooled map[string]bool // session dirs that produced a spool file
330+
sessionsSeen map[string]bool // .claude dirs discovered by the injector
331+
// written is .claude dirs where we wrote (or found current) our hook AND
332+
// have reason to believe it takes effect — the dir was watched at session
333+
// start (a new session we seeded before its CLI started, or a re-merge of a
334+
// hook that was already there). It is not, on its own, proof the hook fires.
335+
written map[string]bool
336+
// unverified is .claude dirs where we wrote a first-time hook onto a session
337+
// whose CLI may already be running. Claude Code's settings watcher only
338+
// watches dirs that had a settings file when the session started, so such a
339+
// write may never load. These are best-effort and must not be reported as
340+
// working until a spool confirms otherwise.
341+
unverified map[string]bool
342+
// spooled is session dirs that produced a spool — ground truth that the hook
343+
// actually fired. A spool promotes a session from unverified to written.
344+
spooled map[string]bool
333345
eventsReplayed int64
334346
linesDropped int64
335347
denied int64
@@ -338,24 +350,31 @@ type health struct {
338350
func newHealth() *health {
339351
return &health{
340352
sessionsSeen: map[string]bool{},
341-
hooked: map[string]bool{},
353+
written: map[string]bool{},
354+
unverified: map[string]bool{},
342355
spooled: map[string]bool{},
343356
}
344357
}
345358

346359
func (h *health) logHeartbeat(opts Options) {
347360
opts.Diagnostic.Printf(
348-
"cowork observe: health: sessions seen=%d hooked=%d spooling=%d events replayed=%d denied=%d malformed dropped=%d\n",
349-
len(h.sessionsSeen), len(h.hooked), len(h.spooled), h.eventsReplayed, h.denied, h.linesDropped,
361+
"cowork observe: health: sessions seen=%d written=%d confirmed=%d unverified=%d events replayed=%d denied=%d malformed dropped=%d\n",
362+
len(h.sessionsSeen), len(h.written), len(h.spooled), len(h.unverified), h.eventsReplayed, h.denied, h.linesDropped,
350363
)
351-
if len(h.sessionsSeen) > len(h.hooked) {
364+
if len(h.unverified) > 0 {
365+
opts.Diagnostic.Printf(
366+
"cowork observe: warning: %d pre-existing session(s) had a hook written but unconfirmed — their CLI likely started before the hook existed, so it will not fire until the session restarts\n",
367+
len(h.unverified),
368+
)
369+
}
370+
if seen := len(h.sessionsSeen) - len(h.written) - len(h.unverified); seen > 0 {
352371
opts.Diagnostic.Printf(
353372
"cowork observe: warning: %d session(s) never received the hook (injection raced CLI startup, or the daemon started after the session)\n",
354-
len(h.sessionsSeen)-len(h.hooked),
373+
seen,
355374
)
356375
}
357-
if len(h.hooked) > 0 && len(h.spooled) == 0 {
358-
opts.Diagnostic.Printf("cowork observe: warning: hook injected but no spool has appeared; the Cowork session layout or mount may have changed\n")
376+
if len(h.written) > 0 && len(h.spooled) == 0 {
377+
opts.Diagnostic.Printf("cowork observe: warning: hook written but no spool has appeared; the Cowork session layout or mount may have changed\n")
359378
}
360379
}
361380

@@ -374,7 +393,9 @@ func inject(opts Options, h *health) {
374393
if err != nil || info.ModTime().Before(cutoff) {
375394
continue
376395
}
377-
mergeInto(opts, h, dir, entry)
396+
// trustFresh: a brand-new session's CLI has not started yet (we win the
397+
// race against it), so even a first-time hook will be loaded.
398+
mergeInto(opts, h, dir, entry, true)
378399
}
379400
}
380401

@@ -402,32 +423,72 @@ func reinjectExisting(opts Options, h *health) {
402423
if err != nil || info.ModTime().Before(cutoff) {
403424
continue // abandoned session dir
404425
}
405-
mergeInto(opts, h, dir, entry)
426+
// trustFresh is false: this session's CLI may already be running. If we
427+
// are writing the first settings file into its dir, Claude Code's
428+
// watcher never watched that dir (it only watches dirs that had a
429+
// settings file at session start), so the write will not take effect —
430+
// record it as unverified rather than working.
431+
mergeInto(opts, h, dir, entry, false)
406432
}
407433
}
408434

409435
// mergeInto records the session for the heartbeat, then writes entry into its
410-
// settings.json when it is missing or a stale-mode variant is present. The
411-
// session is recorded as seen before the write so a hook that fails to land
412-
// shows up as seen-but-not-hooked in the heartbeat.
413-
func mergeInto(opts Options, h *health, dir string, entry map[string]any) {
436+
// settings.json when it is missing or a stale-mode variant is present.
437+
//
438+
// trustFresh says whether a first-time hook write (no hook of ours was already
439+
// present) can be trusted to take effect. It is true for steady-state inject —
440+
// a new session's CLI has not started, so it will load our settings.json — and
441+
// false for the startup pass, where the session may already be running over a
442+
// dir the settings watcher never watched. A write that updates a hook of ours
443+
// already present is always trusted: that dir was watched at session start, so
444+
// the change hot-reloads. Anything written but not trusted is recorded as
445+
// unverified, and only a spool (see collect) confirms it actually fires.
446+
func mergeInto(opts Options, h *health, dir string, entry map[string]any, trustFresh bool) {
414447
h.sessionsSeen[dir] = true
415448
settingsPath := filepath.Join(dir, "settings.json")
416449
existing, err := os.ReadFile(settingsPath)
417450
if err != nil && !os.IsNotExist(err) {
418451
return
419452
}
453+
hadOurHook := hasOurHook(existing)
420454
merged, needed := mergeSettings(existing, entry)
421455
if !needed {
422-
h.hooked[dir] = true
423-
return // already ours
456+
h.written[dir] = true
457+
return // already current
424458
}
425459
if err := writeFileAtomic(settingsPath, merged, 0o644); err != nil {
426460
opts.Diagnostic.Printf("cowork observe: inject %s: %v\n", settingsPath, err)
427461
return
428462
}
429-
h.hooked[dir] = true
430-
opts.Diagnostic.Printf("cowork observe: injected hook into %s\n", settingsPath)
463+
if hadOurHook || trustFresh {
464+
h.written[dir] = true
465+
opts.Diagnostic.Printf("cowork observe: injected hook into %s\n", settingsPath)
466+
return
467+
}
468+
h.unverified[dir] = true
469+
opts.Diagnostic.Printf("cowork observe: wrote hook into pre-existing session %s (unverified; will not fire until the session restarts unless its dir was already watched)\n", settingsPath)
470+
}
471+
472+
// hasOurHook reports whether settings.json already carries a PreToolUse hook the
473+
// injector installed (i.e. one whose command references the spool). Used to tell
474+
// a mode-switch re-merge (the dir was watched at session start, so the change
475+
// hot-reloads) apart from a first-time hook on an already-running session.
476+
func hasOurHook(existing []byte) bool {
477+
if len(bytes.TrimSpace(existing)) == 0 {
478+
return false
479+
}
480+
var settings map[string]any
481+
if json.Unmarshal(existing, &settings) != nil {
482+
return false
483+
}
484+
hooks, _ := settings["hooks"].(map[string]any)
485+
pre, _ := hooks["PreToolUse"].([]any)
486+
for _, candidate := range pre {
487+
if entryIsOurs(candidate) {
488+
return true
489+
}
490+
}
491+
return false
431492
}
432493

433494
type collector struct {
@@ -495,7 +556,14 @@ func (c *collector) collect(opts Options, h *health) {
495556
live := make(map[string]bool, len(spools))
496557
for _, spool := range spools {
497558
live[spool] = true
498-
h.spooled[filepath.Dir(spool)] = true
559+
sessionDir := filepath.Dir(spool)
560+
h.spooled[sessionDir] = true
561+
// A spool is ground truth the hook fired: promote the session out of
562+
// unverified into written, since the watcher clearly did load it.
563+
if claudeDir := filepath.Join(sessionDir, ".claude"); h.unverified[claudeDir] {
564+
delete(h.unverified, claudeDir)
565+
h.written[claudeDir] = true
566+
}
499567
c.drain(opts, h, spool)
500568
c.cleanup(opts, spool)
501569
cleanupOrphanDecisions(opts, filepath.Dir(spool))

internal/coworkobserve/coworkobserve_test.go

Lines changed: 46 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -190,8 +190,8 @@ func TestInjectAndHealthTracking(t *testing.T) {
190190

191191
h := newHealth()
192192
inject(opts, h)
193-
if len(h.sessionsSeen) != 1 || len(h.hooked) != 1 {
194-
t.Fatalf("seen=%d hooked=%d, want 1/1", len(h.sessionsSeen), len(h.hooked))
193+
if len(h.sessionsSeen) != 1 || len(h.written) != 1 {
194+
t.Fatalf("seen=%d written=%d, want 1/1", len(h.sessionsSeen), len(h.written))
195195
}
196196
settings, err := os.ReadFile(filepath.Join(claudeDir, "settings.json"))
197197
if err != nil {
@@ -528,8 +528,8 @@ func TestReinjectExistingRemergesRunningSessionOnRestart(t *testing.T) {
528528
// it so the heartbeat can warn if hooking ever fails.
529529
h := newHealth()
530530
reinjectExisting(opts, h)
531-
if len(h.sessionsSeen) != 1 || len(h.hooked) != 1 {
532-
t.Fatalf("startup pass did not record the running session: seen=%d hooked=%d", len(h.sessionsSeen), len(h.hooked))
531+
if len(h.sessionsSeen) != 1 || len(h.written) != 1 {
532+
t.Fatalf("startup pass did not record the running session: seen=%d written=%d", len(h.sessionsSeen), len(h.written))
533533
}
534534
settings, err := os.ReadFile(filepath.Join(claudeDir, "settings.json"))
535535
if err != nil {
@@ -564,6 +564,48 @@ func TestReinjectExistingSkipsAbandonedSessions(t *testing.T) {
564564
}
565565
}
566566

567+
// When the startup pass writes a first-time hook onto a session that may
568+
// already be running (no hook of ours was there), it cannot know the CLI's
569+
// settings watcher ever watched the dir, so it must record the session as
570+
// unverified, not working. A later spool is ground truth and promotes it.
571+
func TestReinjectExistingFlagsFreshHookUnverifiedUntilSpooled(t *testing.T) {
572+
_, socketPath := startFakeDaemon(t)
573+
opts := testOptions(t, socketPath)
574+
sessionDir := filepath.Join(opts.SessionsRoot, "acct", "ws", "local_pre")
575+
claudeDir := filepath.Join(sessionDir, ".claude")
576+
if err := os.MkdirAll(claudeDir, 0o755); err != nil {
577+
t.Fatal(err)
578+
}
579+
// Recent (within the window) but with no hook of ours: the daemon was down
580+
// when this session's CLI started, so we are creating its first settings
581+
// file only now.
582+
stale := time.Now().Add(-30 * time.Minute)
583+
if err := os.Chtimes(claudeDir, stale, stale); err != nil {
584+
t.Fatal(err)
585+
}
586+
587+
h := newHealth()
588+
reinjectExisting(opts, h)
589+
if len(h.written) != 0 {
590+
t.Fatalf("first-time hook on a pre-existing session was trusted: written=%d", len(h.written))
591+
}
592+
if len(h.unverified) != 1 {
593+
t.Fatalf("first-time hook not flagged unverified: unverified=%d", len(h.unverified))
594+
}
595+
// Still written best-effort, in case the dir happens to be watched.
596+
if _, err := os.Stat(filepath.Join(claudeDir, "settings.json")); err != nil {
597+
t.Fatalf("best-effort settings.json not written: %v", err)
598+
}
599+
600+
// A spool confirms the hook actually fires: promote to written, clear unverified.
601+
writeSpool(t, filepath.Join(sessionDir, spoolName), eventLine("Bash")+"\n")
602+
c := &collector{offsets: map[string]int64{}}
603+
c.collect(opts, h)
604+
if len(h.unverified) != 0 || !h.written[claudeDir] {
605+
t.Fatalf("spool did not confirm the session: unverified=%d written=%v", len(h.unverified), h.written[claudeDir])
606+
}
607+
}
608+
567609
// cleanup must not unlink a drained, idle spool if a hook appends a fresh event
568610
// in the window between the drained check and the remove — the re-stat guard
569611
// has to catch the change and leave the file for the next tick to drain.

0 commit comments

Comments
 (0)