feat(sessions): opt-in sessions.captureText for response-body capture#153
Merged
Conversation
Deferred out of the consensus-step persistence work (PR #152). Captures per-opinion prompt/response BODY text in the session store ONLY behind a new default-off sessions.captureText flag - never in debug.jsonl (metrics-only invariant holds). Records the locked-in hard requirements: mandatory secret scrub as the primary control, PII-strip as best-effort defense-in-depth (not the gate), bounded session-revisit re-capture, and the plaintext-on-disk threat model. Implementation depends on PR #152 landing first.
Adds a uniform, default-OFF gate for persisting each provider's raw RESPONSE body (opinion.text) in the session store. Discovery: opinion.text was ALREADY persisted unconditionally for ask-all and the server-side consensus tool (scrubbed+capped in sanitizeRecord), but NOT for consensus-step (loop results carry no text). captureText now gates it uniformly at the single write chokepoint (persistRun): - default OFF drops opinion.text for ALL callers (ask-all, consensus, consensus- step, session-revisit) - only question + verdict/criticalIssues summaries are stored. This intentionally TIGHTENS the prior ask-all/consensus default (which always stored bodies) - body capture is now opt-in everywhere. - ON (and persist on) stores opinion.text, secret-scrubbed (MANDATORY) then best-effort PII-stripped (email + separator-phone only; not a guarantee), then capped. - consensus-step dispatch_peers retains the raw peer response on the in-memory loop result so it can be captured when ON; the wire response still omits text. - debug.jsonl is untouched - metrics-only, never body text, regardless of the flag. stripPII uses RFC-bounded quantifiers so it stays LINEAR on long attacker- influenced responses (an unbounded email regex was O(n^2): 968ms -> 12ms on 50k). config: sessions.captureText (boolean, default false) in the schema and resolveSessions validation (non-boolean -> false + warning). Tests: stripPII redaction + scrub-then-strip ordering + ReDoS-linearity (PII1-3); captureText config validation (SESS8); chokepoint gating for consensus-step (CS14/CS15) and ask-all (CS16); and no raw text on the dispatch_peers wire (CS17). Lifecycle note (docs): captureText is forward-gating - it does not retroactively strip records already on disk; toggling off stops new capture, it is not a purge.
antonbabenko
force-pushed
the
feat/sessions-capture-text-plan
branch
from
e199706 to
7303cc0
Compare
antonbabenko
changed the title
- stripPII: drop the phone-number heuristic, keep email-only (lower false-positive risk on legitimate content; PII1 test updated). Mandatory secret-scrub and the default-off captureText gate remain the real controls. - Remove docs/captureText-plan.md (planning scratch; superseded by the real docs). - Document consensus-step terminal persistence (#152) and sessions.captureText (#153) across README, TECHNICAL (Session persistence: config table, record shape, scrub/PII note, which tools persist), SETUP, CLAUDE, AGENTS, and config.default.json. Regenerated host artifacts (POWER.md, SKILL.md) via scripts/sync-hosts.js from the AGENTS.md source. - CLAUDE.md: add a MANDATORY "Documentation upkeep on feature completion" section - a feature is not done until README/TECHNICAL/SETUP/CLAUDE/AGENTS + config schema/default are updated in the same PR, generated artifacts regenerated via sync-hosts, and npm run check (host-artifacts + validate drift guards) passes.
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.
Part C, now implemented (was a draft plan; #152 merged, this rebased on top).
What
A uniform, default-OFF
sessions.captureTextflag gating persistence of each provider's raw response body (opinion.text) in the session store.Discovery that shaped it:
opinion.textwas already persisted unconditionally forask-all+ the server-sideconsensustool (scrubbed+capped), but not forconsensus-step. SocaptureTextis a uniform gate at the single write chokepoint (persistRun):opinion.textdropped for all callers (ask-all, consensus, consensus-step, session-revisit). Onlyquestion+ verdict/issue summaries persist. This intentionally tightens the prior ask-all/consensus default (which always stored bodies) - body capture is now opt-in everywhere.opinion.textstored, secret-scrubbed (mandatory) then best-effort PII-stripped, then capped.consensus-stepnow retains the raw peer response on its in-memory loop result so it can be captured when ON; the wire response still omits text.debug.jsonluntouched - metrics-only, never body text, regardless of the flag.Security
captureText;stripPII(email + separator-phone) is best-effort defense-in-depth, not the gate.stripPIIuses RFC-bounded quantifiers -> linear. An unbounded email regex was O(n^2) (968ms -> 12ms on a 50k pathological input); regression testPII3.Cross-model review
ask-all(security-analyst lens): grok LOW, codex MEDIUM, kimi HIGH, gemini timed out, glm thrashed. Acted on the real findings:PII3.CS17assertingdispatch_peersnever returns rawtext.persistRunis the only record-creatingwriteSessioncaller (annotateSessiononly re-writes existing records).Tests
574/574 pass. New:
PII1-3(redaction, scrub-then-strip ordering, ReDoS-linearity),SESS8(config validation),CS14/CS15(consensus-step gating),CS16(ask-all gating, shared chokepoint),CS17(no wire leak).Behavior change to flag for reviewers
ask-all/consensusrecords no longer storeopinion.textby default (only whencaptureTextis enabled). No functional consumer depends on it -session-revisitre-runs fromquestion,analyzeusesverdict.