docs(privacy): say which captured calls keep nothing, and what Write and MultiEdit copy (#737, slice 5) - #976

Claude finished @cdeust's task in 2m 26s —— View job

Review of PR #976 (review 1 of 3)

• Read the diff and checked the new PRIVACY.md sentences against mcp_server/hooks/post_tool_capture.py
• Run the tests and lint: not run, because pytest and ruff needed an approval this runner doesn't grant (CI will report them)
• Ran five specialist reviews (quality, performance, tests, docs, security)
• Posted the review: docs(privacy): say which captured calls keep nothing, and what Write and MultiEdit copy (#737, slice 5) #976 (comment)

1 Blocker: PRIVACY.md:22-25 says a call whose response is under 50 characters is not captured at all. The hook measures the formatted output (_normalize_output), not what the tool returned:

• A Bash command that prints 30 characters reaches 50 once the formatting is added, so it is captured with its command line.
• A one-character Edit is about 64 characters once formatted, so it is captured with its path.

The new text therefore understates what is stored. The new test sends the plain string "ok", so it does not catch this.

3 Notes, no action before merge:

• NotebookEdit is also stored as its whole JSON, but the text names only Write and MultiEdit.
• The gist fallback when an artifact write fails is not mentioned; that text predates this PR.
• The MultiEdit test sends a response shaped like a Write response.

Verdict: block

Review 1 of 3: PR #976

Blocker

1. PRIVACY.md:22-25: the new sentence says a short call is not captured, but small calls are captured. It reads: "A call whose response is shorter is not captured at all, its path or command line included." The CHANGELOG entry (CHANGELOG.md:159-160) says the same. The 50-character check (_should_capture, mcp_server/hooks/post_tool_capture.py:70) is applied to the output of _normalize_output, which already includes the hook's own formatting, not to what the tool returned. I traced two cases through the code:

   • Bash: {"stdout": "x"*30} becomes **stdout:**\n\``\n…\n````, which is 20 characters of formatting plus 30 = 50. The call is captured with its command line, even though the command printed only 30 characters.
   • Edit: {"filePath": "/p/a.py", "oldString": "x", "newString": "y"} becomes **file:** plus **before:** and **after:** blocks, about 64 characters. A one-character edit is captured, path included. In practice the threshold almost never skips an Edit, Write or MultiEdit; their responses as JSON are longer still.

   A user reading the new sentence would think short outputs and small edits leave nothing behind. That understates what is stored, which is the error Auto-capture copies third-party personal data from any project, with no per-project opt-out, and can leave orphan artifacts #737 is meant to remove. The new test (test_capture_privacy_disclosure.py:126-129) sends a plain string "ok", so it does not see the formatting. Fix: say that the 50 characters count the formatted output (stdout with its labels, or the Edit path with before/after), and that in practice it only drops Bash calls that print almost nothing. Then add a test with {"stdout": "x" * 30} that expects a capture, and one with a one-character Edit.

Notes, no action before merge

• NotebookEdit responses also go through json.dumps whole (no oldString/newString), but PRIVACY.md:26-28 names only Write and MultiEdit.
• If writing the artifact fails, _gist_or_full (post_tool_capture.py:164-166) keeps the full response in the row, so "stored as a short gist" (PRIVACY.md:38-39) doesn't hold in that case. This text is not new in this PR.
• The MultiEdit case at test_capture_privacy_disclosure.py:148-153 sends a Write-shaped response (content), not a MultiEdit one (edits).

Everything else checks out against the code in every CORTEX_CAPTURE_MODE: Edit keeps only before/after, originalFile is dropped; Write and MultiEdit keep the whole JSON; any tool outside the three lists is never captured. I couldn't run pytest or ruff here because the commands needed an approval this runner doesn't grant; CI will report them.

View job run
Verdict: block

Claude finished @cdeust's task in 2m 30s —— View job

Review of PR #976 (review 2 of 3)

• Checked the review 1 Blocker: fixed in fa41cf5
• Reviewed only fa41cf5, against _normalize_output, _should_capture and process_event
• Ran five specialist reviews (docs, tests, quality, security, performance)
• Run pytest and ruff: not run, because the commands needed an approval this runner doesn't grant (CI will report them)
• Posted the review: docs(privacy): say which captured calls keep nothing, and what Write and MultiEdit copy (#737, slice 5) #976 (comment)

1 Blocker: PRIVACY.md:27-29 (and the CHANGELOG entry) says a Bash call that prints fewer than 30 characters on a single stream is dropped. That is false in two cases, and in both the call is captured with its command line:

• Both streams print. stdout "ok" plus stderr "warn: x" comes to 51 characters once formatted.
• The call is interrupted. The **interrupted:** True marker adds 21 characters, so 8 characters of stdout is enough.

2 Notes, no action before merge:

• The test hardcodes "30" in its assert.
• The exact 30-character boundary is tested on stdout only.

Verdict: block

Review 2 of 3: PR #976

The review 1 Blocker is fixed in fa41cf5. The 50-character check is now described as counting the formatted output, and the one-character Edit and the 29/30-character Bash cases are tested. I checked them against _normalize_output (mcp_server/hooks/post_tool_capture.py:209-261): a single stream gives 20 + n ≥ 50, and an empty call gives _(no output)_, which is 13 characters. The NotebookEdit wording and the shape of the MultiEdit test from review 1 are also fixed. This review covers fa41cf5 only.

Blocker

1. PRIVACY.md:27-29 (and CHANGELOG.md:160-162): "A Bash call is dropped, command line included, when it prints … fewer than 30 characters on a single stream" is false in two cases, so the text still understates what is stored. _normalize_output adds up everything it formats before the 50-character check (post_tool_capture.py:232-241):

   • Both streams print. stdout "ok" makes 22 characters and stderr "warn: x" makes 27. With the \n\n separator the total is 51, so the call is captured with its command line, even though neither stream reached 30. Commands that write to stderr, such as git, often do this.
   • The call is interrupted. **interrupted:** True adds 21 characters. {"stdout": "12345678", "interrupted": true} makes 28 + 2 + 21 = 51, so it is captured.

   Fix: say the call is kept once its stdout, stderr and interrupted marker, as formatted, reach 50 characters. Then give "about 30 characters when only stdout or only stderr prints and the call is not interrupted" as the example, not the rule. In the test, add a case with both streams ({"stdout": "ok", "stderr": "warn: x"} is captured) and an interrupted case.

Notes, no action before merge

• test_capture_privacy_disclosure.py:127 hardcodes "30" in the assert, so _number_before only adds a check on the format.
• The exact 30-character boundary is tested on stdout only, so a different stderr label would go unnoticed.

I read the code and traced the tests by hand. Running pytest and ruff needed an approval this runner doesn't grant, so CI will report them. Five specialist reviews ran: docs, tests, quality, security and performance. The quality and performance reviews found nothing that would fail CI.

View job run
Verdict: block

Claude finished @cdeust's task in 2m 18s —— View job

Review of PR #976 (review 3 of 3)

• Checked the review 2 Blocker against 8ae887c: fixed. I traced it through _normalize_output.
• Reviewed only 8ae887c
• Ran five specialist reviews (docs, tests, quality, security, performance)
• Run pytest and ruff: not run, because the commands needed an approval this runner doesn't grant (CI will report them)
• Posted the review: docs(privacy): say which captured calls keep nothing, and what Write and MultiEdit copy (#737, slice 5) #976 (comment)

Blockers: none. The new Bash wording in PRIVACY.md matches the hook in every case I traced, and no case stores more than the text says. Every assertion in the new tests holds when I trace it by hand.

3 Notes, no action before merge:

• The CHANGELOG parenthetical "(one stream alone: 30 characters)" doesn't say the call must not be interrupted, or that trailing whitespace is removed first.
• That CHANGELOG line is too long and should be rewrapped.
• The two-streams test has no case where the call is dropped.

Accepted: the whole PR (b9058b0, fa41cf5, 8ae887c).

Verdict: approve

Review 3 of 3: PR #976

The review 2 Blocker is fixed in 8ae887c. PRIVACY.md:27-32 no longer says a short Bash call is dropped. It now says a Bash call is kept once its stdout, stderr and interrupted marker, as formatted, add up to 50 characters. I checked each claim against _normalize_output and _should_capture (mcp_server/hooks/post_tool_capture.py:62-83, 209-261):

• Prints nothing: the call is always dropped. _(no output)_ is 13 characters, and an interrupted marker with no output is 21.
• One stream, not interrupted: the call is dropped below 30 characters, because 20 characters of formatting are added to the output.
• Two streams or the marker: these reach 50 with less output. For example, "ok" plus "warn: x" comes to 51, and 8 characters plus the marker comes to 51.

The new wording doesn't understate what is stored in any case. I traced the new tests by hand: every assertion holds and none passes vacuously, and the two cases from review 2 are now covered. This review covers 8ae887c only.

Blockers: none.

Notes, no action before merge

• CHANGELOG.md:162 "(one stream alone: 30 characters)" doesn't say the call must not be interrupted, or that trailing whitespace is stripped before counting. PRIVACY.md says this precisely.
• CHANGELOG.md:162 is longer than the lines around it; it should be rewrapped.
• The two-streams test has no negative case, for example two streams adding up to 49 that is dropped.

Accepted: the whole PR (b9058b0, fa41cf5, 8ae887c). No file or commit carries an open Blocker.

Five specialist reviews ran: docs, tests, quality, security and performance. The quality review found nothing that would fail ruff or the craftsmanship gate, and the performance review found nothing. I couldn't run pytest or ruff here, because the commands needed an approval this runner doesn't grant, so CI will report them.

View job run
Verdict: approve