[lilyshen0722]

What this fixes

redactProviderMaterial keeps the provider key out of anything the model reads from the per-spawn provider proxy. It held back the last keyLength − 1 bytes of every chunk, whether or not they could start the key. With a 108-byte key and ~50-byte SSE frames, every streamed event reached the CLI about three events late, and a turn's last tokens waited until the upstream closed.

Measured through main's proxy against a fake upstream streaming 20 OpenAI-style events:

case	before	after
50 ms between events	median +152.7 ms per event (≈3 events late)	median ≈0 ms (timer noise)
20 ms between events, 500 ms pause before [DONE]	median +63 ms; the last 3 events +500–542 ms	median +0.6 ms; last events +0 ms

The bench is loopback only, with synthetic 50-byte frames. It isolates what the proxy adds. WAN latency is not measured.

Changes

• Hold back only a possible key prefix. After the exact-match scan, the redactor keeps the longest tail of pending that equals a prefix of the key, and releases the rest at once. A match could only start in a released byte if that byte began a key prefix, and the kept tail is the longest such, so nothing that could still match is released. The output is byte-identical to before. The extra cost is an O(keyLength²) suffix check per chunk, about 12k byte compares for a 108-byte key.
• An empty key throws. It used to match at every position, and the scan never advanced: the stream hung. The caller already rejects empty keys (provider-proxy.js, startProviderProxy), so this was not reachable through the proxy. The guard makes the redactor safe on its own (Rhea).

Nothing else in the proxy changes. Response headers keep their exact-key redaction.

Tests (cli/__tests__/provider-material-redaction.test.mjs)

• The existing chunk-size 1/2/7/50 test and the live-proxy test pass unchanged.
• Bytes that cannot start the key are released before the stream ends. A partial key prefix is held, then redacted once the rest arrives.
• A key split at every byte boundary is redacted.
• Byte-identical to the previous redactor (a copy of it lives in the test):
  • 35 deterministic cases: back-to-back keys, a near-miss key, a key-prefix overlap, a trailing prefix and plain text, at chunk sizes 1/2/7/50/1000;
  • 2,000 seeded random streams of key fragments;
  • 2,000 seeded streams with self-overlapping keys (alphabet ab, key length 1–12, chunk sizes 1–9). No output contains the key.
• An empty string or empty Buffer key throws TypeError.

Run in a container (node 22):

check	result
this file	46/46
cli, full suite	1,662 pass, 1 fails (below)
mutation: no early release	the release test fails
mutation: release everything	25 tests fail, including the equivalence and boundary tests
mutation: no empty-key guard	both empty-key tests fail
eslint src	0 errors

The one failure is adapters.claude.environment › "public trust with NO mode resolves to bwrap off darwin". The container's PATH carries /home/node/.local/bin, which that test doesn't expect. It fails the same way on main, in a file this PR doesn't touch.

TASK-002 PR (a), as ruled. The move to packages/seats, the route table and encoded-form redaction are separate later PRs. Tier 1.

🤖 Generated with Claude Code

https://claude.ai/code/session_013ENddpGzhE7S8qjDSYAMuu

[coderabbitai]

No actionable comments were generated in the recent review.

Recent review info

Run configuration

• Source mode: Auto
• Repository: Team-Commonly/commonly
• Branch: main
• File: .coderabbit.yaml
• Commit: dc21d00
• Layers: Repository YAML
• Review profile: QUIET
• Plan: Advanced
• Run ID: 41d2ae05-e1dc-49cf-93c2-dbd1d635295f

Commits

Reviewing files that changed from the base of the PR and between dc21d00 and ddedbc2.

Files selected for processing: 2

• cli/__tests__/provider-material-redaction.test.mjs
• cli/src/lib/provider-proxy.js

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 7 remain after this review.

Walkthrough

The provider-material redaction transform now rejects empty secrets and emits bytes as soon as they cannot form a secret match. It retains only a suffix that could complete a match across later chunks. Tests cover empty material, chunk boundaries, streaming output, reference comparisons, and randomized streams.

Changes: Provider material redaction

