diff --git a/specs/002-mock-call-real-crm/tasks.md b/specs/002-mock-call-real-crm/tasks.md index fe2bb4a..9c777bd 100644 --- a/specs/002-mock-call-real-crm/tasks.md +++ b/specs/002-mock-call-real-crm/tasks.md @@ -167,12 +167,12 @@ Single project — `src/opencloser/`, `tests/` at repository root (per plan.md ### Tests for User Story 6 -- [ ] T039 [P] [US6] Tests — US6: `[REDACTED]` replacement, summary-only retention writes no transcript file, no-op policy preserves the Slice 1 artifact contract, and a malformed redaction policy fails readiness, in `tests/unit/test_redaction.py` and `tests/integration/test_us6_redaction.py` (SC-009) +- [X] T039 [P] [US6] Tests — US6: `[REDACTED]` replacement, summary-only retention writes no transcript file, no-op policy preserves the Slice 1 artifact contract, and a malformed redaction policy fails readiness, in `tests/unit/test_redaction.py` and `tests/integration/test_us6_redaction.py` (SC-009) ### Implementation for User Story 6 -- [ ] T037 [P] [US6] Implement `src/opencloser/redaction/layer.py` — `RedactionLayer` with `RegexRedactionPolicy` (default `[REDACTED]`), `NoOpPolicy`, and summary-only retention (FR-028–FR-030; contracts/redaction-layer.md) -- [ ] T038 [US6] Route transcript text through `RedactionLayer` in `src/opencloser/artifacts/writer.py` before any transcript disk write, honoring summary-only retention and preserving the Slice 1 summary + transcript-pointer contract +- [X] T037 [P] [US6] Implement `src/opencloser/redaction/layer.py` — `RedactionLayer` with `RegexRedactionPolicy` (default `[REDACTED]`), `NoOpPolicy`, and summary-only retention (FR-028–FR-030; contracts/redaction-layer.md) +- [X] T038 [US6] Route transcript text through `RedactionLayer` in `src/opencloser/artifacts/writer.py` before any transcript disk write, honoring summary-only retention and preserving the Slice 1 summary + transcript-pointer contract **Checkpoint**: The transcript artifact path is privacy-hardened. diff --git a/src/opencloser/artifacts/writer.py b/src/opencloser/artifacts/writer.py index 266fd2e..4f2b18e 100644 --- a/src/opencloser/artifacts/writer.py +++ b/src/opencloser/artifacts/writer.py @@ -23,6 +23,7 @@ TaskPayload, WriteBack, ) +from opencloser.redaction.layer import RedactionLayer _TRANSCRIPT_FILENAME = "transcript.txt" _SESSION_RESULT_FILENAME = "session-result.json" @@ -31,6 +32,10 @@ _ELIGIBILITY_DECISION_FILENAME = "eligibility-decision.json" _CONFLICTING_EVENTS_FILENAME = "conflicting-events.json" +# Cached so repeated calls without an explicit layer don't re-compile the built-in +# redaction patterns on every session write. +_DEFAULT_REDACTION_LAYER = RedactionLayer.default() + @dataclass(frozen=True, slots=True) class ArtifactPaths: @@ -55,11 +60,44 @@ def write_session_artifacts( transcript_text: str | None = None, conflicting_events: list[ConflictingEventAuditRecord] | None = None, task: TaskPayload | None = None, + redaction_layer: RedactionLayer | None = None, ) -> ArtifactPaths: - """Write all per-session artifacts into ``//`` atomically.""" + """Write all per-session artifacts into ``//``. + + Each individual file is written atomically (tempfile + ``os.replace``); the set + of artifacts as a whole is not transactional. Atomicity is per-file, which is + what duplicate-event redelivery (FR-019) and idempotent re-emission need. + + Transcript text always passes through the configured ``RedactionLayer`` before + disk write (FR-028..FR-030). When the layer's retention mode is + ``"summary-only"`` — or no transcript text was supplied — no transcript file + is written; the exported ``transcript_pointer`` is cleared so the + session-result never advertises a file that does not exist. Under + ``"summary-only"`` retention, any pre-existing transcript file from an + earlier run is removed so idempotent re-emit cannot leave PII on disk. + """ session_dir = Path(artifact_root) / session_id session_dir.mkdir(parents=True, exist_ok=True) + # Decide retention BEFORE writing session-result, so the exported pointer never + # advertises a transcript file that the writer is about to skip + # (contracts/redaction-layer.md §Behavior; FR-030). + effective_layer = redaction_layer or _DEFAULT_REDACTION_LAYER + summary_only = ( + transcript_text is not None and effective_layer.retention_mode() == "summary-only" + ) + transcript_will_be_written = transcript_text is not None and not summary_only + if not transcript_will_be_written and normalized_result.transcript_pointer is not None: + normalized_result = normalized_result.model_copy(update={"transcript_pointer": None}) + + # FR-030 + FR-019: when no transcript will be written (summary-only retention OR no + # transcript_text supplied), remove any transcript file from an earlier run BEFORE + # writing session-result.json. If the unlink fails (locked file / permissions), we + # raise here without having advertised a null transcript_pointer that would leave + # the on-disk state both privacy-inconsistent and harder to detect. + if not transcript_will_be_written: + (session_dir / _TRANSCRIPT_FILENAME).unlink(missing_ok=True) + session_result_path = session_dir / _SESSION_RESULT_FILENAME _write_json_atomic(session_result_path, normalized_result) @@ -72,9 +110,9 @@ def write_session_artifacts( _write_json_atomic(task_path, task) transcript_path: Path | None = None - if transcript_text is not None: + if transcript_will_be_written: transcript_path = session_dir / _TRANSCRIPT_FILENAME - _write_text_atomic(transcript_path, transcript_text) + _write_text_atomic(transcript_path, effective_layer.redact(transcript_text)) eligibility_path = session_dir / _ELIGIBILITY_DECISION_FILENAME _write_json_atomic(eligibility_path, eligibility_decision) diff --git a/src/opencloser/core/orchestrator.py b/src/opencloser/core/orchestrator.py index d70b68b..7bad745 100644 --- a/src/opencloser/core/orchestrator.py +++ b/src/opencloser/core/orchestrator.py @@ -43,6 +43,7 @@ PersonaOutput, SessionContext, ) +from opencloser.redaction.layer import RedactionLayer from opencloser.state import store if TYPE_CHECKING: @@ -114,6 +115,7 @@ def process_one_queue_item( conversation_fixture: ConversationFixture | None, transport_fixture_id: str | None, clock: Clock | None = None, + redaction_layer: RedactionLayer | None = None, ) -> RunReport: """End-to-end Slice 1 orchestration for a single queue record. @@ -123,6 +125,13 @@ def process_one_queue_item( `conversation_fixture` is required when a call is expected to reach `connected`. `transport_fixture_id` is the name of the fixture under the transport's fixtures dir. + + `redaction_layer` is the configured transcript redaction layer + (``contracts/redaction-layer.md``; FR-028..FR-030). Slice 2 callers should + build one from ``Slice2Config.redaction`` via ``RedactionLayer.from_config`` + so operator-configured patterns / policy / retention take effect. Slice 1 + callers may omit it; the artifact writer falls back to a default-on layer + so PII never lands on disk unredacted by accident. """ clock = clock or SystemClock() start_ts_monotonic = time.monotonic() @@ -188,6 +197,7 @@ def process_one_queue_item( clock=clock, crm=crm, start_ts_monotonic=start_ts_monotonic, + redaction_layer=redaction_layer, ) return report @@ -206,6 +216,7 @@ def process_one_queue_item( conversation_fixture=conversation_fixture, transport_fixture_id=transport_fixture_id, start_ts_monotonic=start_ts_monotonic, + redaction_layer=redaction_layer, ) @@ -224,6 +235,7 @@ def _finalize_blocked( clock: Clock, crm: WriteBackAdapter, start_ts_monotonic: float, + redaction_layer: RedactionLayer | None = None, ) -> RunReport: """FR-005 block path: queue-status update only, no Phone Call activity, no task.""" new_status = queue_item.callable_status # FR-032: blocked → unchanged @@ -273,6 +285,7 @@ def _finalize_blocked( task=None, transcript_text=None, conflicting_events=None, + redaction_layer=redaction_layer, ) return RunReport( @@ -305,6 +318,7 @@ def _run_allowed_session( conversation_fixture: ConversationFixture | None, transport_fixture_id: str | None, start_ts_monotonic: float, + redaction_layer: RedactionLayer | None = None, ) -> RunReport: del eligibility # already used; kept for signature symmetry @@ -452,6 +466,7 @@ def _run_allowed_session( persona_output=persona_output, transcript_text=transcript_text, conflicting_events=conflicting_events, + redaction_layer=redaction_layer, ) return RunReport( session_id=session.session_id, @@ -497,6 +512,7 @@ def _finalize_session( persona_output: PersonaOutput | None, transcript_text: str | None, conflicting_events: list[ConflictingEventAuditRecord], + redaction_layer: RedactionLayer | None = None, ) -> tuple[NormalizedResult, ArtifactPaths]: """Finalize the session, emit write-backs per FR-031, write artifacts.""" ended_at = clock.now_utc_ms() @@ -511,6 +527,20 @@ def _finalize_session( ended_at=ended_at, ) + # Apply the redaction layer's retention decision to ``normalized.transcript_pointer`` + # BEFORE persisting to the DB. The artifact writer makes the same decision when it + # serializes session-result.json; doing it here too keeps the DB ``normalized_results`` + # row consistent with what is actually written to disk (no DB pointer to a file the + # writer is about to skip under summary-only retention or no-transcript-text paths). + no_transcript = transcript_text is None + summary_only = ( + not no_transcript + and redaction_layer is not None + and redaction_layer.retention_mode() == "summary-only" + ) + if (no_transcript or summary_only) and normalized.transcript_pointer is not None: + normalized = normalized.model_copy(update={"transcript_pointer": None}) + # Persist the disposition, normalized result, and queue-item lifecycle update. # The session's `state` is intentionally NOT flipped to FINALIZED here — that # flip is the LAST commit, after write-backs and artifacts succeed (N3), so a @@ -621,6 +651,7 @@ def _finalize_session( task=task, transcript_text=transcript_text, conflicting_events=conflicting_events or None, + redaction_layer=redaction_layer, ) # FINAL commit: flip the session to FINALIZED only now that every write-back row diff --git a/src/opencloser/models.py b/src/opencloser/models.py index 60d1c90..8227760 100644 --- a/src/opencloser/models.py +++ b/src/opencloser/models.py @@ -735,6 +735,29 @@ class TaskOwnersConfig(BaseModel): review: str +# Default ``[redaction] patterns`` for the regex policy: email + NA phone numbers. +# +# Lives on the config so the implicit and config-driven defaults cannot diverge — +# ``RegexRedactionPolicy`` compiles exactly the patterns it is given, no implicit +# prepending. Operators who supply a custom ``patterns`` list replace these +# defaults outright (set ``policy = "noop"`` to disable redaction entirely). +# +# Phone-number regex is privacy-conservative: separators are OPTIONAL so bare +# 10-digit forms (e.g. ``5551234567``) are also redacted (aligned with the +# default in ``config/slice2.toml``). This may over-match true 10-digit IDs in +# transcript text — operators with that concern should configure stricter +# patterns explicitly. +_BUILTIN_REDACTION_PATTERNS: tuple[str, ...] = ( + # Email addresses. + r"\b[A-Za-z0-9._%+-]+@[A-Za-z0-9.-]+\.[A-Za-z]{2,}\b", + # North American phone numbers. Matches all of: 555-123-4567, 5551234567, + # 555.123.4567, (555) 123-4567, +1 555 123 4567. + r"(?:\+?\d{1,2}[\s.-])?\(?\d{3}\)?[\s.-]?\d{3}[\s.-]?\d{4}", +) + +BUILTIN_REDACTION_PATTERNS: tuple[str, ...] = _BUILTIN_REDACTION_PATTERNS + + class RedactionPolicyConfig(BaseModel): """`[redaction]` section (FR-028, FR-029, FR-030).""" @@ -742,7 +765,9 @@ class RedactionPolicyConfig(BaseModel): policy: Literal["regex", "noop"] = "regex" retention: Literal["full", "summary-only"] = "full" - patterns: list[str] = Field(default_factory=list) + patterns: list[str] = Field( + default_factory=lambda: list(_BUILTIN_REDACTION_PATTERNS), + ) class Slice2Config(BaseModel): diff --git a/src/opencloser/redaction/layer.py b/src/opencloser/redaction/layer.py new file mode 100644 index 0000000..e75a094 --- /dev/null +++ b/src/opencloser/redaction/layer.py @@ -0,0 +1,101 @@ +"""Transcript redaction layer (FR-028, FR-029, FR-030). + +See ``specs/002-mock-call-real-crm/contracts/redaction-layer.md``. The layer is +default-on; disabling redaction requires an explicit ``policy = "noop"`` in +``config/slice2.toml [redaction]``. +""" + +from __future__ import annotations + +import re +from dataclasses import dataclass +from typing import Literal, Protocol + +from opencloser.models import RedactionPolicyConfig + +RetentionMode = Literal["full", "summary-only"] + +DEFAULT_REPLACEMENT = "[REDACTED]" + + +class Policy(Protocol): + def redact(self, transcript_text: str) -> str: ... + + +@dataclass(frozen=True, slots=True) +class NoOpPolicy: + """Returns transcript text unchanged (FR-029).""" + + def redact(self, transcript_text: str) -> str: + return transcript_text + + +@dataclass(frozen=True, slots=True) +class RegexRedactionPolicy: + """Replaces every match of the compiled patterns with ``[REDACTED]`` (FR-028). + + Compiles exactly the patterns it is given — no implicit built-ins. The + config-driven default set lives in + ``opencloser.models.BUILTIN_REDACTION_PATTERNS`` and is wired in via + ``RedactionPolicyConfig.patterns`` so config and code cannot drift. + """ + + _compiled: tuple[re.Pattern[str], ...] + replacement: str = DEFAULT_REPLACEMENT + + @classmethod + def from_patterns( + cls, + patterns: list[str] | tuple[str, ...] = (), + *, + replacement: str = DEFAULT_REPLACEMENT, + ) -> RegexRedactionPolicy: + compiled: list[re.Pattern[str]] = [] + for raw in patterns: + try: + compiled.append(re.compile(raw)) + except re.error as exc: + raise ValueError(f"Invalid redaction regex {raw!r}: {exc}") from exc + return cls(_compiled=tuple(compiled), replacement=replacement) + + def redact(self, transcript_text: str) -> str: + out = transcript_text + for pattern in self._compiled: + out = pattern.sub(self.replacement, out) + return out + + +@dataclass(frozen=True, slots=True) +class RedactionLayer: + """Transforms transcript text and decides whether a transcript file is written.""" + + policy: Policy + _retention_mode: RetentionMode + + def redact(self, transcript_text: str) -> str: + return self.policy.redact(transcript_text) + + def retention_mode(self) -> RetentionMode: + return self._retention_mode + + @classmethod + def from_config(cls, config: RedactionPolicyConfig) -> RedactionLayer: + """Build a layer from validated ``[redaction]`` config (FR-028..FR-030). + + Raises ``ValueError`` if any user pattern is not a valid regex — the + orchestrator surfaces this as a readiness failure. + """ + policy: Policy + if config.policy == "regex": + policy = RegexRedactionPolicy.from_patterns(config.patterns) + elif config.policy == "noop": + policy = NoOpPolicy() + else: # pragma: no cover - pydantic Literal guards this + raise ValueError(f"Unknown redaction policy: {config.policy!r}") + return cls(policy=policy, _retention_mode=config.retention) + + @classmethod + def default(cls) -> RedactionLayer: + """Default-on layer (FR-028): mirrors the unconfigured ``[redaction]`` section + so the implicit default and the config-driven default cannot drift apart.""" + return cls.from_config(RedactionPolicyConfig()) diff --git a/tests/fixtures/artifact_inputs.py b/tests/fixtures/artifact_inputs.py new file mode 100644 index 0000000..40414ba --- /dev/null +++ b/tests/fixtures/artifact_inputs.py @@ -0,0 +1,107 @@ +"""Shared builder for ``write_session_artifacts`` inputs. + +Used by the artifact-writer unit tests and the US6 redaction unit + integration +tests so that the fixture shape lives in one place. Keep this aligned with +``contracts/redaction-layer.md`` and the Slice 1 artifact contract. +""" + +from __future__ import annotations + +from opencloser.models import ( + CallableStatus, + Disposition, + ExportedEligibilityDecision, + NormalizedResult, + PhoneCallActivityPayload, + QueueStatusUpdatePayload, + TaskPayload, + WriteBack, +) + +_T = "2026-05-19T17:00:00.000Z" +_PERSONA_VERSION = "alf-appointment-setter@0.1.0" + +DEFAULT_TRANSCRIPT = "[persona] Hi, this is an AI assistant.\n[contact] Sure.\n" + + +def make_artifact_inputs( + session_id: str = "ses_1", + *, + transcript_text: str | None = DEFAULT_TRANSCRIPT, + summary: str = "Decision-maker confirmed interested; callback requested Thursday 14:00.", + captured_email: str | None = "ok@example.com", +) -> dict[str, object]: + """Build the keyword inputs accepted by ``write_session_artifacts``. + + Every payload is keyed off ``session_id`` so the directory name and the + JSON-payload ``session_id`` fields stay coherent. + """ + nr = NormalizedResult( + session_id=session_id, + queue_item_id="q1", + mock_provider_call_id="call_x", + persona_version=_PERSONA_VERSION, + final_disposition=Disposition.INTERESTED_CALLBACK_REQUESTED, + summary=summary, + # Mirror what an orchestrator would carry: when callers supply transcript + # text, advertise the conventional filename; when they don't, leave the + # pointer null. The writer still has the final say (e.g. summary-only + # retention will null the pointer it actually emits) — this just keeps + # the fixture from injecting a guaranteed-dangling pointer at the source. + transcript_pointer="transcript.txt" if transcript_text is not None else None, + callback_requested=True, + preferred_callback_window="Thursday 14:00", + captured_email=captured_email, + started_at=_T, + ended_at=_T, + ) + activity = PhoneCallActivityPayload( + session_id=session_id, + queue_item_id="q1", + mock_provider_call_id="call_x", + persona_version=_PERSONA_VERSION, + final_disposition=Disposition.INTERESTED_CALLBACK_REQUESTED, + summary=summary, + started_at=_T, + ended_at=_T, + ) + status = QueueStatusUpdatePayload( + session_id=session_id, + queue_item_id="q1", + previous_status=CallableStatus.READY, + new_status=CallableStatus.READY, + transition_reason="interested_callback_requested", + transition_at=_T, + ) + task = TaskPayload( + task_id="task_1", + session_id=session_id, + queue_item_id="q1", + task_kind="callback", + subject="Callback Thursday 14:00", + preferred_callback_window="Thursday 14:00", + captured_email=captured_email, + persona_version=_PERSONA_VERSION, + created_at=_T, + ) + writeback = WriteBack( + session_id=session_id, + phone_call_activity=activity, + queue_status_update=status, + task=task, + ) + eligibility = ExportedEligibilityDecision( + decision_id="dec_1", + queue_item_id="q1", + session_id=session_id, + decided_at=_T, + outcome="allow", + rules={"a": True, "b": True, "c": True, "d": True, "e": True, "f": True}, + ) + return { + "normalized_result": nr, + "writeback": writeback, + "eligibility_decision": eligibility, + "task": task, + "transcript_text": transcript_text, + } diff --git a/tests/integration/test_us6_redaction.py b/tests/integration/test_us6_redaction.py new file mode 100644 index 0000000..c944261 --- /dev/null +++ b/tests/integration/test_us6_redaction.py @@ -0,0 +1,310 @@ +"""US6 Story 6 — Transcript redaction layer (P3, SC-009). + +Independent Test (per spec §Story 6): + + Run a scripted conversation whose transcript contains a redaction-policy match; + confirm the written artifact stores ``[REDACTED]``. Re-run with summary-only + retention; confirm no transcript file is written while the session-result + summary remains. + +Also covers FR-029 (no-op policy preserves the Slice 1 artifact contract) and +the readiness-failure surface for a malformed redaction policy. +""" + +from __future__ import annotations + +import json +import sqlite3 +from datetime import UTC, datetime +from pathlib import Path + +import pytest + +from opencloser.artifacts.writer import write_session_artifacts +from opencloser.core.clock import FrozenClock +from opencloser.core.orchestrator import process_one_queue_item +from opencloser.crm.mock import MockWriteBackAdapter +from opencloser.eligibility.evaluator import BuiltinEligibilityEvaluator +from opencloser.models import ( + ArtifactsConfig, + CallWindowConfig, + EligibilityConfig, + PersonaConfig, + QueueItem, + RedactionPolicyConfig, + SliceConfig, + StateConfig, +) +from opencloser.persona.alf_appointment_setter import ALFAppointmentSetterPersona +from opencloser.persona.base import ConversationFixture, ConversationTurn +from opencloser.redaction.layer import DEFAULT_REPLACEMENT, RedactionLayer +from opencloser.state import store +from opencloser.transport.mock import FixtureDrivenTransport +from tests.fixtures.artifact_inputs import make_artifact_inputs + +pytestmark = pytest.mark.integration + +_REPO_ROOT = Path(__file__).resolve().parents[2] +_QUEUE_ITEM_FIXTURE = _REPO_ROOT / "tests/fixtures/queue_items/alf-prospect-001.json" +_TRANSPORT_FIXTURES = _REPO_ROOT / "tests/fixtures/transport_events" + +_SUMMARY = "Interested; callback requested for Thursday 14:00." +_TRANSCRIPT_WITH_PII = ( + "[persona] Could I confirm the best callback number?\n" + "[contact] Yes, it's 555-123-4567 and email alice@example.com.\n" + "[persona] Thank you — I'll have someone follow up.\n" +) + + +def _inputs(session_id: str) -> dict[str, object]: + return make_artifact_inputs(session_id, transcript_text=_TRANSCRIPT_WITH_PII, summary=_SUMMARY) + + +def test_us6_redacted_token_written_to_transcript(tmp_artifact_dir: Path) -> None: + """Spec §Story 6 acceptance: transcript file contains ``[REDACTED]`` and not the + original PII when the default regex policy is in effect.""" + session_id = "ses_us6" + layer = RedactionLayer.from_config(RedactionPolicyConfig()) # regex / full + paths = write_session_artifacts( + artifact_root=tmp_artifact_dir, + session_id=session_id, + redaction_layer=layer, + **_inputs(session_id), + ) + assert paths.transcript is not None and paths.transcript.exists() + content = paths.transcript.read_text(encoding="utf-8") + assert "555-123-4567" not in content + assert "alice@example.com" not in content + assert DEFAULT_REPLACEMENT in content + sr = json.loads(paths.session_result.read_text(encoding="utf-8")) + assert sr["summary"] == _SUMMARY + assert sr["transcript_pointer"] == "transcript.txt" + + +def test_us6_summary_only_retention_writes_no_transcript(tmp_artifact_dir: Path) -> None: + """Spec §Story 6 acceptance: summary-only retention writes NO transcript file; + the session-result summary remains and the transcript pointer is null so no + artifact reader can be led to a file that does not exist (FR-030).""" + session_id = "ses_us6_sum" + layer = RedactionLayer.from_config( + RedactionPolicyConfig(policy="regex", retention="summary-only") + ) + paths = write_session_artifacts( + artifact_root=tmp_artifact_dir, + session_id=session_id, + redaction_layer=layer, + **_inputs(session_id), + ) + assert paths.transcript is None + assert not (tmp_artifact_dir / session_id / "transcript.txt").exists() + sr = json.loads(paths.session_result.read_text(encoding="utf-8")) + assert sr["summary"] == _SUMMARY + assert sr["transcript_pointer"] is None + + +def test_us6_noop_policy_preserves_slice1_contract(tmp_artifact_dir: Path) -> None: + """FR-029: explicit no-op policy preserves the unredacted transcript contract.""" + session_id = "ses_us6_noop" + layer = RedactionLayer.from_config(RedactionPolicyConfig(policy="noop")) + paths = write_session_artifacts( + artifact_root=tmp_artifact_dir, + session_id=session_id, + redaction_layer=layer, + **_inputs(session_id), + ) + assert paths.transcript is not None and paths.transcript.exists() + content = paths.transcript.read_text(encoding="utf-8") + assert "555-123-4567" in content + assert "alice@example.com" in content + assert DEFAULT_REPLACEMENT not in content + + +def test_us6_summary_only_removes_stale_transcript(tmp_artifact_dir: Path) -> None: + """FR-030 + FR-019: re-emitting a session under summary-only retention must NOT + leave a transcript file from a prior full-retention run on disk.""" + session_id = "ses_us6_rerun" + full_layer = RedactionLayer.from_config(RedactionPolicyConfig()) # regex / full + write_session_artifacts( + artifact_root=tmp_artifact_dir, + session_id=session_id, + redaction_layer=full_layer, + **_inputs(session_id), + ) + stale = tmp_artifact_dir / session_id / "transcript.txt" + assert stale.exists() + + summary_only_layer = RedactionLayer.from_config( + RedactionPolicyConfig(policy="regex", retention="summary-only") + ) + paths = write_session_artifacts( + artifact_root=tmp_artifact_dir, + session_id=session_id, + redaction_layer=summary_only_layer, + **_inputs(session_id), + ) + assert paths.transcript is None + assert not stale.exists(), "Stale transcript must be removed under summary-only retention" + sr = json.loads(paths.session_result.read_text(encoding="utf-8")) + assert sr["transcript_pointer"] is None + + +def test_us6_no_transcript_text_nulls_pointer_and_removes_stale( + tmp_artifact_dir: Path, +) -> None: + """When the caller supplies no transcript text, ``transcript_pointer`` must be + null in session-result.json AND any transcript file from an earlier run must + be removed so the exported pointer stays consistent with what is on disk.""" + session_id = "ses_us6_none" + + # Prime the session dir with a stale transcript file (as if a prior run wrote one). + write_session_artifacts( + artifact_root=tmp_artifact_dir, + session_id=session_id, + **_inputs(session_id), + ) + stale = tmp_artifact_dir / session_id / "transcript.txt" + assert stale.exists() + + # Now re-emit with no transcript text — the writer must clean up. + paths = write_session_artifacts( + artifact_root=tmp_artifact_dir, + session_id=session_id, + # No redaction_layer here — exercising the cached default-on layer too. + **make_artifact_inputs(session_id, transcript_text=None, summary=_SUMMARY), + ) + assert paths.transcript is None + assert not stale.exists(), "Stale transcript must be removed when no transcript_text supplied" + sr = json.loads(paths.session_result.read_text(encoding="utf-8")) + assert sr["transcript_pointer"] is None + assert sr["summary"] == _SUMMARY + + +def test_us6_malformed_policy_fails_readiness() -> None: + """SC-009 / FR-028 readiness gate: a malformed regex in ``[redaction] patterns`` + fails layer construction — the orchestrator surfaces this as a readiness failure + before any session is run.""" + bad_cfg = RedactionPolicyConfig(policy="regex", patterns=[r"(?P SliceConfig: + return SliceConfig( + call_window=CallWindowConfig(start="09:00", end="20:00"), + eligibility=EligibilityConfig(max_attempts=5, default_timezone="America/Los_Angeles"), + artifacts=ArtifactsConfig(dir=str(artifact_dir)), + persona=PersonaConfig(version="alf-appointment-setter@0.1.0"), + state=StateConfig(db=str(db_path)), + ) + + +def _conversation_with_pii() -> ConversationFixture: + return ConversationFixture( + fixture_id="us6_pii_conversation", + expected_disposition="interested_callback_requested", + queue_item_ref="alf-prospect-001", + turns=[ + ConversationTurn( + role="persona", + text="Hi, this is an AI assistant from Medx. Is this a good time?", + ), + ConversationTurn( + role="contact", + text=( + "Sure, I'm the owner. You can reach me at 555-123-4567 " + "or owner@sunsetridge.example. Call me back Thursday at 2 PM please." + ), + ), + ConversationTurn( + role="persona", + text="Great — Thursday at 2 PM Pacific. Thanks for your time!", + ), + ], + expected_extraction={ + "callback_requested": True, + "preferred_callback_window": "Thursday at 2 PM", + "role_confidence": "confident_decision_maker", + "intent_classification": "interested", + }, + ) + + +def test_us6_orchestrator_passes_redaction_layer_to_writer( + tmp_state_db: sqlite3.Connection, tmp_artifact_dir: Path, tmp_path: Path +) -> None: + """End-to-end wiring proof: a ``RedactionLayer`` built from Slice 2 + ``[redaction]`` config and passed into ``process_one_queue_item`` reaches the + artifact writer, so operator-configured patterns / policy / retention actually + take effect in the production call path (addresses the layer-not-wired gap).""" + store.insert_queue_item( + tmp_state_db, + QueueItem.model_validate_json(_QUEUE_ITEM_FIXTURE.read_text(encoding="utf-8")), + ) + layer = RedactionLayer.from_config(RedactionPolicyConfig()) # default-on regex + + report = process_one_queue_item( + "alf-prospect-001", + conn=tmp_state_db, + config=_slice1_config(tmp_artifact_dir, tmp_path / "slice1.db"), + eligibility=BuiltinEligibilityEvaluator(), + transport=FixtureDrivenTransport(_TRANSPORT_FIXTURES), + persona=ALFAppointmentSetterPersona(), + crm=MockWriteBackAdapter(tmp_state_db), + conversation_fixture=_conversation_with_pii(), + transport_fixture_id="connected", + clock=FrozenClock(datetime(2026, 5, 19, 19, 0, 0, tzinfo=UTC)), + redaction_layer=layer, + ) + + transcript_path = report.artifact_dir / "transcript.txt" + assert transcript_path.exists() + content = transcript_path.read_text(encoding="utf-8") + assert "555-123-4567" not in content + assert "owner@sunsetridge.example" not in content + assert DEFAULT_REPLACEMENT in content + + +def test_us6_orchestrator_summary_only_retention_omits_transcript( + tmp_state_db: sqlite3.Connection, tmp_artifact_dir: Path, tmp_path: Path +) -> None: + """When the orchestrator receives a summary-only layer, the writer emits no + transcript file and session-result.json carries a null transcript_pointer.""" + store.insert_queue_item( + tmp_state_db, + QueueItem.model_validate_json(_QUEUE_ITEM_FIXTURE.read_text(encoding="utf-8")), + ) + layer = RedactionLayer.from_config( + RedactionPolicyConfig(policy="regex", retention="summary-only") + ) + + report = process_one_queue_item( + "alf-prospect-001", + conn=tmp_state_db, + config=_slice1_config(tmp_artifact_dir, tmp_path / "slice1.db"), + eligibility=BuiltinEligibilityEvaluator(), + transport=FixtureDrivenTransport(_TRANSPORT_FIXTURES), + persona=ALFAppointmentSetterPersona(), + crm=MockWriteBackAdapter(tmp_state_db), + conversation_fixture=_conversation_with_pii(), + transport_fixture_id="connected", + clock=FrozenClock(datetime(2026, 5, 19, 19, 0, 0, tzinfo=UTC)), + redaction_layer=layer, + ) + + assert not (report.artifact_dir / "transcript.txt").exists() + sr = json.loads((report.artifact_dir / "session-result.json").read_text(encoding="utf-8")) + assert sr["transcript_pointer"] is None + + # DB consistency: the persisted normalized_result row must also have null + # transcript_pointer so the DB does not advertise a file the writer skipped. + row = tmp_state_db.execute( + "SELECT transcript_pointer FROM normalized_results WHERE session_id = ?;", + (report.session_id,), + ).fetchone() + assert row is not None and row["transcript_pointer"] is None diff --git a/tests/test_imports.py b/tests/test_imports.py index e5202af..fd70d3d 100644 --- a/tests/test_imports.py +++ b/tests/test_imports.py @@ -3,13 +3,18 @@ Walks the AST of every `src/opencloser/**/*.py` and asserts the FR-033 boundary's dependency-allowed rules from `contracts/*.md` hold. Per boundary: -- ``core`` may import any boundary module + state + models + artifacts. +- ``core`` may import any boundary module + state + models + artifacts + + redaction (the orchestrator plumbs a configured ``RedactionLayer`` through + to the writer per ``contracts/redaction-layer.md``). - ``eligibility`` / ``transport`` / ``persona`` MAY import `models` and shared `core` primitives (`ids`, `clock`, `idempotency` per orchestrator contract), but MUST NOT import each other. - ``crm`` may import `models` and `state` only — `contracts/crm-writeback.md` explicitly forbids `core` for the write-back boundary. -- ``artifacts`` writer may import `models` only. +- ``artifacts`` writer may import `models` and `redaction` — per + `contracts/redaction-layer.md`, the writer calls the redaction layer + immediately before any transcript disk write (FR-028..FR-030). +- ``redaction`` may import `models` only. - ``state`` may import `models` only. Each group may also import its own submodules — intra-boundary imports @@ -37,6 +42,11 @@ "opencloser.transport", "opencloser.persona", "opencloser.crm", + # contracts/redaction-layer.md: the orchestrator accepts a configured + # RedactionLayer and plumbs it through to the artifact writer + # (FR-028..FR-030). Slice 1 callers omit it and get the writer's + # cached default-on layer. + "opencloser.redaction", "opencloser.core", }, "eligibility": { @@ -65,6 +75,9 @@ }, "artifacts": { "opencloser.models", + # contracts/redaction-layer.md §Behavior: the writer calls the RedactionLayer + # immediately before any transcript disk write (FR-028..FR-030). + "opencloser.redaction", "opencloser.artifacts", # intra-boundary submodules }, # ---- Slice 2 boundaries (contracts/redaction-layer.md, contracts/cli-slice2.md) ---- diff --git a/tests/unit/test_artifacts_writer.py b/tests/unit/test_artifacts_writer.py index 99b8b1a..76a3199 100644 --- a/tests/unit/test_artifacts_writer.py +++ b/tests/unit/test_artifacts_writer.py @@ -8,17 +8,12 @@ from opencloser.artifacts.writer import write_session_artifacts from opencloser.models import ( - CallableStatus, ConflictingEventAuditRecord, Disposition, EventType, - ExportedEligibilityDecision, - NormalizedResult, - PhoneCallActivityPayload, - QueueStatusUpdatePayload, - TaskPayload, WriteBack, ) +from tests.fixtures.artifact_inputs import make_artifact_inputs pytestmark = pytest.mark.module("artifacts") @@ -26,70 +21,7 @@ def _make_inputs(session_id: str = "ses_1") -> dict[str, object]: - nr = NormalizedResult( - session_id=session_id, - queue_item_id="q1", - mock_provider_call_id="call_x", - persona_version="alf-appointment-setter@0.1.0", - final_disposition=Disposition.INTERESTED_CALLBACK_REQUESTED, - summary="Decision-maker confirmed interested; callback requested Thursday 14:00.", - transcript_pointer="transcript.txt", - callback_requested=True, - preferred_callback_window="Thursday 14:00", - captured_email="ok@example.com", - started_at=_T, - ended_at=_T, - ) - activity = PhoneCallActivityPayload( - session_id=session_id, - queue_item_id="q1", - mock_provider_call_id="call_x", - persona_version="alf-appointment-setter@0.1.0", - final_disposition=Disposition.INTERESTED_CALLBACK_REQUESTED, - summary=nr.summary or "", - started_at=_T, - ended_at=_T, - ) - status = QueueStatusUpdatePayload( - session_id=session_id, - queue_item_id="q1", - previous_status=CallableStatus.READY, - new_status=CallableStatus.READY, - transition_reason="interested_callback_requested", - transition_at=_T, - ) - task = TaskPayload( - task_id="task_1", - session_id=session_id, - queue_item_id="q1", - task_kind="callback", - subject="Callback Thursday 14:00", - preferred_callback_window="Thursday 14:00", - captured_email="ok@example.com", - persona_version="alf-appointment-setter@0.1.0", - created_at=_T, - ) - writeback = WriteBack( - session_id=session_id, - phone_call_activity=activity, - queue_status_update=status, - task=task, - ) - eligibility = ExportedEligibilityDecision( - decision_id="dec_1", - queue_item_id="q1", - session_id=session_id, - decided_at=_T, - outcome="allow", - rules={"a": True, "b": True, "c": True, "d": True, "e": True, "f": True}, - ) - return { - "normalized_result": nr, - "writeback": writeback, - "eligibility_decision": eligibility, - "task": task, - "transcript_text": "[persona] Hi, this is an AI assistant.\n[contact] Sure.\n", - } + return make_artifact_inputs(session_id) def test_artifacts_written_to_session_dir(tmp_artifact_dir: Path) -> None: diff --git a/tests/unit/test_redaction.py b/tests/unit/test_redaction.py new file mode 100644 index 0000000..a75cea6 --- /dev/null +++ b/tests/unit/test_redaction.py @@ -0,0 +1,192 @@ +"""Unit tests for the transcript RedactionLayer (FR-028, FR-029, FR-030, SC-009). + +Covers the contract in +``specs/002-mock-call-real-crm/contracts/redaction-layer.md`` — default-on regex +policy with ``[REDACTED]`` replacement, no-op policy preserving transcript bytes, +summary-only retention, and readiness failure for a malformed redaction pattern. +""" + +from __future__ import annotations + +import json +from pathlib import Path + +import pytest + +from opencloser.artifacts.writer import write_session_artifacts +from opencloser.models import BUILTIN_REDACTION_PATTERNS, RedactionPolicyConfig +from opencloser.redaction.layer import ( + DEFAULT_REPLACEMENT, + NoOpPolicy, + RedactionLayer, + RegexRedactionPolicy, +) +from tests.fixtures.artifact_inputs import make_artifact_inputs + +pytestmark = pytest.mark.module("redaction") + +_TRANSCRIPT_WITH_PII = ( + "[persona] Could I confirm the best callback number?\n" + "[contact] Sure, it's 555-123-4567 and email alice@example.com.\n" +) + + +# --------------------------------------------------------------------------- +# RedactionLayer construction & policy behavior +# --------------------------------------------------------------------------- + + +def test_default_layer_redacts_emails_and_phone_numbers() -> None: + layer = RedactionLayer.default() + text = "Call me at 555-123-4567 or email me at ok@example.com." + out = layer.redact(text) + assert "555-123-4567" not in out + assert "ok@example.com" not in out + assert DEFAULT_REPLACEMENT in out + assert layer.retention_mode() == "full" + + +@pytest.mark.parametrize( + "raw,expected_redactions", + [ + ("Call (555) 123-4567 back.", 1), + ("Try 555.123.4567 or +1 555 123 4567.", 2), + ("ping bob@example.org and alice@example.co.uk", 2), + # Privacy-conservative: bare 10-digit phone numbers are also redacted. + ("Phone 5551234567 bare digits.", 1), + ], +) +def test_regex_policy_match_variants(raw: str, expected_redactions: int) -> None: + # Exercise the built-in defaults (email + NA phone) as they ship in the config. + policy = RegexRedactionPolicy.from_patterns(BUILTIN_REDACTION_PATTERNS) + out = policy.redact(raw) + assert out.count(DEFAULT_REPLACEMENT) == expected_redactions + + +def test_noop_policy_returns_text_unchanged() -> None: + layer = RedactionLayer(policy=NoOpPolicy(), _retention_mode="full") + text = "Call me at 555-123-4567 or email ok@example.com." + assert layer.redact(text) == text + assert layer.retention_mode() == "full" + + +def test_from_config_regex_default() -> None: + cfg = RedactionPolicyConfig() # policy="regex", retention="full" defaults + layer = RedactionLayer.from_config(cfg) + assert layer.retention_mode() == "full" + assert isinstance(layer.policy, RegexRedactionPolicy) + assert DEFAULT_REPLACEMENT in layer.redact("555-123-4567") + + +def test_from_config_noop_explicit() -> None: + cfg = RedactionPolicyConfig(policy="noop", retention="full") + layer = RedactionLayer.from_config(cfg) + assert isinstance(layer.policy, NoOpPolicy) + assert layer.redact("555-123-4567") == "555-123-4567" + + +def test_from_config_summary_only_retention() -> None: + cfg = RedactionPolicyConfig(policy="regex", retention="summary-only") + layer = RedactionLayer.from_config(cfg) + assert layer.retention_mode() == "summary-only" + + +def test_from_config_user_patterns_replace_builtins() -> None: + """An explicit ``patterns`` list replaces the built-in defaults — config + code + stay aligned by giving the regex policy exactly what the operator configured. + To keep the built-ins, callers must include them explicitly (or omit ``patterns`` + entirely so the field default supplies them).""" + cfg = RedactionPolicyConfig(policy="regex", patterns=[r"SECRET-\d{4}"]) + layer = RedactionLayer.from_config(cfg) + out = layer.redact("token SECRET-1234 and phone 555-123-4567") + # Only the user pattern matches; the built-in phone pattern is not silently applied. + assert out.count(DEFAULT_REPLACEMENT) == 1 + assert "555-123-4567" in out + + +def test_from_config_user_patterns_can_extend_builtins() -> None: + """When operators want built-ins + extras, they spell that out via + ``BUILTIN_REDACTION_PATTERNS`` in their config — predictable composition.""" + cfg = RedactionPolicyConfig( + policy="regex", + patterns=[*BUILTIN_REDACTION_PATTERNS, r"SECRET-\d{4}"], + ) + layer = RedactionLayer.from_config(cfg) + out = layer.redact("token SECRET-1234 and phone 555-123-4567") + assert out.count(DEFAULT_REPLACEMENT) == 2 + + +def test_malformed_redaction_pattern_fails_readiness() -> None: + """SC-009: a malformed redaction policy fails readiness — surfaced as a ValueError + raised when the layer is built from config (the orchestrator wraps this as a + readiness failure).""" + bad_cfg = RedactionPolicyConfig(policy="regex", patterns=["(unclosed"]) + with pytest.raises(ValueError, match="Invalid redaction regex"): + RedactionLayer.from_config(bad_cfg) + + +# --------------------------------------------------------------------------- +# Writer integration +# --------------------------------------------------------------------------- + + +def test_writer_redacts_transcript_under_regex_policy(tmp_artifact_dir: Path) -> None: + """FR-028: with regex policy + full retention, written transcript contains + ``[REDACTED]`` and not the original PII; the Slice 1 session-result contract + (summary + transcript pointer) is preserved.""" + session_id = "ses_red_1" + layer = RedactionLayer.from_config(RedactionPolicyConfig()) # regex / full defaults + paths = write_session_artifacts( + artifact_root=tmp_artifact_dir, + session_id=session_id, + redaction_layer=layer, + **make_artifact_inputs(session_id, transcript_text=_TRANSCRIPT_WITH_PII), + ) + assert paths.transcript is not None and paths.transcript.exists() + content = paths.transcript.read_text(encoding="utf-8") + assert "555-123-4567" not in content + assert "alice@example.com" not in content + assert DEFAULT_REPLACEMENT in content + sr = json.loads(paths.session_result.read_text(encoding="utf-8")) + assert sr["transcript_pointer"] == "transcript.txt" + + +def test_writer_summary_only_retention_omits_transcript(tmp_artifact_dir: Path) -> None: + """FR-030: summary-only retention writes NO transcript file; session-result + summary is emitted and its ``transcript_pointer`` is null (no dangling pointer).""" + session_id = "ses_red_2" + layer = RedactionLayer.from_config( + RedactionPolicyConfig(policy="regex", retention="summary-only") + ) + paths = write_session_artifacts( + artifact_root=tmp_artifact_dir, + session_id=session_id, + redaction_layer=layer, + **make_artifact_inputs(session_id, transcript_text=_TRANSCRIPT_WITH_PII), + ) + assert paths.transcript is None + assert not (tmp_artifact_dir / session_id / "transcript.txt").exists() + sr = json.loads(paths.session_result.read_text(encoding="utf-8")) + assert sr["summary"].startswith("Decision-maker confirmed interested") + assert sr["transcript_pointer"] is None + + +def test_writer_noop_policy_preserves_slice1_contract(tmp_artifact_dir: Path) -> None: + """FR-029: no-op policy + full retention writes the transcript byte-identically + to the original (Slice 1 unredacted contract).""" + session_id = "ses_noop" + noop_layer = RedactionLayer(policy=NoOpPolicy(), _retention_mode="full") + inputs = make_artifact_inputs(session_id, transcript_text=_TRANSCRIPT_WITH_PII) + + write_session_artifacts( + artifact_root=tmp_artifact_dir / "noop", + session_id=session_id, + redaction_layer=noop_layer, + **inputs, + ) + noop_bytes = (tmp_artifact_dir / "noop" / session_id / "transcript.txt").read_bytes() + + transcript_in = inputs["transcript_text"] + assert isinstance(transcript_in, str) + expected = transcript_in if transcript_in.endswith("\n") else transcript_in + "\n" + assert noop_bytes == expected.encode("utf-8")