Skip to content

Commit ce0b4fe

Browse files
CamSoperclaude
andauthored
Fix content-review lint re-gate false-fail and denial instrumentation (#19739)
Two bugs surfaced by the latest count=5 run: 1. Lint re-gate false-fails. The worker re-runs `make lint` on the fix branch in a workspace that still holds the pre-compute scratch artifacts (.content-review-queue.json, .candidate-claims*.json, .vale-raw.json, …). `make lint` runs `prettier --check .`, which flagged those untracked files and exited 1 — so every fix PR was wrongly flipped to draft and its docs review suppressed (markdown lint itself passed with 0 errors). Add the generated artifacts to .prettierignore. Also gitignore the new .content-review-verdict.json (and the re-gate's .lint.log/.lint-comment.md), and drop the obsolete .content-review-results.json entry. 2. Denial instrumentation read the wrong field. summarize-denials.py looked for `permission_denials_count` on the raw SDK result message, which doesn't carry it (the action derives the count from `resultMsg.permission_denials.length`). So it reported "0 matched" while the real counts were 9/12/6/17/1, and the cross-check never fired. Read the authoritative `permission_denials` array directly (tool_name + tool_input), with a recursive fallback and a values-free shape skeleton when none are found, so a clean run is never confused with a parse miss. Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 parent ed794d0 commit ce0b4fe

3 files changed

Lines changed: 94 additions & 74 deletions

File tree

.gitignore

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -108,11 +108,14 @@ _vendor/
108108
# review-existing-content workflow; the ledger lives in S3, staged through
109109
# .content-review-ledger.json and synced down into .ledger-cache/).
110110
/.content-review-queue.json
111-
/.content-review-results.json
111+
/.content-review-verdict.json
112112
/.content-review-ledger.json
113113
/.ledger-cache/
114114
/.traffic-snapshot
115115
/.synthetic.patch
116+
# Lint re-gate scratch (worker re-lints the fix branch and captures output).
117+
/.lint.log
118+
/.lint-comment.md
116119
# docs-review pre-computation artifacts the worker generates before the
117120
# review (claim extraction/verification, Vale, frontmatter, cross-sibling).
118121
/.fetched-urls.json

.prettierignore

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -61,3 +61,19 @@ static/js
6161
# Link-checker workflow artifacts (gitignored, generated at runtime)
6262
/.broken-links.json
6363
/.broken-links-pr.txt
64+
65+
# Content-review / docs-review pre-compute artifacts (gitignored, generated at
66+
# runtime). The content-review worker re-runs `make lint` on the fix branch in a
67+
# workspace that still holds these scratch files; without this, `prettier
68+
# --check .` flags them and the re-gate false-fails.
69+
/.synthetic.patch
70+
/.fetched-urls.json
71+
/.candidate-claims*.json
72+
/.verified-claims.json
73+
/.frontmatter-validation.json
74+
/.cross-sibling-discovery.json
75+
/.vale-raw.json
76+
/.vale-findings.json
77+
/.content-review-*.json
78+
/.lint.log
79+
/.lint-comment.md

scripts/content-review/summarize-denials.py

Lines changed: 74 additions & 73 deletions
Original file line numberDiff line numberDiff line change
@@ -2,16 +2,21 @@
22
"""Summarize permission-denied tool calls from a Claude Code execution log.
33
44
TEMPORARY DIAGNOSTIC. The content-review worker runs the review model under a
5-
tight `--allowed-tools` allowlist, and runs show a high `permission_denials_count`
6-
(12-21 per run) with no record of *which* commands were rejected — the GitHub log
5+
tight `--allowed-tools` allowlist, and runs show a high permission-denial count
6+
(6-21 per run) with no record of *which* commands were rejected — the GitHub log
77
streams only the init and result events. This reads the action's `execution_file`
8-
(the full stream-json transcript, which only exists on the runner) and prints the
9-
denied command strings so the allowlist can be sized from real data.
8+
(the full message array, which only exists on the runner) and prints the denied
9+
command strings so the allowlist can be sized from real data.
10+
11+
The authoritative source is the SDK result message's `permission_denials` array
12+
(claude-code-action derives its `permission_denials_count` from exactly this:
13+
`resultMsg.permission_denials?.length`). Each entry carries the denied call's
14+
`tool_name` and `tool_input`, so we read it directly rather than scraping
15+
tool_result text.
1016
1117
Safe for a public repo: it emits only the model's own *attempted* (and therefore
1218
un-executed) command strings — never tool results, which can carry command output
13-
or secrets. It cross-checks its tally against the result event's
14-
`permission_denials_count` and notes any mismatch so the parse can be trusted.
19+
or secrets.
1520
1621
Usage: python3 summarize-denials.py <execution_file>
1722
"""
@@ -24,22 +29,9 @@
2429
import os
2530
import sys
2631

27-
# Substrings that mark a tool-permission rejection in a tool_result. Kept broad
28-
# (matching is anchored on an `is_error` result) so a wording change upstream
29-
# doesn't silently drop denials.
30-
DENIAL_MARKERS = (
31-
"haven't granted",
32-
"hasn't been granted",
33-
"requested permissions",
34-
"permission to use",
35-
"not allowed to use",
36-
"permission denied",
37-
"permission to run",
38-
)
39-
40-
41-
def load_events(path: str) -> list[dict]:
42-
"""Return the transcript events, tolerating a JSON array or JSONL."""
32+
33+
def load_events(path: str) -> list:
34+
"""Return the message array, tolerating a JSON array, JSONL, or wrapper."""
4335
text = open(path, encoding="utf-8").read()
4436
try:
4537
data = json.loads(text)
@@ -52,30 +44,58 @@ def load_events(path: str) -> list[dict]:
5244
return []
5345

5446

55-
def content_blocks(event: dict) -> list:
56-
"""The content blocks of an event, whether nested under `message` or not."""
57-
msg = event.get("message", event)
58-
blocks = msg.get("content")
59-
return blocks if isinstance(blocks, list) else []
60-
47+
def find_denials(events: list) -> list:
48+
"""The permission_denials array, from the result message or anywhere it sits.
6149
62-
def block_text(content) -> str:
63-
"""Flatten a tool_result `content` (string or list of text blocks)."""
64-
if isinstance(content, str):
65-
return content
66-
if isinstance(content, list):
67-
return " ".join(
68-
b.get("text", "") for b in content if isinstance(b, dict)
69-
)
70-
return ""
50+
Primary: the `type == "result"` message's `permission_denials`. Fallback: a
51+
recursive search, so a wrapper/shape change still surfaces the data.
52+
"""
53+
for ev in events:
54+
if isinstance(ev, dict) and ev.get("type") == "result":
55+
pd = ev.get("permission_denials")
56+
if isinstance(pd, list):
57+
return pd
58+
59+
found: list = []
60+
61+
def walk(node):
62+
if isinstance(node, dict):
63+
for k, v in node.items():
64+
if k == "permission_denials" and isinstance(v, list):
65+
found.extend(v)
66+
else:
67+
walk(v)
68+
elif isinstance(node, list):
69+
for item in node:
70+
walk(item)
71+
72+
walk(events)
73+
return found
74+
75+
76+
def describe(denial: dict) -> str:
77+
"""A short label for a denied call: tool name + the Bash command if present."""
78+
name = denial.get("tool_name") or denial.get("name") or "?"
79+
inp = denial.get("tool_input") or denial.get("input") or {}
80+
if isinstance(inp, dict):
81+
detail = inp.get("command") or inp.get("file_path") or inp.get("path")
82+
if not detail and inp:
83+
detail = json.dumps(inp, sort_keys=True)[:200]
84+
else:
85+
detail = str(inp)[:200]
86+
return f"{name}: {detail}" if detail else name
7187

7288

73-
def describe(tool_use: dict) -> str:
74-
"""A short label for a tool_use: the Bash command, else the tool name."""
75-
name = tool_use.get("name", "?")
76-
inp = tool_use.get("input") or {}
77-
detail = inp.get("command") or inp.get("file_path") or inp.get("path")
78-
return f"{name}: {detail}" if detail else name
89+
def event_skeleton(events: list) -> str:
90+
"""A values-free shape summary, to refine the parser if denials don't surface."""
91+
type_counts: collections.Counter[str] = collections.Counter()
92+
keys: set[str] = set()
93+
for ev in events:
94+
if isinstance(ev, dict):
95+
type_counts[str(ev.get("type", "<no type>"))] += 1
96+
keys.update(ev.keys())
97+
types = ", ".join(f"{t}×{n}" for t, n in type_counts.most_common())
98+
return f"{len(events)} messages; types: {types or 'none'}; top-level keys: {sorted(keys)}"
7999

80100

81101
def main() -> int:
@@ -92,39 +112,20 @@ def main() -> int:
92112
print(f"summarize-denials: could not parse execution log ({e})")
93113
return 0
94114

95-
# Map each tool_use id to its label, then attribute denial results back to it.
96-
labels: dict[str, str] = {}
97-
reported_count = None
98-
for ev in events:
99-
for b in content_blocks(ev):
100-
if b.get("type") == "tool_use":
101-
labels[b.get("id")] = describe(b)
102-
if ev.get("type") == "result":
103-
reported_count = ev.get("permission_denials_count", reported_count)
115+
denials = find_denials(events)
116+
tally: collections.Counter[str] = collections.Counter()
117+
for d in denials:
118+
tally[describe(d) if isinstance(d, dict) else str(d)[:200]] += 1
104119

105-
denied: collections.Counter[str] = collections.Counter()
106-
for ev in events:
107-
for b in content_blocks(ev):
108-
if b.get("type") != "tool_result" or not b.get("is_error"):
109-
continue
110-
text = block_text(b.get("content")).lower()
111-
if any(m in text for m in DENIAL_MARKERS):
112-
denied[labels.get(b.get("tool_use_id"), "<unknown tool>")] += 1
113-
114-
total = sum(denied.values())
115-
lines = [f"## Tool permission denials ({total} matched)", ""]
116-
if denied:
117-
for label, n in denied.most_common():
120+
lines = [f"## Tool permission denials ({len(denials)})", ""]
121+
if tally:
122+
for label, n in tally.most_common():
118123
lines.append(f"- ({n}×) `{label}`")
119124
else:
120-
lines.append("_No denial markers found in the transcript._")
121-
if reported_count is not None and reported_count != total:
122-
lines += [
123-
"",
124-
f"> Note: result reports {reported_count} denials but {total} were "
125-
"matched here — the denial wording may have changed; widen "
126-
"`DENIAL_MARKERS`.",
127-
]
125+
# No denials surfaced — could be a genuinely clean run, or a shape change.
126+
# The skeleton (no values) lets us tell which without another blind guess.
127+
lines.append("_No permission_denials found._")
128+
lines += ["", f"<sub>shape: {event_skeleton(events)}</sub>"]
128129
report = "\n".join(lines)
129130
print(report)
130131

0 commit comments

Comments
 (0)