v0.5.0 - P1 capture-fidelity batch (#5, #6, #7)#14
Merged
Conversation
The P1 findings from the v0.4.0 audit. The buffer now records why (user intent), captures a wider slice of what a session does, and stamps compaction seams idempotently. A fifth hook joins the set. - #5: UserPromptSubmit -> session-prompt.sh captures redacted, truncated user prompts. Intent previously lived only in the compactable conversation. - #6: PostToolUse matcher widened to Grep/WebFetch/WebSearch/Task/Agent/mcp__.*; Read/Glob deliberately excluded. MCP tools captured name-only. - #7: precompact stamp made idempotent (tail -n 1 guard), mirroring flush. Shared refactor: redact/clean defs and the _tl_err breadcrumb moved into _lib.sh (tl_jq_redact_defs, tl_err, tl_append_line, clamp) so the two capture-side hooks can't drift. Command redact stays byte-identical. Pre-merge high-effort review fixes: - Prose-safe redact_prompt: the command-tuned redact corrupts/inverts English ("password is not the problem" -> "password is ***"). Prompts now use a restricted path (structural token formats + explicit key:value/key=value secrets only); command path unchanged. - Onboard no longer counts prompt-only buffers as unconsumed (a session with no captured action has nothing to distill), preserving the warning's signal. - Prompt clamped to 2000 chars before redaction (multi-MB paste latency). - Task empty-string description falls through to prompt (jq // only skips null). Tests: 95 -> 121 assertions. shellcheck + JSON manifest checks green. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01WqKPWvcm32Acg7UXJF2gQp
… gaps
A second high-effort review of the round-1 fixes found the fixes themselves
introduced new bugs:
- onboard's prompt-only-buffer skip used a substring search for "**prompt**"
anywhere in a line, so a real action line whose own content mentioned that
literal string (a grep for the pattern, routine in this repo) made the
whole buffer look prompt-only and silently dropped it from the unconsumed
warning. Fixed by anchoring to the type-marker position instead.
- redact_prompt's keyword rule matched substrings ("auth" inside "author",
"token" inside "tokens"), corrupting prose like "author: rewrite the intro"
in the exact path introduced to prevent that. Fixed with \b word boundaries.
- redact_prompt dropped the Bearer/Basic scheme rules entirely, leaking a
pasted Authorization header verbatim. Restored via a shared _auth_scheme
def used by both redact paths (shape-constrained, low prose false-positive
risk unlike the bare-token-word rule that stays excluded).
- WebSearch query and Task/Agent description are prose but were still routed
through the command-tuned redact, reproducing the same corruption issue #5
fixed for prompts ("fix token refresh bug" -> "fix Token *** bug"). Switched
both to redact_prompt; Grep (regex) and WebFetch (URL) correctly stay on
redact.
Tests: 121 -> 128 assertions, all fixes reproduced and verified before and
after. shellcheck + JSON manifest checks green.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01WqKPWvcm32Acg7UXJF2gQp
A third high-effort review of round 2's fixes found each one had introduced
a new bug of its own:
- redact_prompt's \b word-boundary (round 2's fix for substring matching)
silently stopped matching SCREAMING_SNAKE_CASE compounds (CLIENT_SECRET,
ACCESS_TOKEN, DB_PASSWORD) since underscore is a \w char. Real secrets in
the standard naming convention passed through completely unredacted. Fixed
with a letter-only lookaround boundary instead of \b: underscore still
counts as a boundary, an adjacent letter still doesn't.
- Onboard's prompt-only filter used a grep-into-grep pipe whose "found
nothing" and "found nothing because there's nothing to find" are
indistinguishable, so a buffer with zero conforming lines (corrupted, or a
capture hook that failed every call) silently fell through the same
continue as a genuine prompt-only buffer - regressing the guarantee that
any existing ended buffer gets surfaced. Fixed with an explicit
total-vs-prompt-count comparison.
- The shared _auth_scheme def (round 2's fix for the Bearer/Basic leak) was
unconstrained enough to corrupt ordinary prose ("basic authentication
support" -> "Basic *** support"), reopening the exact corruption class
redact_prompt exists to prevent. Split into a command variant (unchanged)
and a new _auth_scheme_prose with a 16+ char length floor.
Tests: 128 -> 134 assertions. Also stress-tested additional edge cases not
covered by tests (start-of-string keywords, "credentialing" prefix-match,
no-separator prose) to reduce the odds of a round 4. shellcheck + JSON
manifest checks green.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01WqKPWvcm32Acg7UXJF2gQp
A fourth review found four bugs, two in the same keyword-boundary code round
3 had just patched: the letter-lookaround fix excluded not just the intended
false-positive cases but EVERY keyword followed by any letter, so common real
phrasing ("secrets:", "passwords:", "credentials:") silently stopped being
redacted at all. Three consecutive rounds of "correct for the target case,
breaks a different one" on the same regex - the same shape as the v0.2.0
redaction rework documented earlier in this repo's history. Per that
precedent, stopped patching and did a structural redesign instead of a fifth
regex tweak (user confirmed: drop the generic rule).
- redact_prompt's generic keyword+separator rule removed entirely. There is
no boundary rule that admits "secrets:"/"passwords:" while excluding
"author:"/"tokens:" - the shapes overlap. redact_prompt now masks ONLY
unambiguous structural signals: known token prefixes, PEM, URL-userinfo,
and length-gated Bearer/Token/Basic scheme values. A colon-form secret with
no recognizable prefix/scheme is now a deliberate, CONSISTENT, documented
gap backstopped by the handoff skill's human re-scan - not fixed
differently case-by-case.
- Added the missing Token-scheme rule to _auth_scheme_prose (redact_prompt
never had one - an oversight from splitting it out of redact in round 2).
- The 16-char length floor un-redacting short real Bearer/Basic values is
accepted as the same documented gap, rather than tuning the threshold
further.
- Noted (not fixed, pre-existing, out of scope): a cosmetic quote-handling
bug in redact's generic rule (command path, unchanged) - an orphaned
trailing quote, no secret leak. No longer applies to redact_prompt since
that rule is gone.
Tests: 134 -> 133 (net change reflects removing tests for behavior that no
longer exists and consolidating documented-gap assertions). Stress-tested
broadly beyond the reviewer's cases (plurals, scheme words as ordinary nouns,
quote interaction) before resubmitting. shellcheck + JSON manifest green.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01WqKPWvcm32Acg7UXJF2gQp
A fifth review confirmed the round-4 structural redesign is clean (zero findings against redact_prompt - the three-round regex cycle is over) and found two smaller, unrelated bugs: - session-onboard.sh's prompt-only counters leaked a shell diagnostic to stderr on an unreadable buffer file. grep -c prints an empty string, not "0", when it cannot read the file at all, and the integer test was not guarded - the one error path in this hook not already 2>/dev/null'd. Fixed by defaulting both counts to 0 when empty. - The mcp__* fallback branch embedded the raw tool name directly inside the **...** bold-marker pair without stripping asterisks (every other branch only puts field content after a fixed literal marker). An unusual tool name containing * would break the markdown span and desync onboard's classifier regex, silently miscounting a real action as prompt-only. Fixed by stripping asterisks from the tool name specifically. - Bash branch's hand-rolled truncation migrated to the shared clamp def. Filed issue #16 for three deferred cleanup items (duplicated timestamp format, redundant onboard greps, duplicated auth-scheme regex literals) - none are correctness bugs, deferred to a calmer follow-up rather than more mechanical edits under today's time pressure. Tests: 133 -> 136 assertions. shellcheck + JSON manifest green. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01WqKPWvcm32Acg7UXJF2gQp
tests/run.sh redefined the prompt() test helper twice inline (once to inject THROUGHLINE_DISABLE=1, once back to the original) to test the kill-switch path. CI's shellcheck (apt-installed on ubuntu-latest) flagged every call to prompt() between the definitions as SC2218 "This function is only defined later" - a redefinition pattern local shellcheck did not flag, caught only once the GitHub Actions run itself failed. Replaced the redefine-and-restore pattern with a direct inline pipe for the one disabled-state test, so prompt() is defined exactly once. Verified by running apt-get install shellcheck (matching CI's install method) against the fix in a fresh Ubuntu container - clean. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01WqKPWvcm32Acg7UXJF2gQp
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The three P1 findings from the v0.4.0 audit (
docs/AUDIT-v0.4.0.md), shipped as one v0.5.0 batch. The buffer now records why (user intent), captures a much wider slice of what a session does, and stamps compaction seams idempotently. A fifth hook joins the set.Changes
UserPromptSubmithook (hooks/session-prompt.sh) writes a redacted, 200-char-truncated**prompt**line per user prompt. Intent previously lived only in the compactable conversation - exactly what a context compaction destroys.PostToolUsematcher gainsGrep|WebFetch|WebSearch|Task|Agent|mcp__.*. Each read-side tool emits one redacted one-liner; MCP tools are name-only (zero schema assumptions).Read/Globdeliberately excluded as too noisy. SubagentStop markers considered and deferred (see [P1] Widen PostToolUse capture beyond the 4 mutating tools #6).tail -n 1guard skips a duplicate boundary stamp for the same seam while still stamping each genuine compaction, mirroringsession-flush.sh.Shared refactor
The redaction/cleaning jq defs and the
_tl_errbreadcrumb moved intohooks/_lib.sh(tl_jq_redact_defs,tl_err,tl_append_line,clamp) so the capture-side hooks can't drift - same reasoning astl_resolve_sid. The command-pathredactstays byte-identical (proven by the unchanged pre-existing suite).Pre-merge review history (5 rounds)
This PR went through five rounds of high-effort automated review before merge. The short version: round 1 added a prose-safe
redact_prompt(separate from the command-tunedredact, which corrupts/inverts ordinary English) plus assorted fixes. Rounds 2 and 3 each tried to fix a keyword-boundary regex inredact_promptand each fix was correct for its target case but broke a different real case (substring matching -> word-boundary -> missedSCREAMING_SNAKE_CASEcompounds -> excluded common real phrasing like "secrets:" entirely). That's the same failure shape as a documented precedent in this repo's own history (the v0.2.0 redaction rework, see CHANGELOG). Per that precedent, round 4 stopped patching the regex and did a structural redesign instead:redact_prompt's generic keyword+separator rule was removed entirely. There is no boundary rule that admits "secrets:"/"passwords:" while excluding "author:"/"tokens:" - the shapes overlap.redact_promptnow masks ONLY unambiguous structural signals: known token prefixes, PEM, URL-userinfo, and length-gated Bearer/Token/Basic scheme values. A colon-form secret with no recognizable prefix/scheme is now a deliberate, consistent, documented gap - backstopped by the handoff skill's human re-scan, same class asredact's existing bare-CLI-flag gap - rather than being redacted inconsistently depending on suffix.redact_promptwas missing entirely (an oversight from splitting it out ofredact).Round 5 confirmed the redesign held - zero findings against
redact_promptitself - and found two smaller, unrelated bugs:session-onboard.shleaked a shell diagnostic to stderr on an unreadable buffer file (an unguarded integer test ongrep -c's empty-string output), and themcp__*fallback branch embedded a raw tool name directly inside the**...**bold-marker pair without stripping asterisks, which could desync onboard's classifier regex. Both fixed. Three lower-priority cleanup items (duplicated timestamp format string, redundant onboard greps, duplicated auth-scheme regex literals) were filed as issue #16 rather than fixed under continued time pressure.Full commit-by-commit detail:
1f2f7b2(round 1 + initial fixes),0737a0f(round 2),02ca0ed(round 3),6f2976c(round 4 redesign),bdaa367(round 5).Also filed: issue #15 (pre-existing, out-of-scope cosmetic quote-handling bug in
redact's command path).Deferred: concurrent-fire precompact non-atomicity (compaction isn't concurrent with itself;
flushhas the same pre-existing pattern).Verification
shellcheck -s sh hooks/*.sh tests/*.shcleansh tests/run.sh-> 136 passed / 0 failed (+41 new)Closes #5, closes #6, closes #7
🤖 Generated with Claude Code