fix(pi): make scanner output redaction linear

[yashrajp22]

Unanchored credential-name prefixes made redaction quadratic on long scanner output. Start matching at word boundaries so each word is scanned once, preserving full secret redaction before display truncation.

Validation: 23 Node checks, including 100,000-character stdout and stderr streams and secrets crossing the display boundary.

[rng1995]

[SkillSpector Review]

Hi @yashrajp22, thank you for tracking down the quadratic credential redaction in the pi extension and keeping redaction ahead of display truncation!

Value and readiness: The code change is correct and closes the weakness. Anchoring the credential-name prefix at \b makes redaction linear, and merging the API_KEY and TOKEN passes into one alternation does not change any output I could find. One required change remains: the new regression test cannot fail if the quadratic regex comes back, so the fix is not protected in CI. With an effective timing check this is ready for maintainer review.

Material findings

1. [Blocker] tests/unit/test_pi_extension.mjs:77: the { timeout: 5000 } option does not guard the fix. redactSecrets runs synchronously inside ctx.scan(), and node:test cannot interrupt synchronous work; the test is reported as passed once that work finishes. I reproduced this: a node:test case with { timeout: 500 } that awaits, then runs a 2 s synchronous step, passes. With the old regexes, 1,000,000 characters per stream would block for roughly 50 minutes (extrapolated from my timings below) and then pass. Please measure elapsed time explicitly, sized so the old path is clearly slow and the new one clearly fast. For example, use 100,000-200,000 characters per stream and assert performance.now() - started < 1000 around ctx.scan(). The old path takes about 30 s at 100,000 characters per stream; the new path takes well under 1 ms.

PIC tradeoffs: None identified.

Verification and gaps:

• I timed my own transcription of the old and new redactSecrets in Node on runs of A. Old: 5K/10K/20K/40K/80K characters took 40/146/605/2,423/9,677 ms (quadratic). New: 1M characters took 2.8 ms and 4M took 12 ms.
• I also timed the new regex on adversarial ~2M-character inputs: repeated API_KEY and TOKEN_, TOKEN= followed by 2M spaces, repeated TOKEN= , sk- runs, short sk-/sk-ant- tokens and dotted words. Every case finished in 6-23 ms.
• Differential check: I compared old and new output on 300,000 random strings built from API_KEY, TOKEN, lower-case variants, =, :, whitespace, sk-, NBSP, U+2028, the Kelvin sign and long s. There were 0 differences. The old leftmost match always started at a word-run boundary, so the \b anchor does not change which text is redacted.
• The OpenCode tool already anchors its equivalent rule at \b (.opencode/tools/skillspector_scan_lib.ts:62), so the two integrations now agree.
• The second new test (:89) is a useful ordering guard, but it also passes on main, because redaction already ran before truncation.
• CI: author commit 6fb4006 passed all checks. Head 8256dcd adds only bot merges of main, and the PR diff is unchanged (same patch-id). CI on the head is action_required. There are no conflicts with main, #740, #741 or #742. Tests were not executed locally per policy.

---

Decision: Changes Requested (reviewed head 8256dcdb96e1d8a019096617f956dbc99804593b)

[rng1995]

[SkillSpector Review]

Hi @yashrajp22, thank you for turning the runtime test into a real guard and for confirming that it fails without the fix!

Value and readiness: The extension code is unchanged since the last review, and that review already found it correct: anchoring the credential-name prefix at \b makes redaction linear without changing what gets redacted. The only blocker was the regression test. Commit 31fdf49e fixes it: the test now measures elapsed time around ctx.scan() and fails if the quadratic regex comes back. The changes merged from #747 and #496 are intact. Ready for final maintainer review.

Previous findings (review at 8256dcd. Since then: one author commit, 31fdf49e, which changes only the test, and five bot merges of main.)

• [Blocker] Regression test cannot fail: Resolved. tests/unit/test_pi_extension.mjs:78-91 now uses 100,000 characters per stream and asserts performance.now() - started < 1000 (:84-87).
  • Old code: in a run of A or B, the old regexes restart at every index and rescan to the end of the run, about n²/2 steps per pass. My earlier Node timings of the old function (9.7 s at 80,000 characters) put this input at about 15 s per stream. Your reply measured 10.8 s with only the \b removed. Either way the 1 s ceiling fails.
  • New code: \b holds only at index 0 of each run, so each run is scanned once. My earlier timing was 2.8 ms for 1,000,000 characters, which leaves wide headroom on slow runners.
  • Assertion timing: the elapsed check runs after the synchronous work finishes, so it fails even though { timeout: 5000 } still cannot interrupt that work. The leftover timeout is harmless.
  • Measured span: the module import happens in setup() before started, and no --output is passed. The timed span is therefore mostly redactSecrets plus truncation.

Material findings

1. [Non-blocking] The PR description still says the validation used "million-character stdout/stderr". The test now uses 100,000 characters per stream, so updating that line would keep the description accurate.

PIC tradeoffs: None identified.

Verification and gaps:

• Diff: the PR changes extensions/skillspector.ts (+2/-2, the regex only) and tests/unit/test_pi_extension.mjs (+32). The patch-id is the same at 31fdf49e and at the head, so the bot merges did not alter the PR's change.
• Merged main:
  • extensions/skillspector.ts:151 keeps the timeout from #747, (params.noLlm ?? true) ? 120000 : 630000. Its tests (:125-132) are present.
  • :19 keeps the gemini provider from #496.
  • git merge-tree against current main (f3ec825) is clean. No other open PR touches these files.
• Second new test: the test at :93-107 (secrets crossing the display boundary) is unchanged. As noted before, it also passes on main.
• CI: on 31fdf49e, "OpenCode TypeScript Tests" passed. That job runs node --test tests/unit/test_pi_extension.mjs. Lint and DCO also passed, and test-unit was still running. The head 5103abe is a bot merge with CI action_required.
• Not run: contributor tests, per policy. The old-versus-new behavior above comes from tracing the regexes and from my earlier timings of my own transcription.
• Review thread: you replied to my inline thread at :78 and resolved it. The fix holds.

---

Decision: Approved (reviewed head 5103abe83cda39da33f2a0debe9f4528538ad816)

[yashrajp22]

Thanks for catching that! I corrected the validation description to say 100,000 characters per stream, matching the current test. No code changed.