Repository navigation
fix(mr-hook): no skill hint on background-task notification turns - #71
Merged
Merged
Conversation
Claude Code delivers background-task notifications (a finished background command or subagent, a Monitor event) through UserPromptSubmit, the same event as a typed prompt. The hook ranked them like requests. Measured on one long session: the skill hint fired 25 times on notifications and 7 times on the operator's own prompts, and every hint stays in context for the rest of the session. Shape, measured on real transcripts: every notification prompt starts with <task-notification>, including all 1,710 queued ones (queued_command, commandMode "task-notification"). The hook receives the queued text verbatim: the usage log's prompt_hash equals the sha256 of the transcript's queued prompt. cmd/mr-hook/machineturn.go adds isMachineTurn: the prompt starts, after leading whitespace, with <task-notification>. On such a turn main.go skips retrieval, so nothing reaches the embedder, and the row logs mode "machine-turn". The index load, the identity check and the quota banner run exactly as before, so the GLM hard-stop latch still shows on these turns. The offload nudge is left alone: 0 of 3,274 unique real notifications can trigger it. Human prompts are unchanged. Consumers of the new mode: mr-outcomes skips rows with no surfaced ids, and a reader of cands gets none, which is the same as a gated-empty row. Tests, red first against an inert stub, then green: - TestIsMachineTurnShapes: three real layouts, leading whitespace, a human prompt that quotes a notification, a truncated tag, empty. - TestHintSilentOnTaskNotificationE2E and TestHintSilentOnQueuedNotificationE2E run the built binary against an embedder that scores every prompt 1.0, so the silence can only come from the guard. They assert no output, zero embedder calls, and mode machine-turn. - TestHintStillShownOnHumanPromptE2E is the control arm. - TestGLMLatchBannerOnNotificationTurnE2E runs in an established session, so the first-prompt rule cannot be what shows the banner. It asserts the banner is shown, the hint is absent, and the state dir is byte-identical afterwards. Mutation-proven with a harness that requires each mutant to compile first: 10 of 10 compiling mutants are caught, and a non-compiling control is rejected rather than counted. The caught mutants include: guard off at the call site; retrieval still running; mislabelled row; no whitespace trim; contains instead of prefix; widened tag; banner suppressed on machine turns; a state write on machine turns. B12: +28 non-test LOC, budget raised to 30952 with the prior note kept. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
mr-hookno longer injects the skill hint on background-task notification turns. Human prompts are unchanged, and so is the quota banner.Why
Claude Code delivers background-task notifications (a finished background command or subagent, a Monitor event) through
UserPromptSubmit, the same event as a typed prompt. The hook ranked them as if they were requests. Measured on one long session: the hint fired 25 times on notifications and 7 times on the operator's own prompts. Every hint stays in context and is re-read on every later call.The shape, measured on real transcripts
<task-notification>. That holds for all 1,710 queued ones (queued_command,commandMode: "task-notification") as well as direct user turns. Three tag layouts occur: background command, finished agent (with<note>/<result>), and Monitor event (<event>, with no tool-use id).prompt_hashequals the sha256 of the transcript's queued prompt, and the lengths match.<cross-session-message …>and<agent-message …>. Both are queued withcommandMode: "prompt", there were 10 in total, and they carry actual requests.Change
cmd/mr-hook/machineturn.go:isMachineTurnreturns true when the prompt starts, after leading whitespace, with<task-notification>.cmd/mr-hook/main.go: on a machine turn, retrieval is skipped, so no embedder call is made and the row logsmode:"machine-turn". The index load, the identity check and the quota banner (including the delegate proposal) run exactly as before. This keeps the PR feat: render the quota banner only when it carries news (semantic ChangedAt) #69 semantics: the banner shows on a changed_at within ±15m, a reset within 15m, the first prompt of a session, or the GLM latch.mr-outcomesskips rows with no surfaced ids, and a reader ofcandsgets none, the same as on a gated-empty row.usagelog.Record.Tests (red first, then green)
Before implementation, with an inert stub, the notification tests got the
gstack-qahint and failed. The human control passed, and the GLM test passed its banner assertion and failed only on the leaked hint.TestIsMachineTurnShapesTestHintSilentOnTaskNotificationE2Emode:"machine-turn"TestHintSilentOnQueuedNotificationE2ETestHintStillShownOnHumanPromptE2Emode:"embed"TestGLMLatchBannerOnNotificationTurnE2EMutation proof
The harness requires each mutant to compile (
go test -run '^$') before a failing run counts, and restores the file with a hash check after each mutant.isMachineTurnalways false<task-Gates
go test -count=1 ./...: all 53 packages ok.go vet ./...: clean.semgrep scan --config auto --error <changed files>: exit 0, 0 findings. The new test's launcher of its own freshly built binary carries a targetednosemgrepwith its reason. The existing E2E launchers in untouched test files still trip the same rule 5 times; they are out of scope here.machineturn.go, 6 inmain.go), 30924 → 30952, with the prior note kept as PRIOR.Deploy (after merge, not done here)
From a clean checkout of merged
main:bash scripts/deploy-fleet.sh. It printsbuilt mr-hook @ <rev>for each binary and ends withfleet is uniform at revision <rev>.Risks
<task-notification>gets no hint. That is unlikely, and the cost is one missing hint.🤖 Generated with Claude Code