fix(hooks): redact-secrets reads tool_response, the field CC sends. Fork PR by a first-time contributor, fixes your own #3725. Verdict: correct, minimal, honestly tested. Two maintainer chores at landing.
Verified against main, not the PR description: redact-secrets.ts:33 reads only the legacy aliases; secret-handler.ts:60 already reads tool_response first.
src/hooks/src/skill/redact-secrets.ts - const toolOutput = (input as any).tool_result || (input as any).output || ''; + const toolOutput = + (input as any).tool_response || (input as any).tool_result || (input as any).output || '';
Same precedence as posttool/secret-handler.ts. types.ts documents tool_response as the field CC actually sends (#3418, from the decompiled payload builder).
Ran on a worktree at the PR head, then copied its integration test into main's tree and ran it against main's built bundle.
| run | result | what it says |
|---|---|---|
npm run typecheck on PR head | clean | no type drift from the as any read |
redact-secrets.test.ts + new integration test, PR head | 70 / 70 | two new unit cases (reads tool_response; prefers it over tool_result) plus 3 integration cases |
same integration test against main's plugins/ork/hooks/dist/skill.mjs | 2 fail / 1 pass | the two detection cases fail, the clean-output case passes: the test measures the fix, not the mock |
The integration test drives the built bundle two ways: spawning bin/run-hook.mjs with a CC-shaped PostToolUse payload (asserts exit 0, no token in stdout, the ::warning:: on stderr) and importing dist/skill.mjs in-process. It skips itself when the bundle is absent.
ork has three redaction layers. This one read a field production never fills, so everything passed straight through it while its unit suite fed it the field it wanted.
Read before any of its CI was allowed to run. Additions were also grepped for fetch, URLs, curl, secret-bearing env reads and shell spawns.
| file | kind | note |
|---|---|---|
src/hooks/src/skill/redact-secrets.ts | source | the one-line read order, with a comment naming #3725 |
src/hooks/src/__tests__/skill/redact-secrets.test.ts | test | +2 cases, same fixture style as the file |
src/hooks/src/__tests__/integration/redact-secrets-tool-response.test.ts | test | new; only spawn is node bin/run-hook.mjs (ours); scratch dir under tmp; existing integration/ dir is collected by vitest's src/__tests__/** include |
src/hooks/dist/*, plugins/ork/hooks/dist/* | generated | 17-byte bundle delta, exactly the new read; bundle-stats follow |
Contributor: first PR in this repo. Cross-repository fork, maintainerCanModify is on. No workflow files touched. Risky-string scan of additions: only the docstring mentioning the runner.
run-hook.mjs does import(<absolute dist path>); on win32 that throws ERR_UNSUPPORTED_ESM_URL_SCHEME and the surrounding catch turns it into silent success before stdin is read.
| claim | status | why it matters |
|---|---|---|
win32 absolute path in import() is rejected by Node | plausible, unverified here | Node requires file:// URLs on Windows; a bare C:\... path is exactly the documented failure |
the runner's catch maps it to {"continue":true} | matches the runner's shape | the same fail-open the verdict probes exist to catch; the Windows Smoke Test would not see it if it never asserts a verdict |
Kept out of scope by the contributor, correctly. It needs its own issue with a probe that asserts a verdict on Windows, not just exit 0.
Content is approved. The repo's own conventions (registry changelog entry, Lab playground) are maintainer chores a stranger should not be taxed for.
after you pick:
A stranger found a dead sieve you had already named, fixed it with the smallest possible change, and proved it with a test that fails on main. The only open question is who adds the two lines of house paperwork.