merge queue: checking #796 on main (5184fd8) - #838
Closed
mergify[bot] wants to merge 9 commits into
Closed
mergify[bot] wants to merge 9 commits into
mergify[bot] wants to merge 9 commits into
Conversation
…o the turn Two independent lockouts kept this hook from speaking on Claude Code. One: `already_prompted` was keyed on the transcript path, so the Stop of the reply BEFORE a correction spent the session's single shot. The correction itself then hit an already-prompted key. The key is now the transcript path plus a hash of the reply text, so each distinct reply gets its own chance and the same reply is still judged only once. Two: `user_already_asked_reflect` scanned the whole transcript. One `/reflect` typed at the start of a session switched the detector off for every later reply. The scan is now scoped to the user messages of the turn that produced the reply being judged, which is the case the skip was written for. An unreadable transcript now says so on stderr instead of silently reading as "the user did not ask". The harness-parity test asserts the same reply is judged under both `Stop` and `stop`. It passes before this change too: this hook compares no event name anywhere, so event-name case cannot be what split its hits by harness. The test pins that, rather than leaving the parity unstated. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Change-Id: Idb90d55fa90496fcab44e1f0255b303b5cf0b514 Three: the suppression matched any prose containing the word. A sentence ABOUT the hook switched the hook off. `"Claim I made was wrong" is a trigger for /reflect` describes when the detector fires; it asks for nothing, and nothing is in flight to avoid duplicating. It now matches the harness's own record of an invocation, the `<command-name>/reflect</command-name>` envelope. That envelope starts with `<command-`, which the meta filter drops, so the one unambiguous signal was being discarded while vague prose was kept; the window now includes meta rows for this check. Four: a missing assistant row no longer reads as "no request found". A Stop payload can carry the reply before its row lands, and a session's first turn has none at all; in both cases every row belongs to this turn, and bailing out meant a real request went unhonoured. tests/scenarios/self-correction.json: the scenario pinning this said the user "already asked for /reflect" while its user field only mentioned the word -- its stated intent and its data disagreed. The request case now uses a real invocation; the mention case is its own scenario and must still fire. Both were folded into this commit rather than a later one so no commit in the stack leaves the scenario suite red. Erring toward asking is the safe direction: this detector recorded 1,682 Claude Code invocations and spoke 0 times. Ran 27 tests in 18.811s OK (wrong-check-reflect) Ran 6 tests in 0.098s OK (scenario suite) Change-Id: I60c5103d0541efd34f49e2401d50f4c5e312ddf0
… not the last assistant row The same-turn /reflect skip read the wrong turn. It took the last text-bearing assistant row as the reply under judgment and scanned only back to the assistant row before it. A Stop payload carries the reply before its transcript row is written, so that "last assistant row" was usually the previous turn's reply. The window then sat one turn behind: a /reflect from the turn before suppressed this reply (the session lockout, one turn wide), and the /reflect the person had just typed sat past the window and was ignored, so the hook nagged for a reflect already in flight. A turn also writes more than one assistant row -- mid-turn narration, a subagent's sidechain rows -- and each one pushed the window's start past the message that opened the turn. The window now runs from the person's own last message to the end of the file, so it does not depend on assistant rows at all. A typed slash command counts as that message, so <command-name> leaves the harness prefix list; <local-command stdout, which is harness echo, joins it. Four tests, each failing before this change: reflect this turn before the reply row lands, last turn's reflect while the reply is still being written, mid-turn narration, and a subagent's rows. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…e README scripts/ci/check_no_new_comments.py fails the test job on the three comment lines this branch added above META_USER_PREFIXES. The point they made -- a typed slash command is the person, not harness text -- now sits in the hook's README next to the harness-rows paragraph it belongs with. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…reads like English The PR body validator failed this branch on "self-correction" in the Summary, because tests/scenarios/self-correction.json is a changed file. The suite covered changed folder names and plain hyphenated English separately, but not the overlap, which is what a person hits: an everyday phrase that happens to be a file's name. Both directions are now tested -- it fails, and the reworded Summary passes with the same changed files. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…alidate) Exit code: 0
Claude Code files every tool result as a `type: "user"` row, and unlike its other injections it marks that row with neither `isMeta` nor `isSidechain`. The turn window anchors on the newest real user row, so the first tool call of a turn became the anchor: a `/reflect` typed at the top of the turn fell outside the window and the hook nagged for a reflect already asked for. A tool result quoting the word the other way round could also suppress the nudge. `_is_tool_result_line` reads the record the harness does write -- a `tool_result` content block, or the `toolUseResult` field beside the message -- the same way `engine/hooks/agent-relay-attribution/detect.py` does, and `_is_meta_line` now files those rows under the harness rather than the person. Two tests: a tool result mid-turn no longer hides the turn's own `/reflect`, and `/reflect` printed inside a grep result does not count as a request. Both fail without the detect.py change. Co-Authored-By: Claude Opus 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.
🎉 This pull request has been checked successfully and will be merged soon. 🎉
#796 is queued for merge on branch main (5184fd8).
This pull request has been created by Mergify to check the mergeability of #796.
You don't need to do anything. Mergify will close this pull request automatically when it is complete.
Required conditions of queue rule
admin-bypassfor merge:check-success = lintcheck-success = testcheck-success = validateRequired conditions to stay in the queue:
-draftbase=mainlabel=admin-bypass