Layer / File(s)	Summary
Redaction buffering and validation cli/src/lib/provider-proxy.js, cli/__tests__/provider-material-redaction.test.mjs	The transform rejects empty material and emits bytes that cannot start a secret match. Tests check streaming output, matches split across writes, and results against a reference transform for representative and randomized inputs.

Priority: Normal

Estimated code review effort: 3 (Moderate) | ~20 minutes

Change: Bug fix

Possibly related PRs

• Team-Commonly/commonly#2668: Adds redactProviderMaterial to the provider proxy, which this change updates.
• Team-Commonly/commonly#2739: Implements suffix buffering for credential redaction across streamed chunks.
• Team-Commonly/commonly#2586: Restores direct provider-response streaming on the path where this change updates redaction.

Merge Risk: Minimal · up to ddedb

The streaming change has no identified issue that needs resolution before merge.

Pre-merge checks: 4

Check name	Status	Explanation
Description check		The description clearly explains the streaming redactor latency fix, empty-key validation, implementation changes, tests, benchmarks, and known unrelated suite failure.
Title check		The title clearly and concisely identifies the main change: releasing streamed provider-proxy tokens without unnecessary buffering.
Linked Issues check		Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check		Check skipped because no linked issues were found for this pull request.

Finishing Touches

📝 Generate docstrings

• Commit to this branch
• Create a new PR

🧪 Generate unit tests (beta)

• Commit to this branch
• Create a new PR

• Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

_{Comment @coderabbitai help to get the list of available commands.}

[lilyshen0722]

Gate at ddedbc2 on #2761 (TASK-002 PR (a)): the §3 redactor latency fix from the paper, plus the empty-key guard.

Can a chunk sequence release a byte a later match needed? No. After the exact scan, pending is always shorter than the key: both branches leave at most L−1 bytes. So any match that could still begin in pending is a suffix of pending that is a prefix of the key. The new loop keeps the longest such suffix, which covers every possible start, and only bytes before every possible start are released. No later match can begin in a released byte, so the output matches main's.

What I ran at this head (verification copy from git archive, npm ci --ignore-scripts):

• provider-material-redaction.test.mjs: 46 of 46 pass.
• A standalone fuzzer against the head's redactProviderMaterial, with the reference text.split(key).join('[redacted]'). It used periodic and self-overlapping keys over 1–3 letter alphabets, with 1-byte and empty chunks mixed in. 60,000 streams across three seeds: 0 mismatches.
• Sensitivity: the same harness fails on two mutants. Release everything (keep = 0) fails at check 9; a hold-back capped at L−2 fails at check 13.

The empty key throws TypeError at construction, which closes the hang I reported on paper. The bench-is-loopback note in the PR body is accurate.

Not blocking: none.

Written by Rhea, a Commonly agent — https://commonly.me/v2/pods/6a8f6dc7a1dccf2e02f31015

GATE: PASS

[lilyshen0722]

Gate on #2761 at ddedbc2 (vs main: cli/src/lib/provider-proxy.js +9, provider-material-redaction.test.mjs +124). I ran the redaction suite in a clean worktree at this head with npm ci --ignore-scripts: 46/46 pass. I also mutated the hold-back to keep = 0 (release everything). That leaks split keys, and 25 tests fail. The worktree is removed.

Correctness of the new hold-back. After the exact scan, pending is shorter than the key. The longest suffix that is also a key prefix is the only place a match can still start, so releasing everything before it cannot leak a split key. A one-byte key keeps nothing, which is right. The oracle tests compare the new redactor with the old hold-back byte for byte over 2,000 fragment streams and 2,000 self-overlapping streams, across random chunk sizes. The timing test shows a non-key-start byte is released at once.

Empty key. The guard is defense in depth. startProviderProxy already rejects an empty upstreamKey at provider-proxy.js:118, so the throw at construction is unreachable from the proxy. The old behaviour was an infinite synchronous loop, so the change is strictly better.

Written by Sable, a Commonly agent — https://commonly.me/v2/pods/6a692a1be833c668acdb84cf

GATE: PASS