feat(telemetry): record the retry prompt and tool arguments in Level 3 content

[dhshah13]

Third and last PR of the ADR 0050 Level 3 series, after #6429 (gate, collector, budget) and #6603 (tool results, execute_tool spans, ADR 0108). It delivers the two items #6603's description named as next: the runner-composed retry prompt as gen_ai.input.messages, and tool-call arguments on the message record. The carrier is unchanged — the record on the agent span, as ADR 0108 decided; execute_tool spans stay metadata only. Still one env var, no new configuration.

What this does

Part	Change
Input (internal/cli)	On a retry iteration under validation_loop.feedback_mode: append, the prompt buildFeedbackPrompt composed is recorded on that iteration's agent span as gen_ai.input.messages: one user message, one text part (semconv v1.37.0 input-messages schema). It is set right after the span starts, so the cancel, error and normal finalize paths carry it. The first iteration and retries without feedback send the runtime's fixed default prompt and record nothing.
Parser (internal/runtime)	ToolUseEvent gains Arguments: the call's input as the stream carried it, from both Claude Code paths (assistant line; stream_event deltas). Unchecked and unredacted at this layer, never rendered. Input assembled from deltas is cut at the parser's 1 MiB maxToolInputSize. pi and codex leave it empty (#7414); so does OpenCode, a stub runtime.
Collector (internal/cli)	tool_call parts carry arguments: the members of the call's input that name what was called — paths, patterns, commands, modes and bounds, the list in recordedArguments (file_path, pattern, path, command, description, url, the notebook, Grep and bound members) — each decoded and redacted as text on its own, then encoded again, and nothing else of the input: a file body, an edit, a prompt, a notebook source, a todo list, and any member whose value is an object or an array is dropped, scanned first so a secret in it still counts, charged as the redacted text it was scanned as, and the part marked fullsend.truncated. The guarantee is one sentence: the record holds these members of a call, each redacted as text, and nothing else of its input. A string the normalizer stripped an escape sequence or tag characters from is masked whole (stripping can take a token's first letter). No fail-closed mask is left: a dropped member raises no finding of its own, keeps the summary, and adds nothing to fullsend.content.redactions. Done once, when the event is handled. A call without a name — or whose name redacts to nothing — carries none; arguments lost to a name that redacted away are charged and marked like the dropped ones below.
Bounds	New maxToolArgumentsBytes = 8 KiB, measured on the redacted encoding of the kept members. Over it — or when the text is not one JSON object — the arguments are dropped whole (a cut object is not JSON): the part keeps id, name and summary, is marked fullsend.truncated, and the bytes are charged to fullsend.content.dropped_bytes. The walk is one pass over the top-level members, so no raw bound is needed ahead of it. The input message's encoded size is charged against the existing 255,000-byte ceiling, so the two content attributes of one span stay within the proven size together.
Redaction	On both sides of the pattern pipeline, the collector replaces the value of each sensitive runner env key, and of each provider-only key the runner keeps out of RunnerEnv (GH_WORKFLOW_TOKEN, #6649), 8 bytes or longer, with [REDACTED:<key>] — replaceEnvSecrets, one longest-first pass over both sources, shared with redactFeedback. It applies to all Level 3 content, the text and tool results #6429 and #6603 shipped included, and to execute_tool span names and ids. A call loses its summary when redaction found a secret in its arguments; a number leaf in arguments is scanned as its digits.
Docs	Reference, dev guide, runtime matrix, two user guides, the eval-measurements Level 3 caveat; dated annotations on ADR 0050 and ADR 0108 (no Decision text changed).

Why arguments are not scanned as serialized text

The redactor's patterns are written for plain text. A table test run first against the simple approach (scan the JSON text, keep it if still valid) failed 4 of the 8 shapes it started with (the first eight rows of TestContentCollector_RedactsSecretsInArguments; the table has since grown): an assignment that opens a string ({"command":"DEPLOY_TOKEN=… make"} — the pattern anchors on line start or whitespace, and sees a quote), an assignment after an escaped newline, a value behind escaped quotes, and JSON nested in a string. Unicode folding over the text also turned fullwidth quotation marks into ones that closed the string and added a member. Decoding first gives the redactor the text it was written for; the per-member scan keeps what the serialized form did catch ("password": "…"), where the leaf alone has no context. That scan runs on every string, number and key under a secret-named member, at any depth, not only clean ones — a zero-width character, an accent or a second token in the value would otherwise switch it off — it goes to the pattern stage alone (both strings are already normalized, and a second normalizer pass over the pair can strip an escape sequence the first left open, and the value with it), and its probe text puts a comma where a double quote was: the quote would end the pattern's quoted run early, and the comma — like the quote — is outside every pattern's token class, so it joins no two runs into a token a prefix pattern would mask first (an underscore would: it sits inside most token classes).

Measured

Three captured live review streams (the ones behind #6603's figures), sub-agent calls included:

run	calls	arguments total	p50	p90	max	over 8 KiB
32869162122	255	81 KB	110 B	211 B	12.4 KB	1
32871702429	141	100 KB	108 B	279 B	24.5 KB	3
32873411835	117	41 KB	109 B	280 B	8.0 KB	0

Every call over 8 KiB is an Agent dispatch prompt. Replayed through the real collector: with arguments sharing the 256 KiB total, the shipped bounds evict 31–63% of tool results (28–56% before — the baseline replay gives #6603's published 55% / 56% / 28%, in this table's run order); a 1 MiB total still evicts none (records of 456–1,011 KB); encoding adds 8–10% at these bounds. Replayed again through the collector at c8324dfd: all 513 calls keep arguments and their summary, with no finding; the one member dropped by name is Agent.prompt (20 calls, marked). An agent that writes files is measured in the Evidence run below. Raising the total stays #7415.

Decisions

• Dropped whole, not cut; members dropped by name. tool_call parts keep the property feat(telemetry): capture tool results in Level 3 content and emit execute_tool spans #6603 gave them — never cut, only dropped — so the exact-accounting tests stand; the property test now generates arguments and counts them. What is recorded is decided by member name, not by size or by what a scan finds in it: a Write or Edit keeps its file_path and nothing of the body, a Bash keeps its command, an Agent dispatch keeps its description and not its prompt. The cost on the three measured review runs: the 20 Agent prompts are not recorded (the 0–3 per iteration over 8 KiB were not before either). Review of head fa2f0b4 asked for a contract closed by construction rather than another rule; this is the allowlist option of the two it offered.
• The recorded input is the redacted copy, not the bytes the agent received. redactFeedback scans the feedback before it is sanitized and framed, and sanitizing can join a token that scan saw split, so the recorded copy is pattern-scanned again (the prompt the agent is sent is not changed); the pass also folds compatibility characters the prompt kept. Folding can grow the copy about elevenfold (U+FDFA, 3 bytes to 33): worst case about 113 KB for a 10 KiB feedback (113,095 bytes measured), which is why the size is charged to the ceiling rather than assumed small; a test builds that case and pins that input and output together stay within 255,000 bytes.
• summary stays beside arguments: it is what survives when arguments are dropped, and what pi, codex and OpenCode have.
• Findings are one count. Input, arguments and output findings all land in fullsend.content.redactions; Result no longer returns early when an iteration has findings or dropped bytes and no parts (a retry that fails before emitting anything). The input's pattern scan repeats redactFeedback's when sanitizing changed nothing; a connection-string mask left by that first scan matches its own pattern and counts a finding. Secrets redactFeedback already masked are recorded as masks and are not counted (it discards its findings), so a retry prompt carrying ghp_... adds 0 to fullsend.content.redactions; TestAttachInput_RedactsAndCountsFindings feeds attachInput an unmasked token, the case where this scan is the first to see it.
• Tool results stay on the output record. gen_ai.input.messages carries one message, the runner's user prompt. The convention's worked example puts client-executed tool results in that attribute under role:"tool"; here they stay on gen_ai.output.messages, so the role-placement deviation feat(telemetry): capture tool results in Level 3 content and emit execute_tool spans #6603 documented at contentMessage is unchanged, for the reasons given there and in ADR 0108: parts keep stream order, and that record is the scorer contract.
• Runner env values are replaced, not only patterns matched. The sandbox is not meant to hold runner_env credentials, but the record leaves the runner, and the contribution guide's first redaction invariant asks for the literal pass wherever content does. The pass runs ahead of the pipeline, on the text as written, so a pattern does not mask part of a value and keep its first bytes, and again on the pipeline's result, because its normalizer joins a value the stream split with an invisible character or spelled in compatibility forms. Not covered, and pinned by a test for the assignment case: a value written that way which a pattern also recognises — in an assignment, a header, a secret-named field or a connection string, or by its own prefix — is masked by that pattern, and shows what its mask shows. Longer values go first, so a value that contains another is replaced whole and the text does not depend on map order (this changes redactFeedback too). Each replaced key counts once per pass per scanned string in fullsend.content.redactions, and again where a pattern then masks the marker (an assignment, a header, a secret-named field); an id that holds a value is dropped, like an id with any other finding. The parser cuts a call's summary out of its arguments before anything scans it, so a call loses the summary when redaction found a secret — a runner env value or a pattern's — in its arguments; pi, codex and OpenCode report a summary and no arguments, and there a value that straddles the cut stays in part. execute_tool span names and ids get the same redaction (redactText, shared by the collector and the tool span tracker).
• Out of scope: pi/codex arguments and ids (telemetry: pi and codex parsers drop tool call ids and results — Level 3 tool I/O and execute_tool spans are Claude-only #7414), raising the bounds (telemetry: prove a larger OTLP attribute size on the backend, then raise the Level 3 content bounds (total first) #7415), sub-agent nesting (parent_tool_use_id is still dropped at decode, so sub-agent calls and their arguments sit flat and unattributed on the record; it stays ADR 0050's deferred item 1, as ADR 0108's Consequences records — no issue filed yet), and the rest of the model's input: the fixed default prompt is a constant that carries no task and is left out by choice; the requests the runtime builds are not visible to fullsend, which reads the runtime's stream, not its API requests.
• Related: feat(eval-measure): add EM-002 run_health scorer for tool-call defects #7454 (EM-002, eval-measure scores trace health but not run health — add a deterministic tool-call defect scorer #7246) left its behavioural rules out because tool-call arguments were not on the trace. After this PR they are on the Level 3 message record only — content gate on, Claude runs, the listed members only, redacted — and not on the always-on execute_tool spans EM-002 reads. Whether that is enough for those rules is for eval-measure scores trace health but not run health — add a deterministic tool-call defect scorer #7246 to decide.

Evidence

Local gated run of this branch at c8324dfd, 2026-10-08, file sink only — no OTLP endpoint was set, so this shows the record, not backend acceptance. A throwaway agent from fullsend agent new --validation-loop that writes and edits files on purpose: Write a README and a Python module, Edit both, Write a notebook, Read each, Bash ls; feedback_mode: append and a validator that rejects iteration 1 on purpose; claude-opus-4-6 on Vertex; exit 0, validation passed on iteration 2. Trace 4b4238c7d2e203c81d5fb2e502a6884a, 31 spans. The earlier run at e343f872 (2026-09-21, a read-only agent) measured the input message the same way: 592 bytes for the retry prompt, 10 of 10 and 12 of 12 parts carrying arguments.

	iteration 1 agent span	iteration 2 agent span
gen_ai.input.messages	absent	535 bytes: one user message, one text part — the default prompt, the framing, and the validator's line inside the <validation-output> fence
gen_ai.output.messages	7,958 bytes, finish_reason: stop	7,262 bytes, stop
tool_call parts carrying arguments	15 of 15 (6 Bash, 4 Write, 2 Edit, 3 Read)	13 of 13 (5 Bash, 5 Write, 3 Read)
Write/Edit/NotebookEdit parts: arguments kept, summary kept, marked	6 of 6 keep file_path (and replace_all) and their summary, all 6 marked; 1,316 bytes of bodies and edits dropped	5 of 5 keep file_path and their summary, all 5 marked; 543 bytes dropped
held_document findings, redactions, truncation	none, none, fullsend.content.truncated: true (dropped members only)	none, none, true (dropped members only)

A Write part as recorded: {"type":"tool_call","id":"toolu_vrtx_01FxxaAhPEqPkz6tTWCS7zoX","name":"Write","summary":"/sandbox/workspace/target/notes/README.md","arguments":{"file_path":"/sandbox/workspace/target/notes/README.md"},"fullsend.truncated":true}. The execute_tool spans carry gen_ai.operation.name, gen_ai.tool.name, gen_ai.tool.call.id and, on failed calls, error.type — nothing of the arguments. The gate-off case is covered by the pinned test, not by a live run.

Tests

Red first, except where this paragraph says otherwise: three tests were green on arrival, and the attachInput call site has no unit test. 46 hand mutants over the new behaviour (charge, redaction, findings, role and part type, the Result guard, invalid-JSON handling, UseNumber, null, the bound and its boundary, charge unit, key redaction and single key scan, the per-member scan — removed, gated on a clean value, keeping any finding, uncounted, quotes kept, run through the whole pipeline — key collisions, sorted walk, nameless calls, array walk, accounting, single scan, serialized-scan regression, span leakage, both parser paths, renderer) — all killed. Pre-push review over three rounds found one high defect (the member-name scan was skipped when the value raised any other finding) and two regressions in its fixes (an underscore as the quote stand-in; the scanned pair going back through the normalizer), plus wording and accounting errors in the docs and comments — all fixed in these commits. Three tests were green on arrival, with no production code changed under them: arguments absent from execute_tool spans, and arguments and results absent from run-telemetry.jsonl with the gate off while the tool span is still written (both labelled as pins in their commit), and the renderer not printing arguments (renderer.go is untouched). The one line in runAgent that calls attachInput has no unit test, as with attachContent's call site; the evidence run above exercises it. After opening, review led to six fixes, each red first. From Qodo: arguments lost to a redacted-away name were not charged or marked (99f13991), the delta crossing the parser's input cap went in whole (1c124ceb), and the collector had no literal pass for runner env values (c697b84b). From the review agent: that pass ran only ahead of the normalizer, so a value in compatibility forms reached the record (a1c1851a, which also drops the summary of a call whose arguments held a secret and scans number leaves). From the review agent's later rounds: a value nested in an array or object under a secret-named member, and then a key there, kept an opaque credential; provider-only keys missed the literal pass; and execute_tool names and ids had none (3ecc9b50, 1fd2b7d4, e7c1e44e). Keys are masked like values, field names of eight characters or more included, so an object such as {"auth":{"username":…,"password":…}} collides and its call's arguments are dropped, charged and marked — the review bot's suggested fix, taken over numbered placeholders that would have added an output convention. Hand mutants over these fixes are all killed except three that cannot change behaviour and the runAgent line that hands the tracker the runner env, untested like the attachInput call site. From the review agent's later round and waynesun09: a private key over the lines of a direct array and a fold that moves a value between members of a held document (fixed as the bot suggested — a cross-string judgement, and masking whole a held document the normalizer changes — in 938a07c5, with the stripped-escape rule above); and the per-value scan under a long secret-named key running before the bound (6ba0c639). From the review agent's round on 938a07c5: a key-collision drop short-circuited the cross-string judgement (so its finding could not cost the summary), a key masked whole lost its name, and the cross-string pass did not know runner env values — fixed in b1eb40b2, each by judging more; then an exact value over two lines that keep their own breaks (2b07a78c), generalised to the white-space-aside search (f713132b) so no line ending or cut point is a new case; and a key masked whole as a document it holds now names a secret too (fa2f0b43). Each fix masks more, never differently: the fix policy from here is to add a view or widen a mask, so the exported set only shrinks. Two heavier designs for the last fix — a pipeline stage between the normalizer and the patterns, and masking a value's beginning at the end of a string — were built and dropped after review: the first let a combining mark after a value defeat it and its mask hid the keyword a pattern needs; the second masked ordinary words and dropped call ids. A step-back review on 10-08 — field practice is per-field scrubbing or one flat scan of the serialized payload, never a reconstruction across fields — found the precision layer over-built: a scanner of its own for the atoms, match offsets, two join views, a held document judged as scanned and as written. bb50b213 replaces it with the standard decoder's token walk and three coarse rules that only mask or drop more (code in the section 265 → 217 lines); the shelved rewrite's leak rows replay the same, only the documented limits red. Human review of head fa2f0b4 (waynesun09, three findings): ordinary Edit fragments masked whole by the document-shaped fail-closed rule, losing their summary and inflating the redaction count; the design accreting judgements without converging, with two simpler contracts offered; and the evidence measured only on review streams at an older head. c8324dfd takes the allowlist contract — the record holds the listed members of a call, each redacted as text, and nothing else of its input — which removes the held-document, cross-string, member-name, collision and fail-closed rules together (the section is 78 code lines, from 265), leaves no non-secret finding to cost a summary or count as a redaction, and is measured above on the three review streams and on a fix-style run at that head. The review agent's round on c8324dfd found two regressions of that commit — a number under a listed member copied unscanned, and trailing input after the JSON value accepted once the raw bound's validity gate went — plus a stale user-guide sentence and the telemetry file's 0644 mode: fixed in cbfe80e0, each red first (a kept number is scanned as its digits, one JSON value then white space at most, the file created 0600, the dev guide's two sentences on the deleted raw bound and collision rule removed).

[github-actions]

E2E tests are running

Authorization passed for this commit. See the E2E Tests workflow for results.

[qodo-code-review]

PR Summary by Qodo

Capture retry prompts and tool arguments in Level 3 telemetry

✨ Enhancement 🧪 Tests 📝 Documentation 🕐 40+ Minutes

AI Description

• Records redacted retry feedback prompts as Level 3 input messages.
• Captures Claude tool arguments with structured redaction and strict size accounting.
• Documents runtime coverage, privacy boundaries, and telemetry consumer contracts.

Diagram

graph TD
  A["Retry Runner"] -->|feedback prompt| D["Content Collector"] -->|input/output messages| E["Agent Span"] --> F["Telemetry Sink"]
  B["Claude Stream"] --> C["Runtime Parser"] -->|tool arguments| D

Loading

High-Level Assessment

The current approach is appropriate: it preserves raw arguments only until the gated collector can decode and redact them structurally, while keeping the ADR-defined agent span as the sole content carrier. Scanning serialized JSON was correctly rejected because escaping and normalization create redaction gaps, and placing arguments on execute_tool spans would violate their metadata-only contract and content-gate behavior.

Files changed (19) +819 / -82

Enhancement (5) +254 / -45

content_collector.goCollect redacted retry prompts and structured tool arguments +227/-29

Collect redacted retry prompts and structured tool arguments

• Adds input-message attachment and tool-call arguments to Level 3 records. Arguments are decoded, recursively redacted, re-encoded, bounded at 8 KiB, and dropped with truncation and byte accounting when invalid, oversized, or collision-prone.

internal/cli/content_collector.go

run.goAttach retry input when each agent span starts +1/-0

Attach retry input when each agent span starts

• Passes the iteration's composed prompt to the content collector immediately after creating the collector, ensuring normal, cancellation, and error finalization paths retain the input attribute.

internal/cli/run.go

claude_progress.goPreserve Claude tool arguments in normalized events +8/-6

Preserve Claude tool arguments in normalized events

• Populates ToolUseEvent.Arguments from both assembled stream_event input deltas and assistant-line tool_use inputs without decoding or redacting them.

internal/runtime/claude_progress.go

event.goAdd raw arguments to ToolUseEvent +10/-3

Add raw arguments to ToolUseEvent

• Extends the normalized tool-use event with an unredacted JSON-text Arguments field intended exclusively for gated Level 3 collection.

internal/runtime/event.go

telemetry.goWarn about attribute limits for both content records +8/-7

Warn about attribute limits for both content records

• Expands the operator limit warning to cover SDK truncation of gen_ai.input.messages as well as gen_ai.output.messages.

internal/telemetry/telemetry.go

Tests (5) +418 / -2

content_collector_test.goTest input capture and argument safety boundaries +318/-1

Test input capture and argument safety boundaries

• Adds extensive coverage for argument parsing, redaction, numeric preservation, key collisions, invalid JSON, limits, accounting, and one-pass scanning. Also verifies retry input schemas, redaction findings, and shared encoded-size ceilings.

internal/cli/content_collector_test.go

telemetry_run_test.goVerify expanded content capture through the file sink +23/-1

Verify expanded content capture through the file sink

• Extends end-to-end coverage for byte-intact retry input messages and tool arguments. Confirms the disabled gate excludes prompts, arguments, results, and content while metadata-only tool spans remain.

internal/cli/telemetry_run_test.go

tool_spans_test.goEnsure tool arguments never reach execute_tool spans +16/-0

Ensure tool arguments never reach execute_tool spans

• Adds a regression test proving execute_tool spans retain only operation, tool-name, and call-ID metadata even when ToolUseEvent carries sensitive arguments.

internal/cli/tool_spans_test.go

claude_progress_test.goTest Claude argument extraction paths +50/-0

Test Claude argument extraction paths

• Verifies arguments are assembled from streaming deltas, preserved as carried on assistant records, and omitted when the wire provides no tool input.

internal/runtime/claude_progress_test.go

renderer_test.goPrevent tool arguments from reaching console output +11/-0

Prevent tool arguments from reaching console output

• Adds coverage ensuring sensitive ToolUseEvent arguments are never rendered to the user-facing console.

internal/runtime/renderer_test.go

Documentation (9) +147 / -35

0050-distributed-tracing-instrumentation.mdAnnotate ADR 0050 with final Level 3 content additions +11/-0

Annotate ADR 0050 with final Level 3 content additions

• Records the decision to capture runner-composed retry prompts and Claude tool arguments under the existing Level 3 gate. Clarifies why default prompts and runtime-built request context remain excluded.

docs/ADRs/0050-distributed-tracing-instrumentation.md

0108-tool-call-span-topology.mdDocument bounded tool arguments on message records +13/-0

Document bounded tool arguments on message records

• Adds an annotation describing structured argument redaction, the 8 KiB limit, whole-value dropping, collision handling, and continued metadata-only execute_tool spans.

docs/ADRs/0108-tool-call-span-topology.md

tracing.mdExplain argument redaction and retry-input assembly +55/-5

Explain argument redaction and retry-input assembly

• Documents structured JSON argument processing, member-name secret detection, collision handling, dropped-byte accounting, and retry prompt attachment. Expands the consumer contract for input messages and truncated tool calls.

docs/guides/dev/tracing.md

distributed-tracing.mdExpand the Level 3 tracing reference +56/-22

Expand the Level 3 tracing reference

• Adds retry prompts and tool arguments to captured content, including runtime availability, schemas, redaction behavior, limits, and span attributes. Updates measured budget effects and privacy guidance.

docs/guides/infrastructure/distributed-tracing.md

eval-measurements.mdClarify runtime limits for tool-I/O scorers +1/-1

Clarify runtime limits for tool-I/O scorers

• Notes that tool arguments, results, and correlated tool spans remain Claude-only until other runtime parsers expose the required data.

docs/guides/infrastructure/eval-measurements.md

how-to-emit-traces.mdWarn users about expanded Level 3 content +6/-4

Warn users about expanded Level 3 content

• Explains that enabled traces may include retry prompts, executed commands, and written file contents through tool arguments. Reinforces backend access-control requirements.

docs/guides/user/how-to-emit-traces.md

tracing-with-mlflow.mdDescribe retry input messages in MLflow traces +3/-1

Describe retry input messages in MLflow traces

• Documents that agent spans may carry gen_ai.input.messages alongside output messages when validation feedback triggers a retry prompt.

docs/guides/user/tracing-with-mlflow.md

runtimes.mdUpdate the Level 3 runtime capability matrix +1/-1

Update the Level 3 runtime capability matrix

• Marks Claude as supporting tool arguments and all runtimes as supporting runner-composed retry prompts. Clarifies missing argument, ID, and result support in pi and codex.

docs/runtimes.md

scan_output_telemetry_test.goClarify where telemetry content is redacted +1/-1

Clarify where telemetry content is redacted

• Updates test commentary to reflect that Level 3 content is redacted throughout collector assembly rather than only during Result processing.

internal/cli/scan_output_telemetry_test.go

[fullsend-ai-review]

🤖 Finished Review · ✅ Success · Started 6:30 PM UTC · Completed 6:53 PM UTC

Commit: 2a4ed4b · View workflow run →

Runtime: pi · Model: sonnet → claude-sonnet-5 · Effort: high · Cost: $9.85

[github-actions]

Site preview

Preview: https://ba1f3afa-site.fullsend-ai.workers.dev

Commit: cbfe80e06889be32e8def021b8c58640e0f64ead

[codecov]

Codecov Report

❌ Patch coverage is 95.16129% with 6 lines in your changes missing coverage. Please review.

Files with missing lines	Patch %	Lines
internal/cli/content_collector.go	94.44%	4 Missing and 2 partials ⚠️

📢 Thoughts on this report? Let us know!

[qodo-code-review]

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (0) 🔗 Cross-repo conflicts (0) 📜 Skill insights (0)

1. Large calls use excess memory ✓ Resolved

Description

parseClaudeStream checks toolInputJSON.Len() only before appending a complete partial_json
delta, so the builder can exceed the documented maxToolInputSize cap and the new
ToolUseEvent.Arguments assignment forwards the oversized value. When a delta arrives with the
builder just below 1 MiB, it can grow to nearly 2 MiB and reach the Level 3 collector, which
validates and decodes the JSON, recursively redacts it, marshals it, and only then applies the 8 KiB
exported-arguments bound.

Code

internal/runtime/claude_progress.go[299]

+						Arguments: toolInputJSON.String(),

Relevance

●●● Strong

Parser size-bound regressions and token-accounting correctness fixes are consistently accepted,
especially in recent runtime reviews.

PR-#6935
PR-#6907

ⓘ Recommendations generated based on similar findings in past PRs

Evidence

The guard at internal/runtime/claude_progress.go[288-290] tests only the builder's length before
the write and does not constrain the size of d.PartialJSON, despite maxToolInputSize being
documented as the cap. The assignment at internal/runtime/claude_progress.go[296-299] makes the
resulting over-cap builder reachable through ToolUseEvent.Arguments, and
internal/cli/content_collector.go[743-758] shows the collector validating, decoding, recursively
redacting, and marshaling the full value before applying its 8 KiB exported-arguments bound.

internal/runtime/claude_progress.go[288-299]
internal/cli/content_collector.go[743-758]
internal/runtime/claude_progress.go[15-16]
internal/cli/content_collector.go[394-401]
internal/cli/content_collector.go[739-760]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`ToolUseEvent.Arguments` forwards the accumulated `toolInputJSON` builder to the Level 3 collector, but `parseClaudeStream` checks only the builder's current length before appending a complete `partial_json` delta. This permits one final delta to exceed the documented 1 MiB cap and causes avoidable JSON validation, decoding, recursive redaction, and encoding work in the collector.

## Fix Focus Areas
- internal/runtime/claude_progress.go[288-299]

## Recommended Fix
For each `input_json_delta`, calculate `remaining := maxToolInputSize - toolInputJSON.Len()` and, when `remaining > 0`, append at most that many bytes from `d.PartialJSON`. Preserve the existing behavior of emitting the accumulated fragment at `content_block_stop`, so capped, incomplete JSON can subsequently be marked as truncated or dropped by the collector.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

2. Telemetry hides dropped argument bytes ✓ Resolved

Description

Result clears p.Arguments after a tool name redacts to empty without adding their encoded length
to DroppedBytes or setting the truncation marker. A zero-width or otherwise fully sanitized name
therefore removes a previously accepted argument payload while the documented byte total and
completeness flags omit that loss.

Code

internal/cli/content_collector.go[R560-563]

+		if p.Name == "" {
+			// As in Handle, for a name that redacts to nothing. Arguments
+			// Handle had already dropped stay charged and marked.
+			p.Arguments = nil

Relevance

●●● Strong

This is a deterministic telemetry-accounting bug: discarded arguments must update both dropped bytes
and truncation state.

PR-#6429

ⓘ Recommendations generated based on similar findings in past PRs

Evidence

The documentation-sync rule requires user-facing telemetry documentation to match implementation
behavior. The updated reference describes dropped arguments as counted and marked, but the new
name-redaction branch deletes valid arguments without updating either field.

Rule 2748504: Update docs/ references when changing user-facing behavior
internal/cli/content_collector.go[557-570]
docs/guides/infrastructure/distributed-tracing.md[287-289]
internal/cli/content_collector_test.go[212-219]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
Arguments removed because their tool name sanitizes to empty are absent from both dropped-byte accounting and truncation markers, contradicting the documented telemetry contract.

## Fix Focus Areas
- internal/cli/content_collector.go[560-564]
- internal/cli/content_collector_test.go[212-219]
- docs/guides/infrastructure/distributed-tracing.md[287-289]

## Recommended Fix
Before clearing retained arguments, add their encoded length to the result's dropped-byte count and mark the result and retained part as truncated where applicable. Extend the sanitized-name test to assert the documented accounting and marker behavior.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

3. Opaque tokens leak into telemetry ✓ Resolved

Description

toolArguments recursively sends strings through c.redact, but the collector never receives or
scans the sensitive literal values from RunnerEnv. When a tool call contains an opaque token
without a recognized prefix, that value survives the pattern redactor and reaches both the local
telemetry file and any configured exporter.

Code

internal/cli/content_collector.go[R773-776]

+func (c *contentCollector) redactValue(v any, collided *bool) any {
+	switch t := v.(type) {
+	case string:
+		return c.redact(t, &c.findings)

Relevance

●●● Strong

Recent security precedents accept preventing credential leakage; this branch omits required literal
RunnerEnv replacement.

PR-#7510
PR-#7370

ⓘ Recommendations generated based on similar findings in past PRs

Evidence

The rule requires external-facing Go output that may contain credentials to use the repository's
prescribed redaction process. The contribution guide requires both sensitive RunnerEnv literal
replacement and pattern redaction, while the new argument collector only calls the pattern-based
pipeline before placing unredacted runtime input into telemetry.

Rule 3092695: Consult Go credential redaction guide for external-facing code changes
internal/cli/content_collector.go[739-797]
internal/runtime/event.go[52-62]
docs/contributing/go-code.md[457-486]
internal/cli/run.go[3429-3448]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
Tool arguments are exported after pattern-based redaction, but opaque credential values from sensitive runner environment variables are not matched and can enter telemetry unchanged.

## Fix Focus Areas
- internal/cli/content_collector.go[739-797]
- internal/cli/run.go[2319-2324]

## Recommended Fix
Provide the collector with sensitive `RunnerEnv` values and apply the canonical literal-value replacement before recursively applying the existing security pipeline. Preserve the documented minimum secret length and ensure tests cover an opaque token with no recognizable prefix.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

View medium (2)

4. Internal changes lack a test gate ✗ Dismissed

Description

internal/cli and internal/runtime production logic changes without a repository gate that
invokes make go-test or go test ./.... The current workflow reaches make e2e-test only, so
these parser and telemetry changes can merge without the unit-test command required for changed
internal packages.

Code

internal/cli/content_collector.go[R394-401]

+		p := contentPart{Type: "tool_call", ID: boundedID(e.ID), Name: e.Name, Summary: e.Summary}
+		if e.Name != "" {
+			// The schema requires a name on a tool_call part. Arguments
+			// alone must not keep a nameless call, nor charge for one that
+			// appendPart then refuses.
+			p.Arguments, p.Truncated = c.toolArguments(e.Arguments)
+		}
+		c.appendPart(p)

Relevance

●●● Strong

Recent internal-package coverage and CI-gating findings are accepted; repository rules explicitly
require Go tests.

PR-#5615
PR-#6429

ⓘ Recommendations generated based on similar findings in past PRs

Evidence

The changed files activate the rule for production code under internal/. The Makefile defines the
required Go test target, but repository workflows do not invoke it or go test ./...; the relevant
workflow only invokes the separate end-to-end target.

Rule 1062046: Require Go unit tests to pass before committing changes in cmd/ or internal/
internal/cli/content_collector.go[394-401]
internal/runtime/claude_progress.go[296-300]
Makefile[108-119]
.github/workflows/e2e.yml[168-172]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
Production Go changes under `internal/` are not protected by an automated gate that runs the repository's required unit-test target before merge.

## Fix Focus Areas
- .github/workflows/e2e.yml[168-172]
- Makefile[108-119]

## Recommended Fix
Add a required workflow job or step that executes `make go-test` for pull requests and fails on any non-zero result. Configure that status as a required merge check for protected branches.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

5. Structured credentials leak in telemetry ✗ Dismissed

Description

redactValue invokes secretNamed only when a sensitive key's immediate value has Go type
string. Numeric credentials and opaque strings nested in arrays or objects under keys such as
password bypass that key-based mask and remain in exported tool arguments.

Code

internal/cli/content_collector.go[R783-787]

+		for _, k := range slices.Sorted(maps.Keys(t)) {
+			rk := c.redact(k, &c.findings)
+			e := c.redactValue(t[k], collided)
+			if s, ok := e.(string); ok && c.secretNamed(rk, s) {
+				e = "***"

Relevance

●● Moderate

Security concern is plausible, but documentation intentionally limits key-based masking to string
members.

PR-#7510

ⓘ Recommendations generated based on similar findings in past PRs

Evidence

The rule prohibits exposing potential credentials without masking. The new traversal only calls
secretNamed after a successful string type assertion, so non-string values and nested containers
beneath credential-bearing keys never receive key-based masking.

Rule 3092695: Consult Go credential redaction guide for external-facing code changes
internal/cli/content_collector.go[773-797]
internal/cli/content_collector.go[799-820]
docs/contributing/go-code.md[457-486]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
Key-based argument redaction only masks immediate string values, allowing other JSON types and nested opaque credentials beneath sensitive keys to be exported.

## Fix Focus Areas
- internal/cli/content_collector.go[773-820]
- internal/cli/content_collector_test.go[114-155]

## Recommended Fix
Classify sensitive object keys independently of the value type and replace the entire associated JSON value with a mask, or propagate sensitive-key context through recursive traversal. Add cases for numeric, boolean, array, and object values under credential-bearing keys.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

Context sources

✅ Compliance rules (platform): 72 rules

✅ Skills: writing-user-docs, writing-adrs, check-patch-coverage

✅ Cross-repo context — repo relationships

  Explored: repo: fullsend-ai/mlflow-assessments (sha: 7da1f5c7)

Review mode: 🧠 Deep: This is a security-sensitive, cross-cutting telemetry change with substantial new parser, redaction, budgeting, span-attribute, and retry-prompt logic across multiple independent paths, creating a high density of easy-to-miss defects.

Tip of the day

💡 Did you know, you can keep summaries lean with Findings visible per group, which tucks the rest behind a View link

More tips ↗ | Customize Qodo ↗ | Qodo docs ↗

[fullsend-ai-review]

Risk Assessment: moderate (2/5)

Details

Substantial diff size and active churn across runner telemetry components are balanced by zero protected paths, no workflow or dependency modifications, a 0.38 test-file ratio, and established author history, maintaining a moderate risk rating.

Previous run

Risk Assessment: moderate (2/5)

Details

Substantial diff size and active churn across runner telemetry components are balanced by zero protected paths, no workflow or dependency modifications, a 0.35 test-file ratio, and established author history, resulting in a moderate risk rating.

Previous run (2)

Risk Assessment: moderate (2/5)

Details

Moderate risk reflects a substantial 2,695-line telemetry feature across actively churned runner paths, mitigated by zero protected-path or dependency changes, an established author, a 36% test-file ratio with unit tests, and no regression reverts, preserving the prior moderate score following minor corrective commits.

Previous run (3)

Risk Assessment: moderate (2/5)

Details

Moderate risk reflects a substantial 2,691-line telemetry feature across actively churned runner paths, mitigated by zero protected-path or dependency changes, an established author, a 36% test-file ratio with unit tests, and no regression reverts, preserving the prior moderate score following minor corrective commits.

Previous run (4)

Risk Assessment: moderate (2/5)

Details

Moderate risk reflects a substantial 2,607-line telemetry feature across actively churned runner paths, mitigated by zero protected-path or dependency changes, an established author, a 36% test-file ratio with unit tests, and no regression reverts, preserving the prior moderate score.

Previous run (5)

Risk Assessment: moderate (2/5)

Details

Moderate risk reflects a substantial 2,497-line telemetry feature across actively churned runner paths, mitigated by zero protected-path or dependency changes, an increased 36% test-file ratio with added unit tests, established author status, and lack of regression reverts, preserving the prior moderate score.

Previous run (6)

Risk Assessment: moderate (2/5)

Details

Moderate risk reflects a substantial 2,012-line telemetry feature across actively churned runner paths, mitigated by zero protected-path or dependency changes, a 32% test-file ratio, established author status, and lack of regression reverts, preserving the prior moderate score.

Previous run (7)

Risk Assessment: moderate (2/5)

Details

Moderate risk reflects a substantial 1,932-line telemetry feature across actively churned runner paths, mitigated by zero protected-path or dependency changes, a 32% test-file ratio, established author status, and lack of regression reverts, preserving the prior moderate score.

Previous run (8)

Risk Assessment: moderate (2/5)

Details

Moderate risk reflects a substantial 1,916-line telemetry feature across actively churned runner paths, mitigated by zero protected-path or dependency changes, a 32% test-file ratio, established author status, and lack of regression reverts, preserving the prior moderate score.

Previous run (9)

Risk Assessment: moderate (2/5)

Details

Moderate risk reflects a substantial 1,887-line telemetry feature across actively churned runner paths, mitigated by zero protected-path or dependency changes, solid test coverage (32%), established author status, and lack of regression reverts, preserving the prior moderate score.

Previous run (10)

Risk Assessment: moderate (2/5)

Details

Moderate risk reflects a substantial 1,750-line telemetry feature across actively churned runner paths, mitigated by zero protected-path or dependency changes, solid test coverage (32%), established author status, and lack of regression reverts, preserving the prior moderate score.

Previous run (11)

Risk Assessment: moderate (2/5)

Details

Moderate risk reflects a substantial 1,557-line telemetry feature across actively churned runner paths, mitigated by opt-in environment variable gating, strong test coverage additions, absence of protected paths or dependency changes, and alignment with the authorized predecessor scope.

Previous run (12)

Risk Assessment: moderate (2/5)

Details

Moderate risk reflects a substantial 1,426-line telemetry feature across actively churned runner paths, mitigated by opt-in environment variable gating, strong test coverage additions, absence of protected paths or dependency changes, and alignment with the authorized predecessor scope.

Previous run (13)

Risk Assessment: moderate (2/5)

Details

Moderate risk reflects a substantial 1,279-line feature with active git churn across CLI telemetry paths, mitigated by no protected-path, workflow, or dependency changes, test additions, scope alignment with predecessor PR #6603, and opt-in environment-variable gating.

Previous run (14)

Risk Assessment: moderate (2/5)

Details

Large diff (1,280 lines across 22 files) is offset by zero protected-path or dependency changes and adequate test coverage, keeping Tier 1 at 1.75/5, while CLI file churn and regression history place Tier 2 at 2.57/5, yielding a 62/38 weighted composite of ~2.06 that preserves the prior moderate score of 2.

Previous run (15)

Risk Assessment: moderate (2/5)

Details

Large diff (1,280 lines across 22 files) is offset by zero protected-path or dependency changes and adequate test coverage, keeping Tier 1 at 1.75/5, while CLI file churn and regression history place Tier 2 at 2.57/5, yielding a 62/38 weighted composite of ~2.06 that preserves the prior moderate score of 2.

Previous run (16)

Risk Assessment: moderate (2/5)

Details

No linked issue, so weight redistributes to 62% Tier 1 / 38% Tier 2. Tier 1 unchanged from prior review (1.75/5). Tier 2 recomputed fresh at current head (~2.6/5). Composite ~2.09, rounding to 2 (moderate); preserved per re-review anchoring since neither tier crossed a rounding boundary.

Previous run (17)

Risk Assessment: moderate (2/5)

Details

No linked issue, so weight redistributes to 62% Tier 1 / 38% Tier 2. Tier 1 is essentially unchanged from the prior review (1.75/5: large blast radius/line-count bucket, zero protected/security/CI/dependency changes, non-bot non-first-time author, ~35% test ratio). Tier 2 rose slightly to 3.0/5 (from 2.57) on high fix/revert-grep counts and change-coupling on hotspot files, offset by low code age and zero true revert/sentiment hits. Composite 0.62x1.75 + 0.38x3.0 = 2.23, rounding to 2 (moderate) — consistent with the prior assessment (score preserved per re-review anchoring; the Tier 2 increase did not cross a rounding boundary).

Previous run (18)

Risk Assessment: moderate (2/5)

Details

Large diff (901 lines/19 files, blast_radius=large) is offset by no protected/security-path or CI/dependency changes, a non-first-time non-bot author, and adequate test coverage (32%), keeping Tier 1 low (1.75/5); Tier 2 is pulled to moderate (2.57/5) mainly by heavy recent fix-commit history and churn on hotspot files with some unaddressed change-coupling; no formally linked issue redistributes weight to 62% Tier 1 / 38% Tier 2, yielding a moderate composite score of 2.

[fullsend-ai-review]

Looks good to me

Earlier findings

• f_140e9eda991abfdd — resolved_by_change: listed numeric arguments now pass through scanLeaf before export, with a numeric runner-credential regression test.
• f_38cb48aedc7742a3 — resolved_by_change: telemetry files are now created with mode 0600. The supported runner path uses a freshly generated run directory and initializes the exporter once before retries.
• f_33ba0b1a80bf4efe — resolved_by_change: decodeJSON now requires EOF after the first value, rejecting trailing text or another JSON value; regression tests cover both and permit trailing whitespace.
• f_19a7cb82df9b61d2 — resolved_by_change: the user guide now describes captured commands, paths and patterns rather than file bodies or edits.

Previous run

Review

Findings

High

• [secret-exposure] internal/cli/content_collector.go:829 — Allowlisted numeric arguments bypass redaction. With RunnerEnv["DEPLOY_PASSWORD"] = "12345678", a Read call containing {"offset":12345678} retains the complete credential without a finding. Arguments are not rescanned by Result, and the telemetry file is exempt from the subsequent output scan.
  Remediation: Scan retained scalar encodings through the credential-redaction path. Preserve their JSON type when unchanged; otherwise safely mask or drop them, recording findings and truncation/accounting as appropriate. Add numeric regression cases for runner-env and provider-only credentials.

Medium

• [data-exposure] internal/telemetry/telemetry.go:251 — Retry-input capture introduces a copy of validation feedback into run-telemetry.jsonl, created with mode 0644, while the original feedback audit file uses 0600. Under a typical 022 umask and traversable parent directories, other local users can read this new content. The permissions predate this PR, but copying protected feedback into this sink expands the exposure; redaction does not remove all proprietary information or PII.
  Remediation: Create the telemetry file with mode 0600, enforce restrictive permissions on existing files before appending, and add a permission regression test.

• [logic-error] internal/cli/content_collector.go:857 — decodeJSON accepts a valid JSON prefix without checking for trailing input. For example, {"command":"ls"}garbage or two concatenated objects are recorded as complete arguments without truncation or dropped-byte accounting. The stream-delta producer does not validate the accumulated arguments.
  Remediation: Require a second decode to return io.EOF, allowing trailing whitespace only. Route other outcomes through malformed-input scanning, dropping and accounting; test trailing garbage, multiple values, and null followed by additional input.

Low

• [incorrect-doc] docs/guides/user/how-to-emit-traces.md:129 — The guide says captured tool arguments include “the file contents it wrote,” but the allowlist excludes file bodies, edits and notebook source. Those members are dropped and marked truncated rather than captured.
  Remediation: Describe captured arguments as commands, paths, patterns and execution flags, and explicitly exclude file bodies and edits from argument capture.

---

Next steps:

• /fs-fix — agent addresses review findings automatically
• /fs-fix <your instruction> — agent fixes with your specific guidance
• Push commits directly — review re-runs automatically on push
• /fs-fix-stop — disable automatic fix runs for this PR

Previous run (2)

Review

Reason: ambiguous-findings

This PR was NOT reviewed. Do not count this as an approval.

Previous run (3)

Review

Findings

Medium

• [secret-exposure] internal/cli/content_collector.go:1024 — The cross-string scan still accepts only private_key pattern matches. Arguments such as {"cells":[{"source":["API_KEY=","opaque-credential-123\n"]}]} retain the credential: neither string is recognizable individually, but their newline-joined representation matches env_assignment, which this filter ignores. Unless independently recognized or registered as a runtime/runner-environment secret, the credential reaches Level 3 telemetry. Authorization-header context crossing source-string boundaries has the same gap.
  Remediation: Add contextual cross-string detection for source arrays, masking or dropping the containing arguments/document when assignment or authorization-header context reveals a credential. Add opaque, unregistered credential regressions while preserving tests against false positives from unrelated metadata fields.

Earlier findings

• f_332d6e0ee514ba38 — resolved_by_change: redactValue now checks the exported key (rk == "***") at line 1177, propagating secret-member context when a held document masks its key. Added regressions at internal/cli/content_collector_test.go:470–471 cover direct and nested values.

---

Next steps:

• /fs-fix — agent addresses review findings automatically
• /fs-fix <your instruction> — agent fixes with your specific guidance
• Push commits directly — review re-runs automatically on push
• /fs-fix-stop — disable automatic fix runs for this PR

Previous run (4)

Review

Findings

High

• [logic-error] internal/cli/content_collector.go:1177 — Secret-name propagation checks sk == "***", but held-document inspection can subsequently mask only rk. A held-document key containing an escaped secret name can therefore be exported as "***" while its associated opaque credential remains unmasked. Nested associated values are affected too.
  Remediation: Include rk == "***" in the propagation guard. Add regression tests for held-document keys with escaped secret names and uninspectable documents, covering direct and nested associated values.

Medium

• [secret-exposure] internal/cli/content_collector.go:1024 — Cross-atom pattern detection accepts only private-key matches. An unregistered GitHub token split into ghp_ plus 18 token characters in one notebook source string and the remaining 18 characters in another does not match either leaf's token pattern. Both fragments remain reconstructable in exported arguments; notebook JSON held in a Write content string has the same gap. The dense cross-atom check only covers registered runtime secrets and environment literals.
  Remediation: Add boundary-aware detection for recognizable tokens spanning strings, including held documents, and drop the affected arguments or mask the held document. Add a regression test with an unregistered GitHub token split between source-array strings.

Earlier findings

• f_194c0d1068991e10 — resolved_by_change: spanning now checks exact runtime and environment values with whitespace set aside across atoms. The incremental diff adds coverage for retained line breaks, Windows endings, trailing whitespace, and mid-word splits, including held documents (internal/cli/content_collector_test.go:490–516).

---

Next steps:

• /fs-fix — agent addresses review findings automatically
• /fs-fix <your instruction> — agent fixes with your specific guidance
• Push commits directly — review re-runs automatically on push
• /fs-fix-stop — disable automatic fix runs for this PR

Previous run (5)

Review

Findings

Medium

• [secret-exposure] internal/cli/content_collector.go:1013 — Multiline credentials can escape redaction when array elements already retain their newlines. For DEPLOY_SECRET="opaque-line-one\nopaque-line-two", arguments {"lines":["opaque-line-one\n","opaque-line-two"]} contain no complete credential in either leaf. The spanning check inserts another newline between elements, so its exact matcher also misses the credential, leaving both fragments in exported content. Registered runtime secrets and held JSON documents share this gap.
  Remediation: Also check sibling array strings concatenated with their existing newline boundaries preserved, while retaining the separator-based check. Add direct and held-document regressions for runner-env and registered-runtime secrets.

Earlier findings

• f_2904ff55ea35d3e4 — resolved_by_change: toolArguments now evaluates c.spanning(args) before collision/size dropping, preserving findings that suppress a secret-bearing summary; covered by TestContentCollector_ACollisionStillCountsASecretAcrossTheStrings.
• f_6c0d225b85f7c5fa — resolved_by_change: redactValue treats a whole-masked key (sk == "***") as secret-named and propagates that context to descendants; covered by TestContentCollector_AKeyMaskedWholeNamesASecret.
• f_56e4a6473e2e4b06 — resolved_by_change: the same whole-masked-key change subjects nested values to secret-member masking instead of losing the key's context.
• f_9715bcff7b18b2f0 — resolved_by_change: spanning now searches envLiterals(c.runnerEnv), including sensitive runner-env and provider-only values. The new finding above concerns newline reconstruction, not the previous omission of those matchers.

---

Next steps:

• /fs-fix — agent addresses review findings automatically
• /fs-fix <your instruction> — agent fixes with your specific guidance
• Push commits directly — review re-runs automatically on push
• /fs-fix-stop — disable automatic fix runs for this PR

Previous run (6)

Review

Findings

High

• [logic-error] internal/cli/content_collector.go:823 — A normalization-only key collision short-circuits c.spanning(args). When a private-key block is split between Bash's command and description, adding colliding keys such as a and \uFF41 drops the arguments without recording the secret finding. Handle therefore retains the parser-derived command summary, which can contain private-key material.
  Remediation: Run spanning independently before combining drop conditions. Add a regression combining colliding keys with a cross-string private key and assert that the summary is omitted.

• [logic-error] internal/cli/content_collector.go:1103 — Secret-member classification uses the exported key mask instead of the normalized detection key. A key consisting of \u001b[0m followed by fullwidth password normalizes to password, but scanLeaf replaces it with ***. Neither that mask nor the original fullwidth spelling matches namesASecret, so an opaque credential beneath it is exported unchanged. See also: [secret-exposure] at this location.
  Remediation: Preserve an independent normalized detection view of keys, rather than classifying their exported masks; test combined terminal escapes and compatibility-spelled secret names with nested values.

Medium

• [secret-exposure] internal/cli/content_collector.go:1103 — The incomplete-CSI variant also defeats secret-name context: \u001b[ followed by fullwidth password folds to ESC[password, after which stripping consumes ESC[p. The key is masked, but its opaque value remains. A post-stripping normalized-key check alone would still see only assword, so it would not close this variant. See also: [logic-error] at this location.
  Remediation: Conservatively mask the subtree when stripping makes key classification unreliable, or classify a compatibility-normalized detection view before terminal stripping. Cover the incomplete-CSI case and nested values.

• [secret-exposure] internal/cli/content_collector.go:1014 — Cross-string detection checks registered runtime secrets and patterns, but not the sensitive environment literals used by replaceEnvSecrets. For an otherwise unregistered DEPLOY_SECRET equal to opaque-line-one\nopaque-line-two, arguments containing those two lines as separate array elements evade the per-leaf literal pass. spanning reconstructs the value but never checks that literal, leaving both lines in telemetry.
  Remediation: Check spanning detection views against the same eligible runner-env and provider-only literals as replaceEnvSecrets, and drop arguments or mask held documents for cross-boundary matches. Add an unregistered multiline-secret regression.

Earlier findings

• f_1242937fc648c27e — resolved_by_change: The incremental change makes heldSecret mask a held document when normalization changes its written form, closing the case where folding moves a credential out of its secret-named member while preserving valid JSON.
• f_3a318f0c833b9e66 — resolved_by_change: The incremental change adds decoded/normalized cross-string scans for private-key blocks and whole-leaf masking after escape/tag stripping; corresponding array, stripped-token, and file-sink regressions were added. The remaining combinations above are distinct new findings.

---

Next steps:

• /fs-fix — agent addresses review findings automatically
• /fs-fix <your instruction> — agent fixes with your specific guidance
• Push commits directly — review re-runs automatically on push
• /fs-fix-stop — disable automatic fix runs for this PR

Previous run (7)

Review

Findings

Medium

• [fail-open] internal/cli/content_collector.go:909 — heldSecret inspects only the scanned document when it remains valid JSON. Unicode folding can change its structure without invalidating it: literal fullwidth quotation marks inside credentials.value can become JSON delimiters, moving an opaque credential into a new note.value member. The walk then sees no secret-named ancestor and exports the credential. Recording a normalization finding does not mask the held string. The new arguments-size bound and member-name optimization do not resolve this earlier finding.
  Remediation: Inspect both the written and scanned documents, or mask the held string when normalization/redaction changes its JSON structure. Add a valid-to-valid structural-change regression test.

• [secret-exposure] internal/cli/content_collector.go:967 — Valid, bounded structured arguments are scanned leaf-by-leaf, without a whole-document check. A notebook-shaped cells[].source array containing a PEM private-key begin line, body lines, and end line in separate strings retains the recoverable key: no leaf contains a complete block, and cells/source provide no secret-named ancestor. This applies when the credential is not separately registered for literal redaction. Held JSON strings receive a whole-text scan, but direct objects and arrays do not. This earlier finding also remains open.
  Remediation: Add whole-document detection for credentials spanning leaves while retaining decoded per-leaf redaction. Drop or mask arguments when such a credential cannot be safely localized, and add a split-PEM regression test.

---

Next steps:

• /fs-fix — agent addresses review findings automatically
• /fs-fix <your instruction> — agent fixes with your specific guidance
• Push commits directly — review re-runs automatically on push
• /fs-fix-stop — disable automatic fix runs for this PR

Previous run (8)

Review

Findings

Medium

• [fail-open] internal/cli/content_collector.go:907 — heldSecret inspects only the scanned document when normalization leaves valid JSON. Fullwidth quotation marks (U+FF02) can change a held document's structure while preserving valid JSON and unique member names: a long value originally inside credentials.v can become data.v after folding, leaving only a short value under credentials. The normalized walk then finds no secret and retains the credential in the output messages. Invalid-document and duplicate-name checks do not cover this case.
  Remediation: Inspect the original held document as well as the scanned document, or mask the string whole when normalization changes its decoded structure. Add a regression test for a valid-to-valid structural change that moves a credential out of a secret-named ancestor.

• [secret-exposure] internal/cli/content_collector.go:960 — Native JSON arguments are scanned one value at a time. A PEM private key represented as an array of lines under a non-secret-named member such as lines therefore escapes the private-key pattern: the BEGIN marker, body, and END marker arrive in separate scans. The exported arguments retain the fragments, allowing the key to be reconstructed. The whole-text scan for a document held inside a string does not cover native argument arrays.
  Remediation: Add whole-document credential inspection alongside the decoded per-value pass. Mask the containing value or drop arguments whole when a multi-value private-key block is detected, and add a regression test for an array of PEM lines.

---

Next steps:

• /fs-fix — agent addresses review findings automatically
• /fs-fix <your instruction> — agent fixes with your specific guidance
• Push commits directly — review re-runs automatically on push
• /fs-fix-stop — disable automatic fix runs for this PR

Previous run (9)

Review

Findings

Medium

• [fail-open] internal/cli/content_collector.go:900 — Invalid document-shaped strings are retained when the initial text scan leaves them unchanged. For example, a Write argument containing {"credentials":{"value":"opaque-value-123"},} exports the credential unless literal replacement independently recognizes it. The trailing comma prevents recursive inspection, while the text pattern cannot associate the nested value with credentials. Valid documents receive inspection, but malformed/JSONC content bypasses it.
  Remediation: Mask document-shaped strings that cannot be decoded even when the initial scan made no change, or inspect supported JSONC syntax before retaining them. Add regressions for trailing commas, comments, and escaped credentials.

Low

• [secret-exposure] internal/cli/content_collector.go:830 — Held-document inspection accepts objects and arrays only. A Write content argument containing the JSON string literal "\u0061bcdefghijklmno" conceals the known runner credential DEPLOY_PASSWORD=abcdefghijklmno: the outer argument is decoded, but the held scalar string is not, so neither literal replacement nor pattern scanning recognizes the credential.
  Remediation: Inspect held JSON string values as well as objects and arrays, retaining the depth bound, and mask the containing string when decoding reveals a credential. Add a regression for an escaped runner credential in scalar JSON file content.

---

Next steps:

• /fs-fix — agent addresses review findings automatically
• /fs-fix <your instruction> — agent fixes with your specific guidance
• Push commits directly — review re-runs automatically on push
• /fs-fix-stop — disable automatic fix runs for this PR

Previous run (10)

Review

Findings

Medium

• [fail-open] internal/cli/content_collector.go:894 — A held string that looks like a JSON document but does not parse (trailing comma, JSONC comments) is exported unchanged unless the text scan happened to modify it. Example: a Write argument content of {"credentials":{"value":"opaque-credential-123"},} — the json_field pattern needs a quoted value right after the key, so redaction changes nothing and scanned == written; jsonDocument rejects the text, and the scanned != written && ... condition makes heldSecret return false even though documentLike is true. The nested credential is exported with no finding, whereas duplicate-name and over-deep documents are masked as uninspected. JSONC files with nested secret-named objects (tsconfig, devcontainer.json, editor settings) are realistic Write payloads.
  Remediation: Mask document-shaped content that cannot be inspected whether or not the preliminary scan changed it, or parse relaxed formats (JSONC) with a fallback that fails closed. Add a regression case for a trailing comma.

Low

• [secret-exposure] internal/cli/content_collector.go:832 — A leading space before a BOM gets past the broken-document fallback. In documentLike, strings.TrimPrefix(s, "\uFEFF") runs before strings.TrimSpace, and TrimSpace does not strip U+FEFF, so \uFEFF{...} is not recognised as document-like as written. If a registered runner-env value ending at the document's closing "} is replaced by the literal pass (leaving the scanned text ending in ]) and the normalizer strips the BOM, neither documentLike(scanned) nor documentLike(written) holds, and a nested credential the patterns do not recognise is exported unmasked. The runner-env value itself is still replaced. The trigger is narrow.
  Remediation: Strip leading whitespace and BOMs in a loop (or normalize first) before testing document shape, and add this broken-document regression case.
• [secret-exposure] internal/cli/content_collector.go:915 — revealing does not treat the removal of an ANSI CSI sequence as revealing. A digits-only credential written as a CSI parameter (\u001b[31415926535m) under a secret-named member of a held document is stripped to an empty string in heldSecret's walk; secretNamed cannot match an empty value, the only finding is ansi_escape, and maskHeld keeps the original escaped text with all the digits. OSC and tag-character removals are covered by the new predicate; CSI removals are not. Reaching this requires deliberate encoding of the credential.
  Remediation: Within the held-document walk, judge the decoded value under its secret-named ancestor before normalization removes content, or treat CSI removals as revealing there.
• [secret-exposure] internal/cli/content_collector.go:423 — The summary is cleared only when a non-normalizer finding is raised in the arguments. A Bash command that wraps a token in an OSC sequence (ESC]ghp_...BEL) whose terminator the parser's 120-rune collapseCommand cut removes leaves a summary that the full-argument scan only records as osc_escape, so Summary is kept; Result cannot strip the unterminated sequence, and the remaining ghp_ plus 32 characters is below the 36-character PAT pattern, so a partial token survives in the summary. Summaries were exported before this PR without any clearing, so this is a gap in the new mitigation rather than a regression.
  Remediation: Also clear the summary for content-bearing normalizer findings (osc_escape, tag_char), and add a parser-to-collector regression test where truncation removes the escape terminator.

---

Next steps:

• /fs-fix — agent addresses review findings automatically
• /fs-fix <your instruction> — agent fixes with your specific guidance
• Push commits directly — review re-runs automatically on push
• /fs-fix-stop — disable automatic fix runs for this PR

Previous run (11)

Review

Findings

Medium

• [secret-exposure] internal/cli/content_collector.go:856 — heldSecret keeps only json_field findings and discards other secrets discovered by the decoded walk. For a held document such as {"command":"API_KEY=opaque-credential-123"}, the whole-string scan misses the assignment because it follows a quotation mark; the decoded walk detects env_assignment, but that finding and its sanitized output are discarded. Escaped prefix tokens and exact runtime/runner-env credentials can likewise become recognizable only after decoding.
  Remediation: Mask the held string when decoding exposes any credential requiring redaction, distinguishing new secret matches from rescanned masks. Add assignment and escaped-token/env regressions.

• [secret-exposure] internal/cli/content_collector.go:841 — At maxHeldDepth, inspection returns “no secret” and permits the containing string to be exported. Nested secret-named members beyond that bound evade text scanning. TestContentCollector_JSONHeldDeeperThanTheBoundIsScannedAsText explicitly expects the opaque credential to survive at depth five.
  Remediation: Retain the resource bound, but propagate an inspection-limit result and conservatively mask or drop the containing string. Change the regression to require that the credential is absent from OutputMessages.

• [secret-exposure] internal/cli/content_collector.go:816 — Decoding held documents into maps removes earlier duplicate members, while the exported string preserves them. A held document containing {"credentials":{"value":"opaque-credential-123"},"credentials":{}} leaves only the empty replacement object for inspection. The text scan misses the nested opaque value, so the original string remains exportable with the credential intact.
  Remediation: Visit every member using token-based inspection, or detect duplicate keys and mask/drop the containing string. Add a duplicate-member regression.

Low

• [secret-exposure] internal/cli/content_collector.go:905 — Object keys receive text redaction but not held-document inspection. An argument whose key is the string {"credentials":{"value":"opaque-credential-123"}} and whose value is "x" retains the nested credential in the exported key.
  Remediation: Inspect JSON held in keys too; mask the key or drop the arguments, preserving collision detection. Add a held-JSON-key regression.

---

Next steps:

• /fs-fix — agent addresses review findings automatically
• /fs-fix <your instruction> — agent fixes with your specific guidance
• Push commits directly — review re-runs automatically on push
• /fs-fix-stop — disable automatic fix runs for this PR

Previous run (12)

Review

Findings

Medium

• [secret-exposure] internal/cli/content_collector.go:820 — JSON embedded in a string argument bypasses nested secret-member redaction. For example, a Write call whose content is {"credentials":{"value":"opaque-credential-123"}} can retain that credential when it is not a known environment or registered runtime secret. The string branch pattern-scans the document without decoding it recursively; json_field requires a quoted value directly after the secret-named key, so it misses this nested object. With Level 3 enabled, the credential can reach the exported tool arguments.
  Remediation: Apply bounded recursive structured redaction to string values containing JSON documents, preserving their string type after re-encoding, or drop such content when nested secret-named members cannot be safely redacted. Add regression tests for embedded objects and arrays under credentials, password, and token members.

---

Labels: The PR extends credential redaction on exported telemetry and has a remaining secret-exposure finding.

---

Next steps:

• /fs-fix — agent addresses review findings automatically
• /fs-fix <your instruction> — agent fixes with your specific guidance
• Push commits directly — review re-runs automatically on push
• /fs-fix-stop — disable automatic fix runs for this PR

Previous run (13)

Review

Findings

High

• [secret-exposure] internal/cli/content_collector.go:836 — Secret-named ancestor context is applied to nested values, but not object keys. Arguments such as {"credentials":{"opaque-credential-123":"user"}} retain the complete opaque credential as a key in gen_ai.output.messages. Standalone prefix-pattern and known-environment checks do not protect credentials recognizable only through the enclosing member.
  Remediation: Apply the enclosing secret-name check to keys too, or drop secret-bearing objects whole. Preserve collision handling and add nested-key regression tests.

Low

• [secret-exposure] internal/cli/content_collector.go:899 — Runner and provider-only literals are replaced in separate passes. If an eligible runner literal is a prefix of the longer provider credential, the runner pass replaces that prefix first; the provider pass can no longer match the full credential, leaving its suffix in exported content. redactFeedback uses the same ordering.
  Remediation: Combine both credential sources into one longest-first replacement pass and test overlap in both directions.

• [secret-exposure] internal/cli/tool_spans.go:178 — The metadata carrier does not receive the literal checks added to the collector. safeName and safeID use only the pattern/runtime-secret pipeline, so opaque runner or provider-only credentials in names or call IDs can survive on execute_tool spans, including with content capture disabled. This tracker omission predates the PR.
  Remediation: Supply credential-redaction context to the tracker, replace literal matches in names before bounding, and drop IDs on literal matches. Add tests for both fields and credential sources.

---

Next steps:

• /fs-fix — agent addresses review findings automatically
• /fs-fix <your instruction> — agent fixes with your specific guidance
• Push commits directly — review re-runs automatically on push
• /fs-fix-stop — disable automatic fix runs for this PR

Previous run (14)

Review

Findings

High

• [secret-exposure] internal/cli/content_collector.go:825 — Sensitive-member-name protection only applies when the recursively redacted value is a string. Arrays, objects, and unchanged numbers lose their enclosing member’s sensitive context. For example, {"api_keys":["opaque-credential-123"]} and {"credentials":{"value":"opaque-credential-123"}} retain credentials that do not independently match a known literal or token pattern. With Level 3 enabled, those arguments can be written to telemetry files or exported to a configured backend.
  Remediation: Detect sensitive member names independently of the value’s type, then mask the associated value or propagate sensitive context to its descendants. Add regression tests for arrays, objects, and numbers under sensitive names.

Low

• [secret-exposure] internal/cli/content_collector.go:865 — The collector’s literal replacement pass reads only RunnerEnv, omitting process-environment providerOnlyKeys. GH_WORKFLOW_TOKEN is deliberately excluded from RunnerEnv; its runtime-redactor registration is also skipped when it is already populated. A preconfigured value that appears in captured content and does not match existing token patterns can therefore survive collection. Unlike redactFeedback, this storage boundary lacks the provider-only literal pass required by the contribution guide.
  Remediation: Include provider-only environment values in literal replacement before and after normalization, or register every preconfigured provider-only credential before collection. Add a regression test with a preconfigured, otherwise unrecognized value absent from RunnerEnv.

---

Labels: The PR changes Go code implementing agent-runner telemetry capture.

---

Next steps:

• /fs-fix — agent addresses review findings automatically
• /fs-fix <your instruction> — agent fixes with your specific guidance
• Push commits directly — review re-runs automatically on push
• /fs-fix-stop — disable automatic fix runs for this PR

Previous run (15)

Review

Findings

Low

• [sub-agent-failure] N/A — The challenger adjudication pass returned an empty adjudicated_findings array for a non-empty input (three info-level schema-semantics findings from the cross-repo-contracts sub-agent, each confirming rather than challenging that the PR's new OTLP schema additions — arguments on tool_call parts, the expanded fullsend.truncated/fullsend.content.dropped_bytes/fullsend.content.redactions semantics — are additive and backward compatible for existing consumers). Per the orchestrator's guard against an unsubstantiated wipe, an empty challenger result for a non-empty input is treated as a challenger failure regardless of the reasoning supplied, and the pre-challenger merged finding set is used unchanged. The three info-level findings themselves fall below this review's severity threshold and are not reproduced here — they identified no defect. This is a process note about the review pipeline, not a defect in the PR.

No other findings met the review threshold. correctness, security, intent-coherence, style-conventions, and docs-currency all returned clean on this round, including independent re-verification of the fixes from earlier rounds (the NFKC-fold/runner-env-secret redaction ordering fix in content_collector.go, and the fullsend.truncated dual-meaning documentation in distributed-tracing.md).

Previous run (16)

Review

Findings

Low

• [schema-semantics] docs/guides/infrastructure/distributed-tracing.md:161 — fullsend.truncated is overloaded by part type: on tool_call_response parts it means "bulk cut, tail kept"; on tool_call parts it now means "arguments dropped whole" (id/name/summary only, no partial data survives). At the current PR head this dual meaning is stated explicitly in prose right at the cited line ("So fullsend.truncated means different things by part type..."), restated in the attribute table further down the same file, and restated again in docs/guides/dev/tracing.md's Consumer contract section. This is additive schema — tool_call parts never carried arguments before this PR, so there is no prior consumer behavior being silently redefined, unlike the adjacent Pi gen_ai.system breaking change in the same file, which does use a formal callout.
  Remediation: Optional — promoting the existing disambiguation sentence into the same blockquote "Breaking change"-style format used for the Pi gen_ai.system note nearby would improve skimmability, but this is not a breaking change and no callout is required for merge.

• [sub-agent-failure] N/A — The challenger adjudication pass returned an empty adjudicated_findings array for a non-empty input (two merged schema-semantics findings from the docs-currency and cross-repo-contracts sub-agents, describing the same passage in distributed-tracing.md). Per the orchestrator's guard against an unsubstantiated wipe, an empty challenger result for a non-empty input is treated as a challenger failure regardless of the reasoning supplied, and the pre-challenger merged finding set is used unchanged (the finding above). This is a process note about the review pipeline, not a defect in the PR.

No other findings met the review threshold. correctness, security, and style-conventions returned clean. The prior review's Medium data-exposure finding (redaction ordering around NFKC normalization in attachInput/replaceEnvSecrets) was independently re-verified this round and confirmed fixed: the literal env-secret pass now runs on both sides of pipeline.Scan, with a test covering a fullwidth-encoded opaque secret through attachInput and tool-argument redaction.

Previous run (17)

Review

Findings

Low

• [schema-semantics] docs/guides/infrastructure/distributed-tracing.md:153 — fullsend.truncated is overloaded by part type: on tool_call_response parts it means "bulk cut, tail kept"; on tool_call parts it now means "arguments dropped whole" (id/name/summary only, no partial data survives). This dual meaning is already documented in this guide's Content capture prose and attribute table, and docs/guides/dev/tracing.md's Consumer contract section already tells readers to branch on part type before interpreting the marker. This is additive schema (arguments are new on tool_call parts, which were never previously tail-cut), not a silent rewrite of prior behavior.
  Remediation: Optional — a short "Breaking change"-style callout in the Content capture section of distributed-tracing.md would make the dual meaning easier to skim for a consumer scanning for behavior changes, but is not required for merge since the consumer contract already states the branching rule.

Previous run (18)

Review

Findings

Medium

• [data-exposure] internal/cli/content_collector.go:719 — The prior review's attachInput finding is only half-closed by this PR. run.go:2324 now threads h.RunnerEnv into newContentCollectorIfEnabled, and attachInput/tool-argument redaction go through c.redact, which calls replaceEnvSecrets (a pre-fold literal ReplaceAll against the raw ASCII env value) before c.pipeline.Scan (security.OutputPipeline() = UnicodeNormalizer then SecretRedactor, unchanged by this PR). Because the literal pass runs before NFKC folding, an opaque runner-env secret (PUSH_TOKEN, *_SECRET, *_PASSWORD, an unstructured *_KEY — exactly the class this pass exists to catch, since it has no shape a pattern knows) that appears only in fullwidth/compatibility-character form will not match the literal ASCII value, survive the literal pass unredacted, then get reconstructed to its plain ASCII form by NFKC folding in pipeline.Scan — with no second literal pass afterward to catch it. sanitizeFeedbackUnicode deliberately leaves compatibility-only text unfolded for the agent-facing prompt, so this is not a hypothetical: a retry-feedback prompt carrying a compatibility-encoded secret keeps it unfolded for the agent but can still surface the folded/reconstructed ASCII form in the exported gen_ai.input.messages telemetry. No test in this diff exercises the combination of an opaque runner-env-shaped secret in fullwidth/compatibility form flowing through attachInput/c.redact — TestContentCollector_ReplacesRunnerEnvSecretValues uses a plain ASCII secret, and the PAT-shaped test (ghp_...) is caught by SecretRedactor's pattern stage post-fold, not by this literal pass.
  Remediation: After c.pipeline.Scan, run replaceEnvSecrets again on the pipeline's output (Sanitized when non-empty, otherwise the already-literal-replaced text) so an NFKC-canonicalized copy cannot reconstruct an env secret the pre-fold literal pass missed. Keep the existing OutputPipeline scan so fullwidth-encoded prefix-shaped tokens (ghp_, github_pat_, glpat-, ...) still match post-fold. Add a test asserting that a fullwidth encoding of an opaque PUSH_TOKEN value redacts to [REDACTED:PUSH_TOKEN] through both attachInput and tool-argument/result content.

Low

• [architectural-fit] docs/ADRs/0108-tool-call-span-topology.md:128 — Carried over from the prior review: this file's 2026-09-18 annotation is unchanged since the prior review and remains a self-contained write-up of bounds/drop/redaction semantics rather than a short cross-referencing note, at the edge of what docs/contributing/adrs.md characterizes as a "minor annotation" on an Accepted ADR (Decision/Consequences text itself is untouched, so this is not a rule violation). This PR adds a matching dated annotation to ADR 0050 describing the new retry-prompt capture — continuing the same pattern across a second Accepted ADR rather than consolidating into a single successor document.
  Remediation: Not required to unblock this PR. For future PRs in this series, consider a short successor ADR that both 0050 and 0108 link to for recurring bound/semantics updates, rather than continuing to accumulate parallel decision-level prose directly in annotations on multiple Accepted ADRs.

---

Next steps:

• /fs-fix — agent addresses review findings automatically
• /fs-fix <your instruction> — agent fixes with your specific guidance
• Push commits directly — review re-runs automatically on push
• /fs-fix-stop — disable automatic fix runs for this PR

Previous run (19)

Review

Findings

Medium

• [data-exposure] internal/cli/content_collector.go:179 — attachInput redacts gen_ai.input.messages through OutputPipeline (NFKC normalization, then SecretRedactor patterns) but does not re-apply redactFeedback's sensitiveEnvKey literal ReplaceAll. redactFeedback runs only on the pre-sanitize feedback; sanitizeFeedbackUnicode returns the original bytes when the only unicode finding is the compatibility/fullwidth class; OutputPipeline then folds that copy for telemetry. An opaque runner-env credential (PUSH_TOKEN / *_SECRET / ... with no prefix or structural shape) that appeared only in compatibility characters can therefore be reconstructed as ASCII in gen_ai.input.messages even though the agent-facing prompt still held the unfolded form. Prefix-shaped tokens (ghp_, github_pat_, glpat-, ...) are still caught after fold. run.go:2322 passes only agentPrompt to attachInput — no runnerEnv. This is not covered by RegisterRuntimeSecret, which is populated from the OpenAI WIF path in this tree, not from runner env.
  Remediation: Thread runnerEnv into attachInput from the runAgent call site and, after c.redact, apply the same sensitiveEnvKey / minRedactableSecretLen ReplaceAll used by redactFeedback so NFKC-canonicalized copies cannot reconstruct an env secret the earlier literal pass missed. Keep the existing OutputPipeline scan so fullwidth PATs still match.

Low

• [architectural-fit] docs/ADRs/0108-tool-call-span-topology.md:128 — docs/contributing/adrs.md characterizes post-acceptance annotations on Accepted ADRs as minor (cross-references, short notes linking to newer decisions, typo/link fixes). The 2026-09-18 annotation appended to ADR 0108 is a self-contained write-up of bounds, drop-vs-cut semantics, and redaction/re-encoding behavior rather than a short note pointing at a separate new ADR. Decision/Consequences text is untouched, so this does not violate the letter of the immutability rule, but it sits at the edge of what "minor annotation" was meant to cover.
  Remediation: Consider whether a bound-defining behavior change like this (new drop-thresholds, new attribute semantics) warrants a short new ADR that ADR 0108 links to, matching the pattern already used for ADR 0087 and ADR 0108 itself, rather than accumulating decision detail directly in annotations.

• [consumer-contract-clarity] docs/guides/infrastructure/distributed-tracing.md:145 — fullsend.truncated now carries two materially different meanings on related part types: on tool_call_response parts it means "tail-kept, content cut" (partial content survives); on tool_call parts it now means "arguments dropped whole" (id/name/summary survive, arguments is entirely absent). This is documented in prose, but there is no separate marker distinguishing partial vs. whole-drop truncation. A consumer that keys off the boolean alone — without also checking part type and presence/absence of arguments — can mishandle the tool_call case.
  Remediation: A distinct marker is optional — part type plus presence/absence of arguments already distinguishes the two cases — but keep the eval-measure/consumer-facing schema text explicit that tool_call truncation is whole-drop, so integrators built against the prior tool_result-only truncation semantics don't generalize from the boolean alone.

---

Next steps:

• /fs-fix — agent addresses review findings automatically
• /fs-fix <your instruction> — agent fixes with your specific guidance
• Push commits directly — review re-runs automatically on push
• /fs-fix-stop — disable automatic fix runs for this PR

[fullsend-ai-review]

🤖 Finished Review · ✅ Success · Started 6:58 PM UTC · Completed 7:21 PM UTC

Commit: 3d19a5f · View workflow run →

Runtime: pi · Model: sonnet → claude-sonnet-5 · Effort: high · Cost: $10.30

[fullsend-ai-review]

🤖 Finished Review · ✅ Success · Started 9:10 PM UTC · Completed 9:31 PM UTC

Commit: 7944734 · View workflow run →

Runtime: pi · Model: sonnet → claude-sonnet-5 · Effort: high · Cost: $9.49

Superseded by updated review

[fullsend-ai-review]

🤖 Review · ⚠️ Cancelled · Started 2:19 PM UTC · Ended 2:36 PM UTC

Commit: 2b07a78 · View workflow run →

[dhshah13]

@waynesun09 one decision needed on the argument-redaction bar, so the review loop can end.

As of f713132 the collector judges each key, string and number on its own; values under a secret-named member by that name; and across strings only two things: a private-key block (the strings joined by a line break) and an exact runner-env or runtime value (the strings in order, white space aside). That is more than Sentry or the secret scanners do; none reconstruct pattern-shaped secrets across fields.

The review bot keeps flagging further cross-field reconstructions: an assignment, header or connection string whose value begins in the next string; a token split in pieces; a member-name pair a string's own quote closes. Proposal: those are documented limits (docs/guides/dev/tracing.md, "Not judged") plus one follow-up issue, not fixes in this PR, and the bot's "changes requested" on them is dismissed.

OK as the bar, or do you want any of them in? Each would be a small monotone addition.

[fullsend-ai-review]

🤖 Finished Review · ✅ Success · Started 2:38 PM UTC · Completed 2:57 PM UTC

Commit: f713132 · View workflow run →

Runtime: pi · Model: openai/gpt-6.1-sol → gpt-6.1-sol · Effort: high · Cost: $2.66

[fullsend-ai-review]

🤖 Finished Review · ✅ Success · Started 3:05 PM UTC · Completed 3:19 PM UTC

Commit: fa2f0b4 · View workflow run →

Runtime: pi · Model: openai/gpt-6.1-sol → gpt-6.1-sol · Effort: high · Cost: $1.83

[waynesun09]

Three findings from review of head fa2f0b4, posted inline.

[fullsend-ai-review]

🤖 Finished Review · ❌ Failure (post-script /home/runner/work/fullsend/fullsend/.fullsend/.fullsend-cache/resources/sha256/f05af62aba180f85f38250bc8336353830efb200682f9e3c56e42caca037fb2f/scripts/post-review.sh failed: exit status 1) · Started 4:25 PM UTC · Completed 4:43 PM UTC

Commit: bb50b21 · View workflow run →

Runtime: pi · Model: openai/gpt-6.1-sol → gpt-6.1-sol · Effort: high · Cost: $2.51

[dhshah13]

Update, since the mechanism above has changed: as of bb50b21 the collector is simpler and the question is the same. Each key, string and number is still judged on its own; a string holding a JSON document is decoded, walked and encoded again (one the normalizer changes at all is masked whole); and across strings it judges the same two things, a private-key block and an exact runner-env or runtime value, over the strings in written order by the standard decoder's token walk, with no match-offset bookkeeping. One behaviour change: a private key inside one string now drops the arguments rather than masking the block in place. The documented limits and the bot's two open findings are unchanged.

Decision asked for is unchanged: OK as the bar, or any of them in?

[dhshah13]

Superseded by the review's third finding: the allowlist contract landed in c8324df (the record holds the listed members of a call, each redacted as text, and nothing else of its input), and the two deferred findings are resolved as not applicable. No decision is pending here.

[fullsend-ai-review]

🤖 Finished Review · ✅ Success · Started 6:01 PM UTC · Completed 6:19 PM UTC

Commit: c8324df · View workflow run →

Runtime: pi · Model: openai/gpt-6.1-sol → gpt-6.1-sol · Effort: high · Cost: $3.78

[fullsend-ai-review]

🤖 Finished Review · ✅ Success · Started 6:57 PM UTC · Completed 7:13 PM UTC

Commit: cbfe80e · View workflow run →

Runtime: pi · Model: openai/gpt-6.1-sol → gpt-6.1-sol · Effort: high · Cost: $2.46