feat(formatters): emit data.changed so the per-session rewrote report fills - #3765
Merged
Conversation
… fills The per-session observability report's "Rewrote" block reads a changed boolean the sink copies from data.changed, and no producer sent it. The shared rewrite guard now records a byte verdict at the take (HOOK_REWRITE_CHANGED: true, false, or unknown when it could not snapshot), its six carriers add the key to their telemetry data, and markdown-format and typos-format derive it from their own rewrite signals (the fix-count line, applied corrections). biome, go, powershell and ruff took the disclosure after emitting on some arms; those arms now take first, with stdout unchanged. bash-format takes unconditionally before its emit so a run where shfmt never ran reports false rather than nothing. Every data schema gains the optional key; each suite pins it on a rewriting run and a no-op run; the observability fixture carries one changed row and the case that asserted the block empty now asserts the file is named. Nine patch bumps: bash 0.7.37, biome 0.6.35, eol 0.6.36, go 0.3.39, powershell 0.7.38, ruff 0.6.36, markdown 0.11.45, typos 0.6.43, claude-ops 0.42.7. Closes #3755 Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019DaWEB8Daq1xAXy2Xj1Pme
kyle-sexton
marked this pull request as ready for review
September 5, 2026 18:01
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4ec661124a
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
typos-format's clean arm reached typos and knows the answer, so it now sends changed: false instead of omitting the key; an all-clean session reads "nothing rewritten" rather than "no data". ruff-format's tool-break arm emitted telemetry before taking the disclosure, so a rewrite the fix or format pass had already written was not recorded; the take now precedes the emit as on every other arm. One suite case pins the clean verdict. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019DaWEB8Daq1xAXy2Xj1Pme
…a-changed-ji6rl5 # Conflicts: # plugins/claude-ops/CHANGELOG.md
…a-changed-ji6rl5 # Conflicts: # plugins/claude-ops/.claude-plugin/plugin.json # plugins/claude-ops/CHANGELOG.md
kyle-sexton
added a commit
that referenced
this pull request
Sep 5, 2026
…e at 1.1 (#3769) Closes #3758 Its two bases (#3765, #3762) have merged, `main` is merged in, and the diff is reduced to this change alone. ## Summary The reference sink files an envelope per session only when the envelope carries a session id, and only the nine claude-ops audit hooks sent one, so the per-session report's "hooks fired" table never listed the formatters or the guards. The library now puts the payload's correlation keys on the envelope spine for every producer, at contract 1.1. ## Fix - **`lib/hook-utils.sh`** (synced into its 17 carriers): `hook::emit_telemetry` reads the payload the producer buffered (`HOOK_TELEMETRY_PAYLOAD` when set, else `INPUT`, the variable every fleet hook assigns from `hook::buffer_stdin`) and copies `session_id`, `prompt_id`, `tool_use_id` and `agent_id` onto the envelope between `duration_ms` and `data`, each only when present as a plain id (`[A-Za-z0-9._-]+`), on both the builtin and the jq path. No jq, no subprocess, no producer change. `schema_version` reads `1.1`. - **`docs/conventions/hook-telemetry`**: `envelope.schema.json` gains the four optional properties; the README gains a "Correlation keys" section and rewrites the sink-routing note; the contract CHANGELOG records 1.1 as an additive minor. - **Sink** (`plugins/claude-ops/hooks/hook-telemetry-sink.sh` and the repo-local `.claude/hooks/` copy): routes on the spine `session_id` first and falls back to `data.session_id`, so envelopes from 1.0 producers keep their route. - **Seventeen carrier bumps** with CHANGELOG entries: actionlint 0.8.37, autonomy 0.22.28, bash-format 0.7.38, biome-format 0.6.36, claude-ops 0.42.11, context-guard 0.7.44, desktop-notification 0.6.31, eol-normalizer 0.6.37, go-format 0.3.40, guardrails 0.32.7, instruction-placement 0.11.28, markdown-format 0.11.46, powershell-format 0.7.39, rate-limit-guard 0.7.35, ruff-format 0.6.37, source-control 0.55.54, typos-format 0.6.44. ## The keys are selected by depth Review found that searching the raw payload takes the leftmost match anywhere in it. Two failures, both reproduced against the first version of this branch: - `{"session_id":"sess-real","tool_input":{"options":{"prompt_id":"NESTED-WRONG"}}}` emitted `"prompt_id":"NESTED-WRONG"`. - `{"tool_input":{"n":{"session_id":"NESTED-WRONG"}},"session_id":"sess-real"}` emitted `"session_id":"NESTED-WRONG"` — not a mis-join, the row lands in **another session's** `sessions/<id>.jsonl`. Cutting the search at the first nested container fixed those and broke something else: the documented payload places `tool_use_id` **after** `tool_input` (`session-event-log.sh` says so in its early-stop note, "tool_input closed, tool_use_id still to come"), so the cut dropped it on every tool event, and the fixture added alongside listed the four ids up front so it passed anyway. So the keys are selected by depth instead. Escapes are neutralized, the payload is split on the quote character, and the alternating fields are walked — even fields structure, odd fields string bodies. A string body is kept only at depth 1, so nested objects collapse to brace-and-colon rubble carrying no quotes and no nested key can match, while a root key after a container is still reached. The walk costs one step per string, not one per byte, and what it renders is short, so the four searches run over a small string whatever the payload size. ## Bounded, and the gap is filed The neutralizing passes are superlinear in escape count. Per emit, escape-bearing payload, this container: | payload | ungated walk | shipped | |---|---|---| | 16 KiB | 4 ms | 4 ms | | 64 KiB | 13 ms | 4 ms | | 128 KiB | 38 ms | 6 ms | | 512 KiB | **486 ms** | **22 ms** | The walk is gated at 65536 bytes; past it the payload takes the head cut. That stays safe at any size — nothing nested is reachable — but it is not complete: a root key after the first container is omitted above the gate, so `tool_use_id` is dropped on payloads over 64 KiB. `session_id` and `prompt_id` lead the payload, so routing is unaffected. **#3784** carries that gap with these measurements and two candidate approaches, and the code comment points at it. ## Verification - `lib/hook-utils.test.sh` **303/303**. The suite discriminates against both wrong versions: **3 failures against the un-anchored original** and **3 against the truncating fix**. New cases cover the documented key order (`tool_use_id` after `tool_input`), a nested decoy with no root key, a nested key ahead of the root one, decoys inside a root array, a multi-megabyte payload, and the size-gate boundary. - `hook-telemetry-sink.test.sh` 41/41, `api-error-audit.test.sh` 11/11, `run-guards.test.sh` 99/99, `claude-observability.test.sh` 60/60, `lib/rewrite-guard.test.sh` 22/22. - `scripts/affected-tests.sh --run`: one failure, not this PR's — `block-hook-bypass.test.sh`, "symlink: a genuine temp write in the same root stays allowed". Reproduced identically on unmodified `main` at `73eb4d98` in a clean worktree (PASS=601 FAIL=1). - `scripts/sync-hook-utils.sh --check`, `scripts/sync-rewrite-guard.sh --check`, `scripts/check-changelog-parity.sh --check-bump origin/main`, `--check`, `--check-preserved origin/main`, `scripts/validate-plugins.sh`, `scripts/check-shell-portability.sh origin/main`, `scripts/check-killswitch-hoist.sh`: all pass. shellcheck clean; the one shfmt hunk is pre-existing (`main` carries 49, this branch 1, and it is not in the changed region). ## Renumbered three times #3770 (fleet-wide prompt audit) bumped 64 plugins and took eight of the numbers this branch claimed; #3762 and #3765 then took guardrails 0.32.5 and claude-ops 0.42.10; #3781 then took guardrails 0.32.6. Every one of the seventeen was re-derived as the next patch above `origin/main` and verified against `git show origin/main:plugins/<p>/.claude-plugin/plugin.json` after each move. Each CHANGELOG conflict was resolved by keeping the released entry at its own heading and lifting this change's entry above it; `--check-preserved origin/main` passing is what proves no released entry was dropped. ## Related - Refs #930 (closed, the thread this finishes), #3750 (the nine-hook step and the sink route), #3765 and #3762 (the bases, now merged), #3770 / #3781 (took claimed numbers), #3784 (the size-gate gap), #3410, #3408. 🤖 Generated with [Claude Code](https://claude.com/claude-code) https://claude.ai/code/session_019DaWEB8Daq1xAXy2Xj1Pme --------- Co-authored-by: Claude <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
2 participants
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.
Closes #3755
Summary
The per-session observability report's "Rewrote" block reads a
changedboolean that the reference sink already copies fromdata.changed, and no producer sent it, so the block rendered the no-data line in every report. The eight rewriting formatters now send it.Fix
lib/rewrite-guard.sh(synced into its six carriers): the take records a byte verdict inHOOK_REWRITE_CHANGED:truewhen the file differs from the snapshot,falsewhen identical or when no rewrite was attempted (begin never ran), and empty when begin could not snapshot. The first take's verdict survives a later take, so a producer that emits after its take still reads it. Five new cases inlib/rewrite-guard.test.sh(22/22).bash-format,biome-format,eol-normalizer,go-format,powershell-format,ruff-format):build_data_jsonaddschangedfrom the verdict and omits the key when it is unknown, never guessing. Every arm that emitted telemetry before taking the disclosure (biome, go and ruff clean arms; go, powershell and ruff findings and tool-break arms) now takes first, with stdout unchanged.bash-formattakes unconditionally before its emit so a run where shfmt never ran (no.editorconfigopt-in) reportsfalserather than nothing.markdown-formatderives it from the same "Attempted: N fixes" count line that drives its user-channel disclosure;typos-formatfrom whetherappliedis non-empty, and its clean arm sendsfalse. Both omit the key on skip arms where the tool never ran.docs/conventions/hook-telemetry/data/*.schema.jsongain the optionalchangedboolean (additive rule).claude-ops: the observability context now renders a row perchanged == trueenvelope,_nothing rewritten_when rows carry the key and every value is false, and the no-data line only when no row carries it; the suite fixture carries onechanged: trueand onechanged: falserow, and the case that asserted the block empty now asserts the file is named (60/60).actionlintis named in the issue but does not rewrite files, so it is left alone.Fix (2): a description regression on
mainMerging
mainafter #3770 surfaced a defect that is not this change's. #3770 revertedplugins/claude-ops/.claude-plugin/plugin.json'sdescriptionto a pre-#3750 state, dropping 730 characters: the per-session hook event log, the setup action's log-root and self-ignoring-guard wording, the sink's per-session routing, andaudit-performance's Windows kernel-object census. Sentence-level comparison of the pre-#3770 and post-#3770 descriptions shows #3770 contributed no new wording to this field, so it is a clobber rather than an edit.The conflict resolution restores the full text and regenerates
docs/CATALOG.mdfrom it, since the catalog is generated from plugin descriptions and would otherwise failscripts/validate-plugins.sh.Verification
scripts/affected-tests.sh --run: all 14 selected suites passed or were skipped. Suites run with their tool on this host: bash-format 54/54, eol-normalizer 54/54, ruff-format 63/63, typos-format 148/148, markdown-format 174/174, rewrite-guard 22/22, claude-observability 60/60. Each pinschangedon a rewriting run and a no-op run.biome, nogoimports), and powershell-format's behavioral cases skip (no PSScriptAnalyzer). Their new cases assertfalseon the existing lint-finding fixtures (already-formatted input) andtrueon a new unformatted fixture for biome and go; powershell asserts the key is a boolean, since its formatter settings decide the value. Thetest-linuxlane is where those run.scripts/sync-rewrite-guard.sh --check,scripts/check-changelog-parity.sh --check-bump origin/main,--check,scripts/check-shell-portability.sh origin/main,scripts/validate-plugins.sh: all pass on the merged head. shellcheck, shfmt, markdownlint, typos clean on the changed files (the one shfmt note inmarkdown-format.test.shis a pre-existing line this PR does not touch).Renumbered 0.42.9 to 0.42.10
#3770 bumped 64 plugins and took claude-ops 0.42.9.
mainwas merged in, this change renumbered above it, and the CHANGELOG conflict resolved so #3770's released 0.42.9 entry stays intact with this change's entry above it. None of the eight formatter plugins were touched by #3770, so their numbers are unchanged.Related
changedroute), formatter hooks: nine siblings duplicate the PostToolUse prologue as structure the drift gate cannot see (deepening candidate, top recommendation) #3408 (the formatter prologue where this emission would otherwise be repeated; landing it in the shared rewrite guard is the same consolidation for the six carriers), hook-telemetry: envelope schema validates nothing while ten suites hand-transcribe the field list (deepening candidate) #3410 (the schema additions are hand-maintained until that lands), docs: apply the fleet-wide prompt-audit against Claude Fable 5.1 #3770 (took 0.42.9 and dropped the description restored here).🤖 Generated with Claude Code
https://claude.ai/code/session_019DaWEB8Daq1xAXy2Xj1Pme