diff --git a/.specify/feature.json b/.specify/feature.json index 67b026e..0963034 100644 --- a/.specify/feature.json +++ b/.specify/feature.json @@ -1 +1 @@ -{"feature_directory": "specs/026-darnit-harness"} +{"feature_directory": "specs/027-interactive-resolvers"} diff --git a/CLAUDE.md b/CLAUDE.md index bfc6255..d930060 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -369,6 +369,7 @@ else: - Filesystem only. Composition is resolved in-memory at framework-config load time; no new persistent state. (013-plugin-composition) ## Recent Changes +- 027-interactive-resolvers: adds `--interactive` flag to `darnit harness` and a new `QuestionResolver` Protocol (async, `@runtime_checkable`) that sits downstream of feature 026's `AnswerSource` chain. `InteractiveTerminalResolver` reference implementation prompts on `/dev/tty` (isolated from stdout report / stderr progress streams). Third-party resolvers register via Python entry points under group `darnit.question_resolvers` (mirrors `darnit.frameworks` discovery). Every `Answer` carries `authority: "asserted"` enforced at the model layer via `Literal["asserted"]` with a fixed default. Per-question `resolution_trail` in the report captures which resolvers were offered a question and how each responded (`answered`/`skipped`/`errored`). Fail-fast (<2s) when stdin is not a TTY OR /dev/tty is not openable under `--interactive`. Feature 026's "no re-audit after collect" MVP policy preserved. - 026-darnit-harness: adds `darnit harness` subcommand -- end-to-end audit driver with in-band LLM dispatch (fleet-operator + CI-integrated persona). Consumes `ANTHROPIC_API_KEY` from env; dispatches PENDING_LLM results via `PydanticAILLMStep`. Non-interactive by default; batch answers via pluggable `AnswerSource` Protocol with auto-discovery of `.project/project.yaml` + `--answers` override. Markdown + JSON reports. Four documented exit codes (0/1/2/3) plus grep-able stderr summary. New `darnit.harness` subpackage (`driver`, `answer_sources`, `report`, `exit_codes`). - 025-rfc0001-stage1: RFC-0001 Stage 1. Adds `authority` (`dispositive`|`suggestive`|`asserted`) to every step + result; per-phase Check execution rule ensures only dispositive/asserted results conclude a control (LLM output alone cannot manufacture a PASS). New `darnit.core.action_plan` module exposes `next_action`/`submit_result` as a public typed protocol; `agent.graph.route()` becomes a thin adapter. MCP surface adds `run_next_action`/`submit_action_result` tools (client-owned state). Baseline attestation predicate gains a per-result `authority` field additively within v1. `pydantic-ai-slim[anthropic]` becomes a required runtime dep. - 024-cmd-run-e2e-tests: E2E baseline for `darnit run` pinning header/footer/count/exit-code contract; used as the mechanical regression guarantee for Stage 1's `cmd_run` code path. @@ -378,5 +379,5 @@ else: For additional context about technologies to be used, project structure, shell commands, and other important information, read the current plan: -[`specs/026-darnit-harness/plan.md`](specs/026-darnit-harness/plan.md) +[`specs/027-interactive-resolvers/plan.md`](specs/027-interactive-resolvers/plan.md) diff --git a/packages/darnit/pyproject.toml b/packages/darnit/pyproject.toml index 738664d..7b11bad 100644 --- a/packages/darnit/pyproject.toml +++ b/packages/darnit/pyproject.toml @@ -53,6 +53,13 @@ darnit = "darnit.cli:main" # Framework plugins register here # Example: openssf-baseline = "darnit_baseline:get_framework_path" +[project.entry-points."darnit.question_resolvers"] +# Feature 027: QuestionResolver plugins register here. Entry-point declarations +# are LAZY -- Python only loads the target module when a caller invokes +# `importlib.metadata.entry_points(group=...).load()`. Missing / broken entry +# points are logged and skipped at discovery time, never crash the harness. +interactive_terminal = "darnit.harness.interactive_resolver:build" + [project.optional-dependencies] attestation = [ "sigstore>=3.0.0", diff --git a/packages/darnit/src/darnit/cli.py b/packages/darnit/src/darnit/cli.py index 4428e72..d0bb3be 100644 --- a/packages/darnit/src/darnit/cli.py +++ b/packages/darnit/src/darnit/cli.py @@ -750,6 +750,30 @@ def cmd_harness(args: argparse.Namespace) -> int: output_format = getattr(args, "format", "markdown") output_path = getattr(args, "output", None) answers_path = getattr(args, "answers", None) + interactive = getattr(args, "interactive", False) + per_resolver_timeout_s = getattr(args, "per_resolver_timeout", None) + + # Feature 027: --interactive fail-fast guard (IR-7..IR-9 / SC-005). + # Must run BEFORE any control iteration so a CI misfire never silently + # skips every question. + if interactive: + if not sys.stdin.isatty(): + _emit_exit_summary( + "setup_error, interactive channel unavailable " + "(stdin is not a TTY)", + HarnessExitCode.SETUP_ERROR, + ) + return int(HarnessExitCode.SETUP_ERROR) + try: + _tty_probe = open("/dev/tty", "r+", buffering=1) # noqa: SIM115 + _tty_probe.close() + except OSError as exc: + _emit_exit_summary( + "setup_error, interactive channel unavailable " + f"(/dev/tty not openable: {exc.strerror or type(exc).__name__})", + HarnessExitCode.SETUP_ERROR, + ) + return int(HarnessExitCode.SETUP_ERROR) # Build the resolver via the explicit factory. Any AnswerSourceLoadError # from a bad --answers file surfaces as a SETUP_ERROR. @@ -765,6 +789,29 @@ def cmd_harness(args: argparse.Namespace) -> int: _emit_exit_summary(f"setup_error, {exc}", HarnessExitCode.SETUP_ERROR) return int(HarnessExitCode.SETUP_ERROR) + # Feature 027: build the QuestionResolver chain via entry-point discovery. + # PR #367 review Constitution IV fix: external resolvers stay out of the + # chain unless --allow-external-resolvers is set. An answer they produce + # is recorded with authority="asserted", so silent invocation would let + # any installed third-party package produce dispositive-strength values + # without operator opt-in. + allow_external_resolvers = getattr(args, "allow_external_resolvers", False) + try: + question_resolvers = HarnessRun.build_default_resolver_chain( + interactive=interactive, + allow_external_resolvers=allow_external_resolvers, + ) + except HarnessSetupError as exc: + _emit_exit_summary(f"setup_error, {exc}", HarnessExitCode.SETUP_ERROR) + return int(HarnessExitCode.SETUP_ERROR) + + if question_resolvers: + harness_logger = get_logger("harness") + harness_logger.info( + "harness: resolvers configured: %s", + [getattr(r, "name", "unknown") for r in question_resolvers], + ) + run = HarnessRun( local_path=repo_path, framework_name=getattr(args, "framework", None), @@ -773,6 +820,8 @@ def cmd_harness(args: argparse.Namespace) -> int: llm_step=PydanticAILLMStep(), per_call_timeout_s=getattr(args, "per_call_timeout", 60), total_run_timeout_s=getattr(args, "total_run_timeout", 900), + question_resolvers=question_resolvers, + per_resolver_timeout_s=per_resolver_timeout_s, ) try: @@ -803,11 +852,18 @@ def cmd_harness(args: argparse.Namespace) -> int: if not text.endswith("\n"): sys.stdout.write("\n") - # Exit summary + # Exit summary. Order matches CLI-13 exactly: + # `complete,

PASS, FAIL, WARN, pending, exit ` + # so CI parsers written against the contract keep working. Answered + # count is appended AFTER `pending`; the CLI-13 grep pattern is a + # prefix match on positional fields, so the extra trailing field is + # additive rather than positional-shifting. PR #367 review fix. s = report.summary + answered_count = len(report.answered_feedback) + pending_count = len(report.pending_feedback) _emit_exit_summary( f"complete, {s.pass_} PASS, {s.fail} FAIL, {s.warn} WARN, " - f"{len(report.pending_feedback)} pending", + f"{pending_count} pending, {answered_count} answered", HarnessExitCode(report.exit_class), ) return report.exit_class @@ -1185,6 +1241,37 @@ def create_parser() -> argparse.ArgumentParser: default=900, help="Total audit-run timeout in seconds (default: 900 = 15 min)", ) + harness_parser.add_argument( + "--interactive", + action="store_true", + help=( + "Prompt the operator at the terminal for any pending feedback " + "question not covered by --answers or .project/project.yaml. " + "Requires stdin to be a TTY and /dev/tty to be openable; fails " + "fast with exit 2 otherwise." + ), + ) + harness_parser.add_argument( + "--per-resolver-timeout", + type=float, + default=None, + help=( + "Per-resolver timeout in seconds. Default: no timeout. " + "Setting a global bound is usually inappropriate (an operator at " + "a terminal cannot be on the same clock as a webhook resolver)." + ), + ) + harness_parser.add_argument( + "--allow-external-resolvers", + action="store_true", + help=( + "Include third-party question resolvers registered under the " + "`darnit.question_resolvers` entry-point group. Off by default: " + "external resolvers produce authority='asserted' values, so " + "silently including them would violate Constitution Principle IV " + "(only human confirmation may assert)." + ), + ) harness_parser.set_defaults(func=cmd_harness) # install command diff --git a/packages/darnit/src/darnit/harness/__init__.py b/packages/darnit/src/darnit/harness/__init__.py index e69de29..fb92881 100644 --- a/packages/darnit/src/darnit/harness/__init__.py +++ b/packages/darnit/src/darnit/harness/__init__.py @@ -0,0 +1,60 @@ +"""darnit.harness: end-to-end audit driver (feature 026) + interactive resolvers (feature 027). + +Public API surface -- re-exports from submodules so downstream consumers can +import from `darnit.harness` directly without knowing about internal layout. +""" + +from darnit.harness.answer_sources import ( + AnswerResolver, + AnswerSource, + FileAnswerSource, + ProjectYamlAnswerSource, +) +from darnit.harness.driver import ( + HarnessRun, + HarnessRunTimeout, + HarnessSetupError, +) +from darnit.harness.exit_codes import HarnessExitCode +from darnit.harness.interactive_resolver import InteractiveTerminalResolver +from darnit.harness.question_resolvers import ( + Answer, + InteractiveAborted, + QuestionResolver, + ResolutionTrailEntry, +) +from darnit.harness.report import ( + AnsweredFeedbackEntry, + HarnessReport, + HarnessSummary, + PendingFeedbackEntry, +) +from darnit.harness.resolver_discovery import ( + build_default_resolver_chain, + discover_registered_resolvers, +) + +__all__ = ( + # Feature 026: driver + report + "HarnessRun", + "HarnessRunTimeout", + "HarnessSetupError", + "HarnessExitCode", + "HarnessReport", + "HarnessSummary", + "PendingFeedbackEntry", + # Feature 026: answer sources + "AnswerResolver", + "AnswerSource", + "FileAnswerSource", + "ProjectYamlAnswerSource", + # Feature 027: question resolvers + "Answer", + "AnsweredFeedbackEntry", + "InteractiveAborted", + "InteractiveTerminalResolver", + "QuestionResolver", + "ResolutionTrailEntry", + "build_default_resolver_chain", + "discover_registered_resolvers", +) diff --git a/packages/darnit/src/darnit/harness/driver.py b/packages/darnit/src/darnit/harness/driver.py index b8f4bfb..baf5d72 100644 --- a/packages/darnit/src/darnit/harness/driver.py +++ b/packages/darnit/src/darnit/harness/driver.py @@ -26,7 +26,17 @@ ProjectYamlAnswerSource, ) from darnit.harness.exit_codes import HarnessExitCode -from darnit.harness.report import HarnessReport, HarnessSummary, PendingFeedbackEntry +from darnit.harness.question_resolvers import ( + Answer, + InteractiveAborted, + ResolutionTrailEntry, +) +from darnit.harness.report import ( + AnsweredFeedbackEntry, + HarnessReport, + HarnessSummary, + PendingFeedbackEntry, +) from darnit.sieve.models import LLMConsultationResponse, PassOutcome from darnit.tools.audit import prepare_audit, run_checks @@ -86,6 +96,12 @@ class HarnessRun: per_call_timeout_s: int = 60 total_run_timeout_s: int = 15 * 60 + # Feature 027: pluggable active resolvers (interactive terminal, A2A, etc). + # Runs downstream of the AnswerSource chain. Default empty list preserves + # feature 026 behavior (batch-only collection). See data-model.md section 7. + question_resolvers: list[Any] = field(default_factory=list) + per_resolver_timeout_s: float | None = None + # Counters populated during .run() llm_calls_total: int = 0 llm_provider: str = "anthropic:claude-sonnet-5" @@ -116,6 +132,37 @@ def build_default_resolver( resolver.add(FileAnswerSource(answers_path)) return resolver + # ------------------------------------------------------------------ + # Factory for QuestionResolver chain (feature 027 T017/T018) + # ------------------------------------------------------------------ + + @classmethod + def build_default_resolver_chain( + cls, + interactive: bool, + *, + allow_external_resolvers: bool = False, + ) -> list[Any]: + """Compose the CLI's canonical resolver chain. + + Delegates to `darnit.harness.resolver_discovery.build_default_resolver_chain` + which uses `importlib.metadata` entry-points under group + `darnit.question_resolvers` (contract QR-14..QR-16). + + - If interactive=True: the `interactive_terminal` resolver is first. + - Third-party resolvers are included only when + ``allow_external_resolvers=True`` (PR #367 review fix; + Constitution Principle IV). + """ + # Local import so a broken discovery module doesn't crash the driver's + # module load. + from darnit.harness.resolver_discovery import build_default_resolver_chain + + return build_default_resolver_chain( + interactive=interactive, + allow_external_resolvers=allow_external_resolvers, + ) + # ------------------------------------------------------------------ # Startup checks (T010, T011) # ------------------------------------------------------------------ @@ -396,32 +443,71 @@ async def _llm_continuation_loop( # Collect (T014) # ------------------------------------------------------------------ - def _collect_unanswered( + def _enumerate_framework_pending(self) -> list[Any]: + """Return framework-declared context questions unresolved so far. + + Extracted to a method so isolated tests can monkeypatch it and + assert on only the questions their fixtures synthesized. Failure + is not fatal -- returns [] on any error and logs at DEBUG. + """ + try: + from darnit.config.context_storage import get_pending_context + + return list(get_pending_context(self.local_path, level=self.level)) + except Exception as exc: + logger.debug("get_pending_context failed: %s", exc) + return [] + + async def _collect_unanswered( self, results: list[dict[str, Any]], - ) -> tuple[list[dict[str, Any]], list[PendingFeedbackEntry], dict[str, str]]: + ) -> tuple[ + list[dict[str, Any]], + list[PendingFeedbackEntry], + list[AnsweredFeedbackEntry], + dict[str, str], + ]: """Apply resolver answers to any feedback questions in the results. + Two-phase per contract QR-19: + 1. AnswerSource chain (feature 026 -- passive lookup by key). + 2. QuestionResolver chain (feature 027 -- active resolution). + + Contract QR-19: (1) runs strictly before (2); a question answered by + (1) never reaches the resolver chain. + Per data-model.md "State transitions" COLLECT_UNANSWERED: does NOT re-audit. A control's verdict RETAINS its pre-Collect status. The - answer is captured in context_values + on the question object; it - does NOT retroactively change the verdict. Also does NOT persist - to .project/ (research.md R4 idempotence argument). + answer is captured in context_values + on the question object + in + the report's answered_feedback list; it does NOT retroactively change + the verdict. Also does NOT persist to .project/ (research.md R4 + idempotence argument). Feedback questions come from two sources (PR #365 review fix): 1. Any ``result["feedback_questions"]`` a caller has already attached (unchanged legacy path). 2. The framework's own pending-context enumerator - (``darnit.config.context_storage.get_pending_context``). This - is the only source that currently fires in production; before + (``darnit.config.context_storage.get_pending_context``). Before this fix, ``--answers`` had nothing to match against and was - effectively inert. + effectively inert because sieve results never carry + ``feedback_questions``. + + Each source funnels through Phase 1 (AnswerSource chain, same + semantics as before) and then Phase 2 (QuestionResolver chain, + added in PR #367). - Returns (mutated_results, remaining_pending_feedback, context_values). + Returns (mutated_results, pending_feedback, answered_feedback, + context_values). """ context_values: dict[str, str] = {} remaining_pending: list[PendingFeedbackEntry] = [] + answered: list[AnsweredFeedbackEntry] = [] + + # Collect questions that survive the AnswerSource pass. These will + # be offered to the QuestionResolver chain (phase 2). Each element: + # (result_dict, question_dict_or_obj, ctx_key, question_text) + unresolved: list[tuple[dict[str, Any], Any, str, str]] = [] # (1) Legacy attach path: caller-populated feedback_questions. for result in results: @@ -430,12 +516,15 @@ def _collect_unanswered( if isinstance(q, dict): ctx_key = q.get("context_key") already = q.get("answered", False) + q_text = q.get("question", "") else: ctx_key = getattr(q, "context_key", None) already = getattr(q, "answered", False) + q_text = getattr(q, "question", "") if not ctx_key or already: continue + # Phase 1: AnswerSource chain. answer, source_name = self.answer_resolver.resolve(ctx_key) if answer is not None: context_values[ctx_key] = answer @@ -446,52 +535,287 @@ def _collect_unanswered( else: q.answered = True q.answer = answer - else: - q_text = q.get("question", "") if isinstance(q, dict) else getattr(q, "question", "") - remaining_pending.append( - PendingFeedbackEntry( + answered.append( + AnsweredFeedbackEntry( control_id=result.get("id", ""), context_key=str(ctx_key), question=str(q_text), + answer=str(answer), + origin=str(source_name or "unknown"), ), ) + else: + unresolved.append( + (result, q, str(ctx_key), str(q_text)), + ) # (2) Framework pending-context enumerator: read `[context.*]` keys # that the framework declared and that current .project/project.yaml # has not yet answered. Route each through the answer resolver so # `--answers` and the auto-discovered `.project/project.yaml` are - # actually consulted. Failure to enumerate is not fatal -- log and - # continue with whatever the caller already attached. - try: - from darnit.config.context_storage import get_pending_context - - pending_ctx = get_pending_context(self.local_path, level=self.level) - except Exception as exc: - logger.debug("get_pending_context failed: %s", exc) - pending_ctx = [] - - seen_ctx_keys = set(context_values.keys()) | {e.context_key for e in remaining_pending} + # actually consulted (PR #365 review fix). Anything the resolver + # can't answer joins the `unresolved` list so it also gets the + # QuestionResolver chain (PR #367). + # + # Extracted through `_enumerate_framework_pending()` so isolated + # resolver-chain tests can monkeypatch it to [] and assert on just + # the questions they synthesized. + pending_ctx = self._enumerate_framework_pending() + + seen_ctx_keys = ( + set(context_values.keys()) + | {ctx_key for _r, _q, ctx_key, _t in unresolved} + ) for req in pending_ctx: ctx_key = req.key if ctx_key in seen_ctx_keys: continue seen_ctx_keys.add(ctx_key) - answer, _source = self.answer_resolver.resolve(ctx_key) + answer, source_name = self.answer_resolver.resolve(ctx_key) if answer is not None: context_values[ctx_key] = answer + answered.append( + AnsweredFeedbackEntry( + control_id=(req.control_ids[0] if req.control_ids else ""), + context_key=str(ctx_key), + question=str( + getattr(req.definition, "prompt", None) + or getattr(req.definition, "hint", None) + or ctx_key + ), + answer=str(answer), + origin=str(source_name or "unknown"), + ), + ) continue - question_text = getattr(req.definition, "prompt", None) or getattr(req.definition, "hint", None) or ctx_key - remaining_pending.append( - PendingFeedbackEntry( - control_id=(req.control_ids[0] if req.control_ids else ""), - context_key=ctx_key, - question=str(question_text), - ), + question_text = ( + getattr(req.definition, "prompt", None) + or getattr(req.definition, "hint", None) + or ctx_key + ) + synth_q = { + "control_id": (req.control_ids[0] if req.control_ids else ""), + "context_key": ctx_key, + "question": str(question_text), + "answered": False, + } + synth_result = {"id": (req.control_ids[0] if req.control_ids else "")} + unresolved.append((synth_result, synth_q, str(ctx_key), str(question_text))) + + # Phase 2: QuestionResolver chain. + if unresolved and self.question_resolvers: + resolver_answered, resolver_pending, resolver_ctx = await self._run_resolver_chain( + unresolved, ) + answered.extend(resolver_answered) + remaining_pending.extend(resolver_pending) + context_values.update(resolver_ctx) + elif unresolved: + # No resolver chain configured; every unresolved question stays + # pending with an empty trail. + for _result, _q, ctx_key, q_text in unresolved: + remaining_pending.append( + PendingFeedbackEntry( + control_id=_result.get("id", ""), + context_key=ctx_key, + question=q_text, + ), + ) + + return results, remaining_pending, answered, context_values + + async def _run_resolver_chain( + self, + unresolved: list[tuple[dict[str, Any], Any, str, str]], + ) -> tuple[ + list[AnsweredFeedbackEntry], + list[PendingFeedbackEntry], + dict[str, str], + ]: + """Offer each unresolved question to the resolver chain in order. + + Emits FR-013a bookend lines on `darnit.harness` INFO. Wraps each + resolver call in `asyncio.wait_for` when `per_resolver_timeout_s` is + set (FR-011). Builds a `resolution_trail` per question with one + `ResolutionTrailEntry` per resolver visited. + + The interactive resolver gets `(position, total)` threaded in via + keyword args so its prompt payload can carry the position indicator + (FR-013b). Other resolvers only receive the question. + """ + # Local imports to avoid circular import at module load. + from darnit.harness.interactive_resolver import InteractiveTerminalResolver + + total = len(unresolved) + logger.info( + "harness: starting interactive collection (%d pending questions)", + total, + ) + + answered_out: list[AnsweredFeedbackEntry] = [] + pending_out: list[PendingFeedbackEntry] = [] + ctx_values: dict[str, str] = {} + counts = {"answered": 0, "skipped": 0, "aborted": 0} + + aborted = False + for idx, (result, q, ctx_key, q_text) in enumerate(unresolved, start=1): + if aborted: + # Preserve Ctrl+C semantics: skip the rest without offering + # to any resolver. Question stays pending with no trail. + pending_out.append( + PendingFeedbackEntry( + control_id=result.get("id", ""), + context_key=ctx_key, + question=q_text, + ), + ) + # PR #367 review fix: bump the aborted counter here too -- + # previously only the question that triggered the abort + # got counted, so the exit-summary undercounted aborts by + # (total - 1). + counts["aborted"] += 1 + continue + + trail: list[ResolutionTrailEntry] = [] + resolved_answer: Answer | None = None + + for resolver in self.question_resolvers: + # Thread position + total into the interactive resolver. + extra_kwargs: dict[str, Any] = {} + if isinstance(resolver, InteractiveTerminalResolver): + extra_kwargs = {"position": idx, "total": total} + + try: + coro = resolver.resolve(q, **extra_kwargs) + if self.per_resolver_timeout_s is not None: + result_ans = await asyncio.wait_for( + coro, + timeout=self.per_resolver_timeout_s, + ) + else: + result_ans = await coro + except InteractiveAborted: + trail.append( + ResolutionTrailEntry( + resolver_name=getattr(resolver, "name", ""), + outcome="skipped", + ), + ) + aborted = True + break + except TimeoutError: + trail.append( + ResolutionTrailEntry( + resolver_name=getattr(resolver, "name", ""), + outcome="errored", + error_summary=( + f"resolver timed out after {self.per_resolver_timeout_s}s" + ), + ), + ) + continue + except Exception as exc: # noqa: BLE001 + logger.warning( + "resolver %s raised %s during resolve()", + getattr(resolver, "name", ""), + type(exc).__name__, + ) + safe = _redact_secrets(str(exc))[:200] or "(no error message)" + trail.append( + ResolutionTrailEntry( + resolver_name=resolver.name, + outcome="errored", + error_summary=safe, + ), + ) + continue + + # Success path: None => skip, Answer with non-empty stripped + # value => answered, empty/whitespace-only Answer => skip + # (FR-006a, symmetric with interactive prompt handling). + if result_ans is None: + trail.append( + ResolutionTrailEntry( + resolver_name=resolver.name, outcome="skipped", + ), + ) + continue + if not result_ans.value.strip(): + trail.append( + ResolutionTrailEntry( + resolver_name=resolver.name, outcome="skipped", + ), + ) + continue + + # Answered. + trail.append( + ResolutionTrailEntry( + resolver_name=resolver.name, outcome="answered", + ), + ) + resolved_answer = result_ans + break + + # Record on the question dict + context_values regardless of shape. + if resolved_answer is not None: + ctx_values[ctx_key] = resolved_answer.value + if isinstance(q, dict): + q["answered"] = True + q["answer"] = resolved_answer.value + q["answered_by"] = resolved_answer.origin + else: + q.answered = True + q.answer = resolved_answer.value + + answered_out.append( + AnsweredFeedbackEntry( + control_id=result.get("id", ""), + context_key=ctx_key, + question=q_text, + answer=resolved_answer.value, + origin=resolved_answer.origin, + resolution_trail=trail, + ), + ) + counts["answered"] += 1 + else: + pending_out.append( + PendingFeedbackEntry( + control_id=result.get("id", ""), + context_key=ctx_key, + question=q_text, + resolution_trail=trail, + ), + ) + if aborted: + counts["aborted"] += 1 + else: + counts["skipped"] += 1 + + # Close any resolvers exposing a close() method (e.g., interactive). + for resolver in self.question_resolvers: + close_fn = getattr(resolver, "close", None) + if callable(close_fn): + try: + close_fn() + except Exception as exc: # noqa: BLE001 + logger.warning( + "resolver %s raised during close(): %s", + getattr(resolver, "name", "?"), + type(exc).__name__, + ) + + logger.info( + "harness: finished interactive collection: %d answered, %d skipped, %d aborted", + counts["answered"], + counts["skipped"], + counts["aborted"], + ) - return results, remaining_pending, context_values + return answered_out, pending_out, ctx_values # ------------------------------------------------------------------ # Report assembly (T018) @@ -503,6 +827,7 @@ def _assemble_report( target_owner: str, target_repo: str, pending_feedback: list[PendingFeedbackEntry], + answered_feedback: list[AnsweredFeedbackEntry] | None = None, ) -> HarnessReport: summary_counts = {"PASS": 0, "FAIL": 0, "WARN": 0, "N/A": 0, "ERROR": 0, "PENDING_LLM": 0} for r in results: @@ -528,6 +853,10 @@ def _assemble_report( else: exit_class = HarnessExitCode.SUCCESS + resolvers_used = [ + getattr(r, "name", "unknown") for r in self.question_resolvers + ] + return HarnessReport( target={ "local_path": self.local_path, @@ -540,6 +869,8 @@ def _assemble_report( answer_sources_used=self.answer_resolver.sources_used(), llm_calls={"total": self.llm_calls_total, "provider": self.llm_provider}, exit_class=int(exit_class), + resolvers_used=resolvers_used, + answered_feedback=answered_feedback or [], ) # ------------------------------------------------------------------ @@ -551,9 +882,11 @@ async def run(self) -> HarnessReport: Lifecycle per data-model.md "State transitions": 1. Credentials check (missing key -> raise HarnessSetupError) - 2. Initial audit (stop_on_llm=True) + 2. Initial audit (stop_on_llm=True; bounded by total_run_timeout_s) 3. LLM continuation loop (bounded by total_run_timeout_s) - 4. Collect unanswered (does NOT re-audit; MVP policy) + 4. Collect unanswered (NOT bounded; interactive operator think-time + MUST NOT count against the audit budget -- PR #367 review + blocker fix) 5. Assemble + return report Raises HarnessSetupError on class-2 conditions; caller in @@ -567,10 +900,16 @@ async def run(self) -> HarnessReport: logger.info("harness: starting audit of %s", self.local_path) logger.info(self.answer_resolver.summary()) - # Bound the whole run by total_run_timeout_s. + # Steps 2-3 (initial audit + LLM continuation) are bounded by + # total_run_timeout_s -- these are non-interactive and can hang + # on a bad repo or a stuck LLM call. Step 4 (collect) is NOT + # bounded, since interactive resolvers block on operator input + # and there is no useful ceiling on that. Per-resolver preemption + # is handled inside `_run_resolver_chain` via + # `per_resolver_timeout_s`. try: - report = await asyncio.wait_for( - self._run_body(), + audit_output = await asyncio.wait_for( + self._run_audit_and_llm(), timeout=self.total_run_timeout_s, ) except TimeoutError as exc: @@ -582,14 +921,28 @@ async def run(self) -> HarnessReport: f"audit exceeded total-run timeout of {self.total_run_timeout_s}s", ) from exc - return report + results, owner, repo, _default_branch = audit_output - async def _run_body(self) -> HarnessReport: - """Body of run(), separated so run() can wrap it in wait_for.""" + # Collect unanswered feedback questions (NOT bounded). + results, pending_feedback, answered_feedback, _context_values = await self._collect_unanswered(results) + + # Assemble report + return self._assemble_report( + results, owner, repo, pending_feedback, answered_feedback, + ) + + async def _run_audit_and_llm( + self, + ) -> tuple[list[dict[str, Any]], str, str, str]: + """Run the initial audit + LLM continuation loop (non-interactive). + + Bounded by ``total_run_timeout_s`` at the call site. Interactive + collect is intentionally NOT in here -- see `run()` docstring. + """ # Initial audit. run_sieve_audit is synchronous and calls out to gh/git # shell handlers that can block for arbitrary time on a bad repo. # Run it in a worker thread so `asyncio.wait_for(total_run_timeout_s)` - # around _run_body can actually preempt a stuck audit. + # around _run_audit_and_llm can actually preempt a stuck audit. results, owner, repo, default_branch = await asyncio.to_thread( self._initial_audit, ) @@ -618,12 +971,7 @@ async def _run_body(self) -> HarnessReport: # LLM continuation loop results = await self._llm_continuation_loop(results, owner, repo, default_branch) - - # Collect unanswered feedback questions - results, pending_feedback, _context_values = self._collect_unanswered(results) - - # Assemble report - return self._assemble_report(results, owner, repo, pending_feedback) + return results, owner, repo, default_branch class HarnessRunTimeout(Exception): diff --git a/packages/darnit/src/darnit/harness/interactive_resolver.py b/packages/darnit/src/darnit/harness/interactive_resolver.py new file mode 100644 index 0000000..fcd311e --- /dev/null +++ b/packages/darnit/src/darnit/harness/interactive_resolver.py @@ -0,0 +1,173 @@ +"""InteractiveTerminalResolver: the reference QuestionResolver (feature 027 T008). + +Prompts on `/dev/tty` -- the private operator channel used by git, ssh, sudo. +Isolated from stdout (report body) and stderr (progress + exit summary), so +feature 026's stream contracts stay intact even when a user runs the harness +interactively. + +Contract: + - contracts/interactive-resolver-behavior.md (IR-1..IR-31) + - contracts/question-resolver-protocol.md (QR-*) + +Design notes: + - Stream injection: tests pass `io.StringIO` for both streams; production + uses `/dev/tty`. The two-argument constructor exists exclusively for tests. + - Lazy /dev/tty open: on first `resolve()`, not on `__init__`. + - Empty / whitespace-only input -> None (skip). + - Ctrl+C (KeyboardInterrupt) or EOF (empty readline) -> InteractiveAborted. + +See specs/027-interactive-resolvers/plan.md + research.md R3. +""" + +from __future__ import annotations + +import asyncio +from typing import Any, TextIO + +from darnit.harness.question_resolvers import ( + Answer, + InteractiveAborted, + QuestionResolver, +) + + +class InteractiveTerminalResolver: + """Reference implementation of `QuestionResolver`. Prompts on `/dev/tty`.""" + + name = "interactive_terminal" + + def __init__( + self, + input_stream: TextIO | None = None, + output_stream: TextIO | None = None, + ) -> None: + # Both streams None => open /dev/tty lazily on first resolve(). + # Either non-None => tests / library callers supplied a channel. + self._input_stream = input_stream + self._output_stream = output_stream + self._tty: TextIO | None = None + self._closed = False + + def _ensure_streams(self) -> tuple[TextIO, TextIO]: + """Return (input, output) streams; open /dev/tty on demand. + + Per contract IR-23: /dev/tty is NOT opened when EITHER argument + is non-None. Per IR-25, partial injection is not supported -- + callers who wire only one side silently lost the other before + PR #367 review fixed it; that now raises a programming error. + """ + if (self._input_stream is None) != (self._output_stream is None): + raise ValueError( + "InteractiveTerminalResolver requires both input_stream and " + "output_stream, or neither. Providing only one is a " + "programming error (contract IR-25).", + ) + if self._input_stream is not None and self._output_stream is not None: + return self._input_stream, self._output_stream + + if self._tty is None: + # Import here so a HarnessSetupError from darnit.harness.driver + # doesn't create an import-cycle at module load time. + from darnit.harness.driver import HarnessSetupError + + try: + self._tty = open("/dev/tty", "r+", buffering=1) # noqa: SIM115 + except OSError as exc: + raise HarnessSetupError( + "interactive channel unavailable (/dev/tty not openable): " + f"{exc.strerror or type(exc).__name__}", + ) from exc + + return self._tty, self._tty + + def _format_prompt( + self, + question: Any, + position: int, + total: int, + ) -> str: + """Produce the prompt payload written to /dev/tty (IR-10). + + Order: blank line, `[N of M]`, control_id, question text, optional + Help block, `> ` chevron with no trailing newline. + """ + control_id = getattr(question, "control_id", None) or ( + question.get("control_id", "") if isinstance(question, dict) else "" + ) + question_text = getattr(question, "question", None) or ( + question.get("question", "") if isinstance(question, dict) else "" + ) + help_text = getattr(question, "help_md", None) or ( + question.get("help_md", "") if isinstance(question, dict) else "" + ) + + lines: list[str] = [] + lines.append("") # blank separator + lines.append(f"[{position} of {total}]") + lines.append(str(control_id)) + lines.append(str(question_text)) + if help_text: + lines.append(f" Help: {help_text}") + # Final line: chevron without newline (input appears inline). + return "\n".join(lines) + "\n> " + + async def resolve( + self, + question: Any, + *, + position: int = 1, + total: int = 1, + ) -> Answer | None: + """Prompt the operator for one question. Return Answer or None (skip). + + Raises InteractiveAborted on Ctrl+C or EOF. + """ + if self._closed: + raise RuntimeError("resolver is closed") + + in_stream, out_stream = self._ensure_streams() + + prompt = self._format_prompt(question, position=position, total=total) + out_stream.write(prompt) + out_stream.flush() + + # PR #367 review blocker fix: readline() is blocking and, when + # called directly in an async def, freezes the event loop -- so + # the driver's `asyncio.wait_for(coro, per_resolver_timeout_s)` + # never fires and operator think-time can't be preempted. Wrap + # the blocking call in `asyncio.to_thread` so the event loop + # keeps running and the driver's timeout is honored. + try: + raw = await asyncio.to_thread(in_stream.readline) + except KeyboardInterrupt as exc: + raise InteractiveAborted("operator sent SIGINT during prompt") from exc + + # EOF (Ctrl+D or piped-stdin exhausted). + if raw == "": + raise InteractiveAborted("EOF at interactive prompt") + + stripped = raw.rstrip("\n").strip() + if not stripped: + return None # skip + + return Answer(value=stripped, origin=self.name) + + def close(self) -> None: + """Release /dev/tty. Idempotent.""" + if self._closed: + return + self._closed = True + if self._tty is not None: + try: + self._tty.close() + except OSError: + pass + self._tty = None + + +def build() -> QuestionResolver: + """Entry-point factory for `darnit.question_resolvers = interactive_terminal`.""" + return InteractiveTerminalResolver() + + +__all__ = ("InteractiveTerminalResolver", "build") diff --git a/packages/darnit/src/darnit/harness/question_resolvers.py b/packages/darnit/src/darnit/harness/question_resolvers.py new file mode 100644 index 0000000..ff80acd --- /dev/null +++ b/packages/darnit/src/darnit/harness/question_resolvers.py @@ -0,0 +1,126 @@ +"""QuestionResolver Protocol + entities for the darnit harness (feature 027). + +Introduces an active, async answer producer that sits DOWNSTREAM of feature 026's +`AnswerSource` chain. Semantics: + +- `AnswerSource` (feature 026): passive lookup ("here's a preloaded value"). +- `QuestionResolver` (this module): active resolution ("get me an answer somehow + -- ask a human, call an API, open an issue"). + +The Protocol is `@runtime_checkable` so any class exposing `name: str` and +`async def resolve(question) -> Answer | None` conforms without explicit +inheritance. Registration is hybrid: + + - Python entry points under group `darnit.question_resolvers` (for third-party + packages, matching the `darnit.frameworks` discovery pattern). + - Direct injection into `HarnessRun.question_resolvers` (for tests and inline + library use). + +Constitution IV interaction: every `Answer` a resolver produces carries +`authority: "asserted"` -- enforced at the model level via `Literal["asserted"]` +with a fixed default. A resolver author physically cannot construct an `Answer` +with a different authority; Pydantic raises `ValidationError` at construction. + +Empty and whitespace-only answer values are treated as skip by the driver (FR-006a); +resolvers need not defensively check for them. + +See: + - `specs/027-interactive-resolvers/spec.md` + - `specs/027-interactive-resolvers/data-model.md` sections 1-3, 8 + - `specs/027-interactive-resolvers/contracts/question-resolver-protocol.md` +""" + +from __future__ import annotations + +from typing import Literal, Protocol, runtime_checkable + +from pydantic import BaseModel, ConfigDict, model_validator + + +class Answer(BaseModel): + """Value returned by a `QuestionResolver` for one pending feedback question. + + Fields: + - value: the string answer. Non-empty / non-whitespace-only invariant is + enforced at the driver layer, not on the model. See FR-006a. + - origin: provenance string. Convention: starts with the resolver's `name`. + - authority: fixed to "asserted" via Literal + default. FR-009 / SC-003. + """ + + model_config = ConfigDict(extra="forbid") + + value: str + origin: str + authority: Literal["asserted"] = "asserted" + + +class ResolutionTrailEntry(BaseModel): + """One entry in a `PendingFeedbackEntry.resolution_trail` list. + + Records which resolver was offered a question and how it responded. The + driver appends one entry per resolver visited, in order. See FR-015a. + """ + + model_config = ConfigDict(extra="forbid") + + resolver_name: str + outcome: Literal["answered", "skipped", "errored"] + error_summary: str | None = None + + @model_validator(mode="after") + def _check_error_summary(self) -> ResolutionTrailEntry: + # RT-6: outcome == "errored" requires error_summary + if self.outcome == "errored" and not self.error_summary: + raise ValueError( + "outcome='errored' requires a non-empty error_summary", + ) + if self.outcome != "errored" and self.error_summary is not None: + raise ValueError( + f"outcome={self.outcome!r} MUST NOT carry an error_summary", + ) + return self + + +@runtime_checkable +class QuestionResolver(Protocol): + """Active answer producer for one pending feedback question. + + Any object exposing: + - `name: str` (class or instance attribute) + - `async def resolve(self, question) -> Answer | None` + conforms to this Protocol. `isinstance(obj, QuestionResolver)` verifies + the shape at runtime; the async signature is validated on first call. + + Contract summary (see contracts/question-resolver-protocol.md for the full + list of rules QR-1..QR-27): + + - Return None to skip. Empty / whitespace-only Answer collapses to skip + at the driver layer. + - Raise on failure; the driver catches, redacts, and records `errored` + in the trail. Never crashes the harness. + - KeyboardInterrupt from within `resolve()` should propagate (except for + the interactive terminal resolver, which converts it to InteractiveAborted). + """ + + name: str + + async def resolve(self, question: object) -> Answer | None: + ... + + +class InteractiveAborted(Exception): + """Raised by the interactive terminal resolver on Ctrl+C or EOF. + + Signals the driver to stop offering further questions to any resolver + (not just the interactive one) but PRESERVE answers already collected in + the current collect phase. Never treated as an internal error; the harness + still assembles and returns the report. + """ + + +__all__ = ( + "Answer", + "ResolutionTrailEntry", + "QuestionResolver", + "InteractiveAborted", +) diff --git a/packages/darnit/src/darnit/harness/report.py b/packages/darnit/src/darnit/harness/report.py index cfba931..d87ae5b 100644 --- a/packages/darnit/src/darnit/harness/report.py +++ b/packages/darnit/src/darnit/harness/report.py @@ -1,14 +1,26 @@ """Harness report models: Markdown + JSON output. Feature 026 T016-T017. Contract report-format.md. + +Feature 027 additions: +- `PendingFeedbackEntry.resolution_trail`: which resolvers were offered a + question that ultimately remained pending. +- `AnsweredFeedbackEntry` (new): captures each answered feedback question so + provenance is recoverable from the report alone (SC-006). +- `HarnessReport.resolvers_used`: names of QuestionResolvers configured for + the run. +- `HarnessReport.answered_feedback`: list of answered feedback questions, + each carrying `origin`, `authority`, and `resolution_trail`. """ from __future__ import annotations -from typing import Any +from typing import Any, Literal from pydantic import BaseModel, ConfigDict, Field +from darnit.harness.question_resolvers import ResolutionTrailEntry + class HarnessSummary(BaseModel): """Aggregate counts across all controls in the audit.""" @@ -25,11 +37,39 @@ class HarnessSummary(BaseModel): class PendingFeedbackEntry(BaseModel): - """One unanswered feedback question captured in the report.""" + """One unanswered feedback question captured in the report. + + Feature 027 addition: `resolution_trail` records which resolvers were + offered this question before it was left pending (all skipped/errored). + Empty when no resolvers were configured or the AnswerSource chain didn't + forward this question. + """ control_id: str context_key: str question: str + resolution_trail: list[ResolutionTrailEntry] = Field(default_factory=list) + + model_config = ConfigDict(extra="forbid") + + +class AnsweredFeedbackEntry(BaseModel): + """One ANSWERED feedback question captured in the report. + + Feature 027 addition. Distinct model from PendingFeedbackEntry because + the schemas differ: answered entries carry `answer`, `origin`, and + `authority` fields that are meaningless for still-pending questions. + An auditor reading the report can iterate `answered_feedback` to see + how every user-judgment value was obtained (SC-006). + """ + + control_id: str + context_key: str + question: str + answer: str + origin: str + authority: Literal["asserted"] = "asserted" + resolution_trail: list[ResolutionTrailEntry] = Field(default_factory=list) model_config = ConfigDict(extra="forbid") @@ -54,6 +94,10 @@ class HarnessReport(BaseModel): pending_feedback: list[PendingFeedbackEntry] answer_sources_used: list[str] llm_calls: dict[str, Any] + # Feature 027 additions. Both default to empty so feature-026-era + # constructions still validate. + resolvers_used: list[str] = Field(default_factory=list) + answered_feedback: list[AnsweredFeedbackEntry] = Field(default_factory=list) # exit_class NOT emitted in JSON body per RF-8; kept as an attribute # for the driver but excluded from serialization. exit_class: int = Field(default=0, exclude=True) @@ -137,6 +181,64 @@ def to_markdown(self) -> str: lines.append("None.") lines.append("") + # Resolvers Used (feature 027; only if non-empty) + if self.resolvers_used: + lines.append("## Resolvers Used") + lines.append("") + for resolver_name in self.resolvers_used: + lines.append(f"- {resolver_name}") + lines.append("") + + # Answered Feedback (feature 027; only if non-empty) + if self.answered_feedback: + lines.append("## Answered Feedback") + lines.append("") + for entry in self.answered_feedback: + lines.append( + f"- **{entry.control_id}** -- {entry.context_key}", + ) + lines.append(f" - Question: {entry.question}") + lines.append( + f" - Answered: `{entry.answer}` " + f"(origin: {entry.origin}, authority: {entry.authority})", + ) + if entry.resolution_trail: + lines.append(" - Resolution trail:") + for trail_entry in entry.resolution_trail: + detail = ( + f" -- {trail_entry.error_summary}" + if trail_entry.error_summary + else "" + ) + lines.append( + f" - `{trail_entry.resolver_name}`: " + f"{trail_entry.outcome}{detail}", + ) + lines.append("") + + # Pending Feedback (feature 027 -- render trails if present) + if self.pending_feedback: + lines.append("## Pending Feedback") + lines.append("") + for pending in self.pending_feedback: + lines.append( + f"- **{pending.control_id}** -- {pending.context_key}", + ) + lines.append(f" - Question: {pending.question}") + if pending.resolution_trail: + lines.append(" - Resolution trail:") + for trail_entry in pending.resolution_trail: + detail = ( + f" -- {trail_entry.error_summary}" + if trail_entry.error_summary + else "" + ) + lines.append( + f" - `{trail_entry.resolver_name}`: " + f"{trail_entry.outcome}{detail}", + ) + lines.append("") + # LLM Calls (RF-6) lines.append("## LLM Calls") lines.append("") @@ -165,4 +267,9 @@ def _format_control_line(control: dict[str, Any], compact: bool = False) -> str: return f"- {control_id} {status} ({authority}) -- {message}" -__all__ = ["HarnessSummary", "PendingFeedbackEntry", "HarnessReport"] +__all__ = [ + "HarnessSummary", + "PendingFeedbackEntry", + "AnsweredFeedbackEntry", + "HarnessReport", +] diff --git a/packages/darnit/src/darnit/harness/resolver_discovery.py b/packages/darnit/src/darnit/harness/resolver_discovery.py new file mode 100644 index 0000000..95d8752 --- /dev/null +++ b/packages/darnit/src/darnit/harness/resolver_discovery.py @@ -0,0 +1,147 @@ +"""Entry-point discovery for QuestionResolver plugins (feature 027 T017). + +Resolves the `darnit.question_resolvers` entry-point group via +`importlib.metadata`. Matches the pattern used by `darnit.frameworks` +(compliance implementations). + +Contract: + - QR-14..QR-16 from contracts/question-resolver-protocol.md + - Research decision R2 (importlib.metadata, lazy, warn-and-skip on failure) +""" + +from __future__ import annotations + +from importlib import metadata +from typing import Any + +from darnit.core.logging import get_logger +from darnit.harness.question_resolvers import QuestionResolver + +logger = get_logger("harness.resolver_discovery") + +ENTRY_POINT_GROUP = "darnit.question_resolvers" + + +def discover_registered_resolvers() -> dict[str, QuestionResolver]: + """Discover resolvers registered via Python entry points. + + Returns a dict mapping entry-point name -> resolver instance. Failures + during `ep.load()` or the follow-up `isinstance()` check log a WARNING + and are skipped; other entry points still register. + """ + found: dict[str, QuestionResolver] = {} + + try: + eps = metadata.entry_points(group=ENTRY_POINT_GROUP) + except TypeError: + # Python 3.9 shape (kwargs not supported); we require 3.11+ so this + # should not fire, but defensive fallback: filter manually. + eps = [ + ep + for ep in metadata.entry_points() # type: ignore[call-arg] + if getattr(ep, "group", None) == ENTRY_POINT_GROUP + ] + + for ep in eps: + try: + factory = ep.load() + except Exception as exc: # noqa: BLE001 + logger.warning( + "resolver entry point %r failed to load: %s: %s", + ep.name, + type(exc).__name__, + exc, + ) + continue + + try: + instance = factory() + except Exception as exc: # noqa: BLE001 + logger.warning( + "resolver entry point %r factory raised: %s: %s", + ep.name, + type(exc).__name__, + exc, + ) + continue + + if not isinstance(instance, QuestionResolver): + logger.warning( + "resolver entry point %r produced %s which does not satisfy " + "the QuestionResolver Protocol", + ep.name, + type(instance).__name__, + ) + continue + + found[ep.name] = instance + + return found + + +def build_default_resolver_chain( + interactive: bool, + *, + allow_external_resolvers: bool = False, +) -> list[Any]: + """Build the CLI's canonical resolver chain. + + Contract QR-21 + research.md R7: + - Discover all entry-point registered resolvers. + - If interactive=True: put `interactive_terminal` first (raise if absent). + - Append every OTHER resolver in discovery order. + + PR #367 review Constitution IV fix: third-party resolvers whose + answers become ``authority="asserted"`` are gated behind + ``allow_external_resolvers``. Constitution Principle IV allows only + human confirmation to produce an asserted value; a resolver that + silently loads from an installed Python package is not that. The + fleet operator must opt in explicitly (via ``--allow-external-resolvers`` + on the CLI or the keyword here) before external resolvers enter the + chain. Without it, only the interactive terminal (if requested) runs. + + Returns a list of resolver instances. The list may be empty when + interactive=False and no third-party resolvers are opted in. + """ + all_resolvers = discover_registered_resolvers() + + # Always remove `interactive_terminal` from the general pool -- it's + # only included in the chain when interactive=True is explicit. A + # fleet operator running non-interactive should never trigger a prompt. + terminal = all_resolvers.pop("interactive_terminal", None) + + chain: list[Any] = [] + + if interactive: + if terminal is None: + from darnit.harness.driver import HarnessSetupError + + raise HarnessSetupError( + "interactive channel unavailable " + "(the `interactive_terminal` resolver entry point is not registered)", + ) + chain.append(terminal) + + # External resolvers only enter the chain when the operator opted in. + # Without the opt-in, log at INFO which resolvers were discovered but + # NOT invoked, so a fleet operator can see them and choose to enable. + if allow_external_resolvers: + for _name, resolver in all_resolvers.items(): + chain.append(resolver) + elif all_resolvers: + logger.info( + "harness: %d external resolver(s) discovered but NOT invoked: %s. " + "Pass --allow-external-resolvers to include them (their answers " + "will be recorded with authority='asserted').", + len(all_resolvers), + sorted(all_resolvers.keys()), + ) + + return chain + + +__all__ = ( + "discover_registered_resolvers", + "build_default_resolver_chain", + "ENTRY_POINT_GROUP", +) diff --git a/specs/027-interactive-resolvers/checklists/requirements.md b/specs/027-interactive-resolvers/checklists/requirements.md new file mode 100644 index 0000000..05f3a06 --- /dev/null +++ b/specs/027-interactive-resolvers/checklists/requirements.md @@ -0,0 +1,66 @@ +# Specification Quality Checklist: Interactive Question Resolvers + +**Purpose**: Validate specification completeness and quality before proceeding to planning + +**Created**: 2026-08-07 + +**Feature**: [spec.md](../spec.md) + +## Content Quality + +- [X] No implementation details (languages, frameworks, APIs) +- [X] Focused on user value and business needs +- [X] Written for non-technical stakeholders +- [X] All mandatory sections completed + +## Requirement Completeness + +- [X] No [NEEDS CLARIFICATION] markers remain +- [X] Requirements are testable and unambiguous +- [X] Success criteria are measurable +- [X] Success criteria are technology-agnostic (no implementation details) +- [X] All acceptance scenarios are defined +- [X] Edge cases are identified +- [X] Scope is clearly bounded +- [X] Dependencies and assumptions identified + +## Feature Readiness + +- [X] All functional requirements have clear acceptance criteria +- [X] User scenarios cover primary flows +- [X] Feature meets measurable outcomes defined in Success Criteria +- [X] No implementation details leak into specification + +## Notes + +- Spec is scoped tightly to the collection mechanism. It intentionally does NOT introduce automatic re-audit on interactively supplied answers (the "no re-audit after collect" MVP policy from feature 026 remains in effect); that is a separate follow-up. +- The load-bearing property is the extensibility of the `QuestionResolver` Protocol. SC-002 is the enforceable statement of that property (a third-party resolver can be added without editing `packages/darnit/src/darnit/harness/`). +- Constitution IV interaction: interactive answers carry `authority: "asserted"`. This is documented in FR-009 + SC-003 and echoed in the Assumptions block. Deliberate: a human answering at a terminal IS confirmation, but never dispositive; the audit remains conservative. + +## Clarification Session Log + +Five clarifications were recorded during the 2026-08-07 clarify session: + +1. **Registration mechanism** -> Hybrid: entry points + direct injection (FR-014). +2. **Prompt output stream** -> `/dev/tty` for MVP; pluggable output channel as a resolver-internal seam so future variants can route to event streams / log sinks (FR-004, FR-004a; SC-005 updated). +3. **Progress display during interactive collect** -> Bookend lines only on stderr; position indicator inside the prompt payload; ordinary [N/M] audit lines suppressed for the interactive phase (FR-013a, FR-013b; SC-008). +4. **Programmatic empty-Answer semantics** -> Symmetric with interactive: empty/whitespace-only Answer collapses to skip; no supported way to assert an empty value (FR-006a; Answer entity updated). +5. **Resolution trail in the report** -> Full trail: per-question `resolution_trail` list with `answered`/`skipped`/`errored` outcomes for every resolver that was offered the question (FR-015a; ResolutionTrailEntry added to Key Entities; SC-009). + +All five were HIGH or MEDIUM-HIGH impact. No question was deferred; no [NEEDS CLARIFICATION] markers were introduced or remain. + +## /speckit-analyze findings applied + +The 2026-08-08 analyze pass surfaced 11 findings (0 CRITICAL, 3 HIGH, 4 MEDIUM, 4 LOW). All 7 HIGH+MEDIUM findings were remediated in-line by editing spec.md, plan.md, data-model.md, tasks.md, and two contracts: + +- **C1 (authority=asserted)**: `Answer.authority: Literal["asserted"] = "asserted"` enforces at construction time; `PendingFeedbackEntry.answer_authority: Literal["asserted"] | None` surfaces in the report; cross-field validator; test coverage added to T005, T014, T016. +- **C2 (per-resolver timeout)**: `HarnessRun.per_resolver_timeout_s: float | None` (default None). `asyncio.wait_for` wrapping in T009; timeout test in T014. +- **C3 (bookend count)**: explicit assertion added to T014. +- **M1 (empty-Answer skip)**: explicit unit test added to T005 and driver test in T014. +- **M2 (no values in logs)**: explicit test in T014 using distinctive-value substring assertion. +- **M3 (SC-006 reconstructibility)**: external-consumer JSON reconstruction test added to T016. +- **M4 (Constitution IV alignment reasoning)**: one-line explanatory note added to spec.md Assumptions block. + +The four LOW findings (L1: T003 laziness note; L2: T022 reframe; L3: branch-base note; L4: T029 cross-reference) were all applied as documentation nits. + +Coverage after remediation: 30/30 requirements have >= 1 task; 30/30 have >= 1 test task. No unmapped tasks; no constitution violations. diff --git a/specs/027-interactive-resolvers/contracts/interactive-resolver-behavior.md b/specs/027-interactive-resolvers/contracts/interactive-resolver-behavior.md new file mode 100644 index 0000000..720086f --- /dev/null +++ b/specs/027-interactive-resolvers/contracts/interactive-resolver-behavior.md @@ -0,0 +1,79 @@ +# Contract: `InteractiveTerminalResolver` Behavior + +**Feature**: 027-interactive-resolvers | **Consumers**: fleet operators running `darnit harness --interactive` and tests exercising the terminal behavior. + +The Protocol contract in `question-resolver-protocol.md` covers what ALL resolvers must do. This contract covers the specific behavior of the reference implementation shipping in darnit-core. + +## 1. Registration + naming + +- **IR-1**: The resolver's `name` is the literal string `"interactive_terminal"`. +- **IR-2**: The resolver is registered by darnit-core itself as an entry point in `darnit.question_resolvers`. Operators do not need to install a separate package. +- **IR-3**: The CLI flag `--interactive` on `darnit harness` is equivalent to placing this resolver at the head of the resolver chain. + +## 2. Channel + +- **IR-4**: The resolver writes prompts to and reads answers from `/dev/tty`. Both directions use the same file descriptor pair. +- **IR-5**: The resolver MUST NOT write to `sys.stdout` or `sys.stderr` at any point during `resolve()`. Feature-026 stream contracts (stdout = report body, stderr = progress + exit summary) are preserved. +- **IR-6**: If `/dev/tty` cannot be opened (detached process, unusual chroot, non-POSIX platform), the resolver raises `HarnessSetupError` at first invocation. The CLI translates this to exit code 2 (`SETUP_ERROR`) with a stderr summary naming the missing channel. + +## 3. Availability guard (fail-fast) + +- **IR-7**: `cmd_harness` with `--interactive` MUST verify BOTH conditions BEFORE any control runs: + - `sys.stdin.isatty()` returns True + - `open("/dev/tty", "r+")` succeeds +- **IR-8**: Either check failing results in exit code 2 within 2 seconds. Stderr summary includes the phrase `interactive channel unavailable` and identifies whether stdin-not-TTY or `/dev/tty`-not-openable was the cause. +- **IR-9**: The guard exists so a CI environment invoking `--interactive` cannot silently degrade to a run that skips every question. + +## 4. Prompt payload + +- **IR-10**: Each prompt payload written to `/dev/tty` MUST include, in order: + 1. A blank line (separator from any previous prompt) + 2. A position header: `[N of M]` where `N` is the 1-indexed question number and `M` is the total pending + 3. The control identifier on its own line (e.g. `STAGE1-REF-SECURITY-01`) + 4. The question text, wrapped to a reasonable line width (target: 80 chars, but wrapping is a plan-phase detail) + 5. Any control-level help text available, indented (target: 2 spaces) and preceded by a `Help:` marker line + 6. A prompt line ending in `> ` (chevron + single space) with no trailing newline, so the operator's input appears on the same line + +- **IR-11**: The prompt payload MUST NOT include: + - The `Answer.value` from any previously answered question (privacy) + - The API key from `ANTHROPIC_API_KEY` or any environment variable + - The resolver's `name` (redundant; the operator knows they're in `--interactive` mode) + +- **IR-12**: The exact byte sequence of a prompt for a given `(control_id, question, help_text, position)` tuple is LOCKED by a golden-file test at implementation time. Format changes require updating the golden. + +## 5. Input handling + +- **IR-13**: The resolver reads one line at a time via `readline()`. The trailing newline is stripped. The stripped result is the operator's raw input. +- **IR-14**: `.strip()` is applied to the raw input to normalize surrounding whitespace. +- **IR-15**: If the stripped input is empty (Enter pressed, or only whitespace typed), the resolver returns `None` (SKIP). Per FR-006 and FR-006a. No `Answer` is constructed. +- **IR-16**: If the stripped input is non-empty, the resolver returns `Answer(value=, origin="interactive_terminal")`. The `value` is the stripped string; leading/trailing whitespace is not preserved (they are almost certainly typos). + +## 6. Interrupt handling + +- **IR-17**: `readline()` raising `KeyboardInterrupt` (Ctrl+C) causes the resolver to raise `InteractiveAborted`. The driver's collect loop catches this and stops offering further questions to any resolver. +- **IR-18**: `readline()` returning empty string (EOF / Ctrl+D) is treated identically to Ctrl+C -- raises `InteractiveAborted`. +- **IR-19**: The resolver DOES NOT install any signal handlers. The `KeyboardInterrupt` machinery is Python's default; the resolver just reacts to what surfaces from `readline()`. + +## 7. Lifecycle + +- **IR-20**: The `/dev/tty` file handle is opened lazily on first `resolve()` call, not in `__init__`. This allows tests to construct the resolver without touching `/dev/tty`. +- **IR-21**: The resolver exposes a `close()` method that closes the `/dev/tty` handle. `_collect_unanswered` calls it after finishing the interactive phase (or on `InteractiveAborted`). +- **IR-22**: Calling `close()` twice is a no-op (idempotent). Calling `resolve()` after `close()` raises `RuntimeError`. + +## 8. Test-injectable streams + +- **IR-23**: `InteractiveTerminalResolver(input_stream=..., output_stream=...)` uses the provided streams verbatim. `/dev/tty` is NOT opened when either argument is non-`None`. +- **IR-24**: Test code SHOULD pass `io.StringIO` for both streams. Writing to `output_stream` and reading from `input_stream` behaves identically to writing to and reading from `/dev/tty`. +- **IR-25**: Neither `input_stream` nor `output_stream` is a supported production configuration. The two-argument constructor exists exclusively for tests. + +## 9. Trail contribution + +- **IR-26**: An interactively answered question produces one trail entry with `outcome: "answered"` and `Answer.origin: "interactive_terminal"`. +- **IR-27**: A skipped question (empty input, or whitespace-only) produces one trail entry with `outcome: "skipped"` and no `error_summary`. +- **IR-28**: Ctrl+C mid-question produces one trail entry with `outcome: "skipped"` for that question. Questions the driver never got to offer (because collection was aborted) get NO trail entry -- they simply remain pending in the report with an empty `resolution_trail`. + +## 10. Non-goals for MVP + +- **IR-29**: No line editing (arrow keys, history, tab-completion). Vanilla `readline()`; the operator types and hits Enter. +- **IR-30**: No colored output, no ANSI escapes, no `rich`/`click`. Terminal ergonomics beyond plain text are out of scope. +- **IR-31**: No confirmation prompt ("You entered X, confirm? y/n"). The operator's Enter is the confirmation. If they typo, they can leave the question pending in a later run by editing `.project/project.yaml` -- this feature is about first-pass collection, not correction. diff --git a/specs/027-interactive-resolvers/contracts/question-resolver-protocol.md b/specs/027-interactive-resolvers/contracts/question-resolver-protocol.md new file mode 100644 index 0000000..3f5ca3b --- /dev/null +++ b/specs/027-interactive-resolvers/contracts/question-resolver-protocol.md @@ -0,0 +1,81 @@ +# Contract: `QuestionResolver` Protocol + +**Feature**: 027-interactive-resolvers | **Consumers**: third-party resolver authors (A2A, GitHub-issue, Slack, webhook, custom) and the harness driver. + +This document is the CONTRACT for the `QuestionResolver` Protocol. External implementers can conform to it to plug into `darnit harness`. + +## 1. Shape + +```python +from typing import Protocol, runtime_checkable + +@runtime_checkable +class QuestionResolver(Protocol): + name: str + async def resolve(self, question: FeedbackQuestion) -> Answer | None: ... +``` + +- **QR-1**: A resolver MUST expose a `name: str` attribute (class-level or instance-level). The `name` MUST be stable across a run and unique within the resolver chain. +- **QR-2**: A resolver MUST expose an async method `resolve(question: FeedbackQuestion) -> Answer | None`. +- **QR-3**: A resolver MAY carry additional attributes and methods; the harness ignores them. +- **QR-4**: A resolver MUST pass `isinstance(instance, QuestionResolver)` (verified by the harness at first invocation). + +## 2. Return-value semantics + +- **QR-5**: Returning `None` means "I have no answer for this question." The question stays pending; the trail entry is `outcome: "skipped"`. +- **QR-6**: Returning `Answer(value="...", origin="...")` with a non-empty, non-whitespace-only `value` means "here is the answer." The question is marked answered; the trail entry is `outcome: "answered"`. +- **QR-7**: Returning `Answer(value="")` or `Answer(value=" ")` (whitespace-only) is EQUIVALENT to returning `None`. The harness collapses these to skip. This is enforced at the driver layer -- resolver authors need not defensively check for empty values. +- **QR-8**: Returning a non-`Answer` non-`None` object is undefined behavior. The harness will treat it as an error (trail entry `outcome: "errored"`, `error_summary` naming the wrong return type). + +## 3. Exception semantics + +- **QR-9**: A resolver's `resolve()` MAY raise. The harness catches all exceptions except `InteractiveAborted`, logs a warning, produces a trail entry with `outcome: "errored"` and a redacted, 200-char-truncated `error_summary`, and CONTINUES to the next resolver. +- **QR-10**: A resolver MUST NOT catch `KeyboardInterrupt` inside `resolve()` unless it is the interactive terminal resolver (which converts it to `InteractiveAborted`). Programmatic resolvers letting `KeyboardInterrupt` propagate is preferred; the driver's collect loop handles interruption uniformly. +- **QR-11**: A resolver SHOULD strip credential material from any exception it raises before it propagates. The harness's `_redact_secrets` pass is a safety net, not a substitute for resolver-side hygiene. + +## 4. Timing semantics + +- **QR-12**: A resolver's `resolve()` SHOULD return promptly for programmatic sources; the harness does not impose a per-resolver timeout in the MVP. Third-party resolvers with long-running side effects (GH issue polling, Slack DM wait) SHOULD implement their own internal timeout and return `None` if the source can't answer in time. +- **QR-13**: The interactive terminal resolver has NO timeout by design -- a human may take arbitrary time to respond. Ctrl+C is the operator's abort signal. + +## 5. Registration mechanisms + +Two mechanisms, both supported (hybrid decision from clarify Q1): + +### 5.a Entry point (for third-party packages) + +- **QR-14**: A third-party package SHOULD declare a Python entry point in the group `darnit.question_resolvers`. Example `pyproject.toml`: + + ```toml + [project.entry-points."darnit.question_resolvers"] + my_gh_issue_resolver = "my_pkg.resolvers:build_gh_issue_resolver" + ``` + +- **QR-15**: The referenced callable MUST accept zero arguments and return a `QuestionResolver` instance. +- **QR-16**: Discovery is lazy at CLI startup. Failures during `ep.load()` or the subsequent `isinstance` check log a warning and skip that entry point; other resolvers in the group still register successfully. + +### 5.b Direct injection (for tests and library consumers) + +- **QR-17**: A library consumer MAY inject resolvers directly via `HarnessRun(question_resolvers=[MyResolver(), ...])`. This bypasses discovery entirely. +- **QR-18**: Test code SHOULD use direct injection with `MockQuestionResolver` fixtures. No wheel or entry-point setup is required for tests. + +## 6. Ordering + composition + +- **QR-19**: The resolver chain runs AFTER the `AnswerSource` chain from feature 026. Any question resolved by an `AnswerSource` (project.yaml, `--answers` file) never reaches the resolver chain. +- **QR-20**: Resolvers run in the order they appear in `HarnessRun.question_resolvers`. First non-None wins for a given question; subsequent resolvers are not offered that question. +- **QR-21**: The CLI's `--interactive` flag registers `interactive_terminal` at the HEAD of the chain. Other entry-point resolvers follow in `importlib.metadata` discovery order. + +## 7. Provenance surfacing + +- **QR-22**: Every `Answer` a resolver produces MUST have an `origin` field. The harness does not synthesize one; resolvers set it explicitly. Convention: `origin` starts with the resolver's `name` and may be extended for adapter-specific detail (e.g. `gh_issue_42_comment_3`). +- **QR-23**: The harness records the full trail per question (see `resolution-trail-schema.md`). Resolvers do NOT populate the trail themselves -- the driver does, based on what each resolver returned or raised. + +## 8. Constitution IV compatibility + +- **QR-24**: Every `Answer` a resolver produces carries `authority: "asserted"` -- fixed as a `Literal["asserted"]` on the `Answer` Pydantic model with a default value. Resolvers do NOT need to set it explicitly; constructing `Answer(value="v", origin="o")` is enough. Attempting to construct `Answer(authority="dispositive")` or any other value raises a Pydantic `ValidationError` at construction time. This makes the FR-009 safety property a physical constraint of the type, not a policy the driver has to remember to apply. +- **QR-25**: A resolver MUST NOT infer values from heuristics and return them as `Answer` objects without an explicit human (or explicit external system) speaking. Detection-only "candidate" behavior belongs in an `AnswerSource` (with `allow_sieve_hints`), not in a `QuestionResolver`. This constraint is not enforceable at the type level; it is a contract obligation on resolver authors. + +## 9. Version stability + +- **QR-26**: The Protocol shape defined in this contract is v1. Backwards-incompatible changes (removing `name`, renaming `resolve`, changing return type) constitute a new feature-level spec change, not a minor evolution. +- **QR-27**: Additive changes (optional new methods with default behaviors, new fields on `Answer`) MAY happen within v1 as long as third-party resolvers that don't implement them continue to work. diff --git a/specs/027-interactive-resolvers/contracts/resolution-trail-schema.md b/specs/027-interactive-resolvers/contracts/resolution-trail-schema.md new file mode 100644 index 0000000..9fbe00f --- /dev/null +++ b/specs/027-interactive-resolvers/contracts/resolution-trail-schema.md @@ -0,0 +1,101 @@ +# Contract: `resolution_trail` Schema in `HarnessReport` + +**Feature**: 027-interactive-resolvers | **Consumers**: report readers, downstream aggregators, CI dashboards, audit-trail verification tools. + +## 1. Location in the report + +- **RT-1**: Every `PendingFeedbackEntry` in `HarnessReport.pending_feedback` gains a `resolution_trail` field. Every entry, always emitted (even when empty), so consumers can rely on the shape. +- **RT-2**: The report gains a top-level `resolvers_used: list[str]` field. Contents: the `name` of every resolver that was configured for the run, in chain order. +- **RT-2a**: Every `PendingFeedbackEntry` gains an `answer_authority: "asserted" | null` field. Set to `"asserted"` whenever `answered == true`; `null` otherwise. Enforces FR-009 / SC-003 at the report shape level -- downstream consumers can filter for `answer_authority == "asserted"` to identify human-provided values without inspecting the trail. + +## 2. `resolution_trail` field shape + +Type: `list[ResolutionTrailEntry]` + +Empty list when no resolver was offered the question (e.g., the question was answered by an `AnswerSource` before reaching the resolver chain, OR no resolvers were configured for the run). + +Non-empty list contains one entry per resolver that was offered the question, in the order they were offered. + +## 3. `ResolutionTrailEntry` JSON shape + +```json +{ + "resolver_name": "interactive_terminal", + "outcome": "answered", + "error_summary": null +} +``` + +- **RT-3**: `resolver_name: string` -- the `name` attribute of the resolver. Non-empty. +- **RT-4**: `outcome: string` -- one of `"answered"`, `"skipped"`, `"errored"`. Closed set. +- **RT-5**: `error_summary: string | null` -- present ONLY when `outcome == "errored"`. Contains a redacted, 200-char-truncated `str(exc)` of the exception the resolver raised. `null` otherwise. + +## 4. Cross-field invariants + +- **RT-6**: `outcome == "errored"` implies `error_summary` is a non-empty string. +- **RT-7**: `outcome == "answered"` implies the parent `PendingFeedbackEntry.answered == true`, `answer` is set, and `answer_authority == "asserted"`. +- **RT-8**: Exactly zero or one entries in `resolution_trail` for a given `PendingFeedbackEntry` have `outcome == "answered"` (first non-None wins, no subsequent resolvers were offered). +- **RT-9**: If any entry has `outcome == "answered"`, it is the LAST entry in the list. +- **RT-9a**: `PendingFeedbackEntry.answered == false` implies `answer_authority == null`. The two fields are cross-validated by a Pydantic model validator; a report with `answered: false, answer_authority: "asserted"` is a schema violation. + +## 5. Redaction guarantee + +- **RT-10**: `error_summary` MUST pass through the same `_redact_secrets` regex table used by feature 026's `_dispatch_llm_step`. Credential patterns (`sk-ant-*`, `Authorization: Bearer *`, `x-api-key: *`, `api_key=*`) are replaced with placeholder strings before the trail entry is constructed. +- **RT-11**: `error_summary` is truncated to 200 characters AFTER redaction. Truncation is a hard character bound; no attempt is made to preserve word boundaries. + +## 6. Markdown rendering + +`HarnessReport.to_markdown()` renders the trail as a nested list under each pending question: + +```markdown +### Pending questions + +- **OSPS-GV-01.01** -- security_contact + - Question: Who is the security contact for this project? + - Resolution trail: + - `interactive_terminal`: skipped + - `gh_issue_comment`: errored -- HTTP 404 on repo lookup + - `slack_dm`: answered +``` + +Empty trails are omitted in Markdown (no `Resolution trail:` header) to avoid noise. JSON always emits the field. + +## 7. Backwards compatibility + +- **RT-12**: A HarnessReport JSON produced by feature 027 is a strict SUPERSET of one produced by feature 026 alone. Consumers reading a 026-era report will not see `resolution_trail` or `resolvers_used`; consumers reading a 027-era report will see them (possibly empty). +- **RT-13**: A consumer that ignores unknown fields on `PendingFeedbackEntry` (Pydantic `extra="allow"` or plain dict access with `.get()`) will process both eras without change. +- **RT-14**: A consumer with `extra="forbid"` on their own model of `PendingFeedbackEntry` will need to add `resolution_trail` and `resolvers_used` fields. This is documented as a schema-evolution break for that specific consumer style; it is not enforced by darnit. + +## 8. Example: three-resolver trail + +Question offered to three resolvers -- first errors, second skips, third answers: + +```json +{ + "control_id": "OSPS-GV-01.01", + "context_key": "security_contact", + "question": "Who is the security contact for this project?", + "answered": true, + "answer": "security@example.com", + "answer_authority": "asserted", + "resolution_trail": [ + { + "resolver_name": "gh_issue_comment", + "outcome": "errored", + "error_summary": "HTTP 404 while fetching /repos/foo/bar: repository not found" + }, + { + "resolver_name": "interactive_terminal", + "outcome": "skipped", + "error_summary": null + }, + { + "resolver_name": "slack_dm", + "outcome": "answered", + "error_summary": null + } + ] +} +``` + +An auditor reading this can reconstruct: "we asked GitHub first (errored, no such repo), then the terminal (operator skipped), then Slack (came back with an answer)." All in the report; no log spelunking required. diff --git a/specs/027-interactive-resolvers/data-model.md b/specs/027-interactive-resolvers/data-model.md new file mode 100644 index 0000000..52bec13 --- /dev/null +++ b/specs/027-interactive-resolvers/data-model.md @@ -0,0 +1,198 @@ +# Phase 1 Data Model: Interactive Question Resolvers + +**Feature**: 027-interactive-resolvers | **Date**: 2026-08-08 + +## 1. `QuestionResolver` (Protocol) + +Module: `packages/darnit/src/darnit/harness/question_resolvers.py` + +```python +@runtime_checkable +class QuestionResolver(Protocol): + name: str + async def resolve(self, question: FeedbackQuestion) -> Answer | None: ... +``` + +**Fields / methods**: + +- `name: str` -- stable identifier for the resolver. Used in log lines, `Answer.origin`, and `ResolutionTrailEntry.resolver_name`. Convention: snake_case, matches the resolver's entry-point name where applicable (e.g. `interactive_terminal`, `gh_issue_comment`). +- `resolve(question) -> Answer | None` (async) -- given one pending feedback question, produce an answer or return None. Empty and whitespace-only `Answer` values are collapsed to skip by the driver per FR-006a; resolver authors need not defensively check for them. + +**Validation rules**: none at Protocol level -- `@runtime_checkable` only checks method / attribute presence. Actual conformance (async signature, return type) is validated at first invocation by the driver. + +**Lifecycle**: resolvers are constructed once at CLI startup (or once at test time), then reused for the whole run. Resolvers MAY hold internal state across `resolve()` calls but MUST NOT rely on question ordering. + +**Test conformance**: `MockQuestionResolver(name="mock", answer=Answer(value="v", origin="mock"))` is provided in the test fixtures; passes `isinstance(mock, QuestionResolver)`. + +## 2. `Answer` + +Module: `packages/darnit/src/darnit/harness/question_resolvers.py` + +```python +class Answer(BaseModel): + value: str + origin: str + authority: Literal["asserted"] = "asserted" +``` + +**Fields**: + +- `value: str` -- the string value the resolver produces. Non-empty, non-whitespace-only invariant is enforced at the driver layer, not on the model itself (see below). +- `origin: str` -- provenance string, typically the resolver's `name`. May be more specific for adapters (e.g. `gh_issue_42_comment_3` instead of just `gh_issue_comment`). +- `authority: Literal["asserted"] = "asserted"` -- fixed to `"asserted"` at the model level. FR-009 says every answer produced by a resolver carries `authority: "asserted"`; enforcing this via a `Literal` type with a fixed default means a resolver author physically cannot construct an `Answer` with a different authority. Consistent with feature 025's authority model where `asserted` denotes "a human said so." + +**Validation rules**: Pydantic `BaseModel` with `extra="forbid"`. No `min_length` on `value` at the model level -- the driver collapses empty/whitespace-only to skip so the corner is handled once, in one place, symmetric across all resolvers (interactive, programmatic, future). The `authority` field's `Literal["asserted"]` constraint makes any attempt to set another value a Pydantic validation error at construction time. + +**Why not `min_length=1`?**: We considered making empty `Answer` a construction-time error. Rejected in Q4 of clarify: a resolver author might legitimately want to log "I tried" via a trail-visible skip; forcing them into `None` vs. `Answer("")` at the Protocol level makes their code more error-prone, not less. The driver layer is the single choke point. + +## 3. `ResolutionTrailEntry` + +Module: `packages/darnit/src/darnit/harness/question_resolvers.py` + +```python +class ResolutionTrailEntry(BaseModel): + resolver_name: str + outcome: Literal["answered", "skipped", "errored"] + error_summary: str | None = None +``` + +**Fields**: + +- `resolver_name: str` -- the `name` attribute of the resolver that produced this entry. +- `outcome: Literal["answered", "skipped", "errored"]` -- closed set (FR-015a). No other values permitted. +- `error_summary: str | None` -- present iff `outcome == "errored"`. Contains the exception `str(exc)` after passing through feature 026's `_redact_secrets`, truncated to 200 characters. `None` for `answered` and `skipped`. + +**Validation rules**: Pydantic `BaseModel` with `extra="forbid"`. Cross-field: `error_summary` is required when `outcome == "errored"` and forbidden otherwise (Pydantic model validator). + +**Ordering**: In the final `PendingFeedbackEntry.resolution_trail: list[ResolutionTrailEntry]`, entries appear in the order the resolvers were offered the question. A reader iterates the list to reconstruct the chain. + +## 4. `FeedbackQuestion` (reused, no changes) + +Reused from feature 026's `HarnessReport.pending_feedback[*]`. Fields as-is: `control_id`, `context_key`, `question`, `answered`, and (after this feature) an optional `resolution_trail` field. + +**No schema changes** to `FeedbackQuestion` itself in this feature. The `resolution_trail` lives at the report level attached to each pending entry, not on the `FeedbackQuestion` sieve-side type. + +## 5. `PendingFeedbackEntry` (updated) + +Module: `packages/darnit/src/darnit/harness/report.py` + +**Additions**: + +```python +class PendingFeedbackEntry(BaseModel): + # ... existing fields from feature 026 ... + resolution_trail: list[ResolutionTrailEntry] = Field(default_factory=list) + answer_authority: Literal["asserted"] | None = None +``` + +- `resolution_trail: list[ResolutionTrailEntry]` -- default empty list. Populated only when the question was offered to at least one resolver. +- `answer_authority: Literal["asserted"] | None` -- present iff `answered == true`. Set to `"asserted"` whenever an answer originates from a `QuestionResolver` (this feature) OR from an `AnswerSource` (feature 026 -- both flavors are human assertions). Absent (`None`) when the question is still pending. Downstream consumers can filter for `answer_authority == "asserted"` to identify human-provided values without inspecting the trail. + +**Validation rules**: `extra="forbid"` retained from feature 026. Both new fields are additive; existing 026-era tests continue to pass because the new fields default sensibly (empty list, `None`). Cross-field: `answer_authority` is required to be `"asserted"` when `answered == true`; forbidden otherwise (Pydantic model validator). + +## 6. `HarnessReport` (updated) + +Module: `packages/darnit/src/darnit/harness/report.py` + +**Additions**: + +```python +class HarnessReport(BaseModel): + # ... existing fields from feature 026 ... + resolvers_used: list[str] = Field(default_factory=list) +``` + +- `resolvers_used: list[str]` -- the `name` of every resolver that was CONFIGURED for this run, in the order they appeared in the chain. Includes resolvers that never received a question (e.g., the terminal resolver was configured but no questions were pending). Empty list for non-interactive runs with no third-party resolvers registered. + +**Serialization**: `to_json()` emits `resolvers_used` unconditionally (empty array when unused). `to_markdown()` emits a "Resolvers used" section immediately after "Answer sources used" when the list is non-empty. + +## 7. `HarnessRun` (updated) + +Module: `packages/darnit/src/darnit/harness/driver.py` + +**Additions**: + +```python +@dataclass +class HarnessRun: + # ... existing fields from feature 026 ... + question_resolvers: list[QuestionResolver] = field(default_factory=list) + per_resolver_timeout_s: float | None = None +``` + +- `question_resolvers: list[QuestionResolver]` -- ordered list of resolvers to try after the AnswerSource chain is exhausted. Default empty list preserves feature 026 behavior (batch-only collection). +- `per_resolver_timeout_s: float | None` -- per-resolver `resolve()` timeout in seconds. Default `None` means no timeout (matches the interactive resolver's documented behavior -- a human may take arbitrary time). When set to a positive float, each `resolver.resolve(question)` call is wrapped in `asyncio.wait_for(..., timeout=per_resolver_timeout_s)`; a timeout is captured as a `ResolutionTrailEntry(outcome="errored", error_summary="resolver timed out after Ns")` and the driver moves on to the next resolver. Fleet operators MAY set this via a CLI flag; the interactive resolver documents that setting it globally is usually inappropriate (an operator at a terminal can't be timed out on the same clock as a webhook resolver). Enforces FR-011. + +**New classmethod**: + +```python +@classmethod +def build_default_resolver_chain(cls, interactive: bool) -> list[QuestionResolver]: + """Factory for the CLI wiring. See research.md R7.""" +``` + +Returns the CLI's canonical chain: interactive terminal first (if `interactive=True`), then every other entry point in `darnit.question_resolvers` in discovery order. + +**Behavior in `_collect_unanswered`**: after the existing AnswerSource-based pass, iterate `question_resolvers` for each remaining pending question per research.md R6. Populate `resolution_trail` on each `PendingFeedbackEntry` (whether or not it ended up answered). + +**No re-audit invariant**: unchanged from feature 026. An interactively supplied answer is captured in the report but does NOT trigger re-evaluation of the associated control's status. Verified by the existing invariant test. + +## 8. `InteractiveAborted` (new exception) + +Module: `packages/darnit/src/darnit/harness/question_resolvers.py` + +```python +class InteractiveAborted(Exception): + """Raised by InteractiveTerminalResolver on Ctrl+C or EOF. + + Signals the driver to stop offering further questions to any resolver + but preserve answers already collected in this collect phase. + """ +``` + +**Behavior**: caught specifically by `_collect_unanswered`; produces a trail entry with `outcome="skipped"` for the currently-being-asked question and terminates the collection loop. Not treated as an internal error; the harness still assembles and returns the report. + +## 9. `InteractiveTerminalResolver` (concrete implementation) + +Module: `packages/darnit/src/darnit/harness/interactive_resolver.py` + +**Shape** (not a data model per se; the concrete class implementing `QuestionResolver`): + +```python +class InteractiveTerminalResolver: + name = "interactive_terminal" + + def __init__( + self, + input_stream: TextIO | None = None, + output_stream: TextIO | None = None, + ) -> None: ... + + async def resolve(self, question: FeedbackQuestion) -> Answer | None: ... + + def _open_tty(self) -> tuple[TextIO, TextIO]: ... # opens /dev/tty when streams are None + def _format_prompt(self, question, position, total) -> str: ... + def close(self) -> None: ... # releases /dev/tty handle after collect +``` + +**Stream contract**: + +- Constructor with default `input_stream=None, output_stream=None` opens `/dev/tty` on first call (or raises `HarnessSetupError` on failure). +- Constructor with explicit streams uses them verbatim (test path). + +**Position indicator**: the resolver receives `question` only; the "N of M" position is passed as an out-of-band argument from the driver's collect loop, threaded into `_format_prompt`. + +## 10. State transitions + +Feature 027 introduces no persistent state. All state is per-run in memory: + +``` +question pending --resolver.resolve() returns Answer(non-empty)--> answered + --resolver.resolve() returns None or empty Answer--> pending (try next resolver) + --resolver.resolve() raises--> pending (try next resolver; error captured in trail) + --resolvers exhausted--> pending (in the report; caller may re-run with --answers) + +driver --Ctrl+C or EOF from interactive--> collection loop terminates, remaining questions stay pending +``` + +Nothing persists between runs. A subsequent audit with the value written to `.project/project.yaml` re-evaluates the associated control. diff --git a/specs/027-interactive-resolvers/plan.md b/specs/027-interactive-resolvers/plan.md new file mode 100644 index 0000000..cb7960c --- /dev/null +++ b/specs/027-interactive-resolvers/plan.md @@ -0,0 +1,127 @@ +# Implementation Plan: Interactive Question Resolvers + +**Branch**: `027-interactive-resolvers` | **Date**: 2026-08-08 | **Spec**: [spec.md](spec.md) + +**Input**: Feature specification from `specs/027-interactive-resolvers/spec.md` (with 5 clarifications from `/speckit-clarify` on 2026-08-07: hybrid entry-point + direct-injection registration; `/dev/tty` for MVP prompt output with a pluggable channel seam; bookend-only progress display; symmetric empty-Answer skip; full `resolution_trail` for auditability). + +## Summary + +Adds `--interactive` to `darnit harness` and a new `QuestionResolver` Protocol that sits DOWNSTREAM of the existing `AnswerSource` chain from feature 026. Any question left uncovered by `.project/project.yaml` + `--answers ` is offered to registered resolvers in order until one returns an answer or the chain is exhausted. + +Ships: +- **New Protocol**: `QuestionResolver` (async, `@runtime_checkable`), distinct from `AnswerSource`. Semantics differ -- `AnswerSource` is passive preloaded-value lookup; `QuestionResolver` is an active resolver that goes and gets an answer somehow. Serves interactive today; A2A / GitHub-issue-comment / Slack / webhook resolvers tomorrow (as separate feature-branches). +- **Reference implementation**: `InteractiveTerminalResolver` prompting on `/dev/tty` (private operator channel, isolated from stdout report stream and stderr progress stream). Empty and whitespace-only input treated as skip. Ctrl+C stops further prompts but preserves already-collected answers. +- **Registration mechanism (hybrid)**: Python entry points under `darnit.question_resolvers` (matching darnit's existing `darnit.implementations` discovery pattern) AND direct injection into `HarnessRun.question_resolvers`. Third-party packages ship a wheel with an entry-point declaration; the harness discovers them at CLI startup. +- **CLI flag**: `--interactive` on `darnit harness`, default off. Registers the terminal resolver at the head of the resolver chain. Non-TTY / no-`/dev/tty` under `--interactive` -> fail-fast `SETUP_ERROR` in under 2s. +- **Auditability**: Per-question `resolution_trail` in the report -- one entry per resolver that was offered the question, with an outcome (`answered` | `skipped` | `errored`). Answers carry `authority: "asserted"` (enforced at the `Answer` model level via `Literal["asserted"]` with a fixed default -- resolver authors physically cannot construct an `Answer` with a different authority) and an `origin` string identifying the resolver. The `PendingFeedbackEntry` also surfaces `answer_authority` alongside `answered`/`answer` so downstream consumers can filter for human-provided values without inspecting the trail. +- **Per-resolver timeout**: `HarnessRun.per_resolver_timeout_s: float | None`. Default `None` (no timeout). When set, each `resolver.resolve()` call is wrapped in `asyncio.wait_for`; a timeout becomes a `ResolutionTrailEntry(outcome="errored", ...)` and the driver moves on. Enforces FR-011. + +Non-scope for this feature: re-audit after collect (feature 026's "no re-audit" MVP policy stays intact); alternative output channels beyond `/dev/tty` (FR-004a designs the seam, the specific event/log adapters are future features). + +## Technical Context + +**Language/Version**: Python 3.11 / 3.12 (workspace targets, unchanged). + +**Primary Dependencies (new)**: None. This feature is stdlib-only on the production surface (`typing.Protocol`, `typing.runtime_checkable`, `importlib.metadata.entry_points`, direct `open("/dev/tty", ...)`, `asyncio`). + +**Primary Dependencies (in use)**: `pydantic >= 2.0` (for the `Answer` and `ResolutionTrailEntry` models). Feature 026 internals: `HarnessRun`, `AnswerResolver`, `HarnessReport`, `PendingFeedbackEntry`. Feature 025 internals: `authority` Literal type. `pytest` for tests. + +**Storage**: Filesystem only (unchanged). Interactive answers land in the same `HarnessReport` artifact feature 026 writes; the report gains `resolvers_used` and per-question `resolution_trail` fields. No new persistent state. + +**Testing**: pytest. `InteractiveTerminalResolver` accepts injectable input/output streams so tests can pass `io.StringIO` in place of `/dev/tty`. Entry-point discovery is exercised via `importlib.metadata`'s test hooks. A `MockQuestionResolver` fixture returns preconfigured answers or raises to exercise the trail's `errored` outcome. + +**Target Platform**: POSIX for MVP (`/dev/tty` availability). Windows is intentionally out of scope for the MVP -- FR-004a's pluggable-channel design accommodates it later without a Protocol change. + +**Project Type**: Additive to the existing `darnit harness` subcommand. Ships in `packages/darnit/src/darnit/harness/`. + +**Performance Goals**: SC-001 -- five questions answered end-to-end in under three minutes at a terminal (target ergonomics). SC-005 -- fail-fast in under two seconds when no operator channel is available. + +**Constraints**: +- **Constitution IV**: an interactive answer is a human confirmation, tagged `authority: "asserted"`. Feature 026's "no re-audit after collect" MVP policy is preserved -- the audit's PASS/FAIL doesn't silently flip based on interactive input; the assertion is captured for a later run. +- **Feature-026 stream contracts unchanged**: stdout carries the report body when `--output` is unset; stderr carries progress + exit summary. Prompts write to `/dev/tty` (a third stream, physically separate on POSIX). +- **API key redaction from feature 026 applies unchanged**: any exception message that surfaces in the `resolution_trail`'s `errored` summary passes through `_redact_secrets` before landing in the report. + +**Scale/Scope**: MVP is one reference resolver + one CLI flag + report additions. Expected slice size: ~400-600 lines net production + ~400-500 lines tests. + +## Constitution Check + +Constitution v1.3.0. Five Core Principles evaluated as gates. + +| Principle | Applicable? | Verdict | Rationale | +|-----------|-------------|---------|-----------| +| I. Plugin Separation | Yes | PASS | `QuestionResolver` Protocol lives in `darnit-core` (`packages/darnit/src/darnit/harness/question_resolvers.py`). Third-party resolvers live outside `packages/darnit/` and register via a new entry-point group `darnit.question_resolvers`, mirroring the existing `darnit.implementations` pattern. SC-002 enforces: a resolver defined outside `packages/darnit/src/darnit/harness/` is invoked without any change to files under that directory. | +| II. Conservative-by-Default | Yes | PASS + REINFORCED | Feature 026's "no re-audit after collect" MVP policy stays. Interactively supplied values are recorded in the report but do NOT silently promote a FAIL to PASS. A control that was FAIL because a value was missing at audit time stays FAIL in this report; a subsequent audit run with the value persisted to `.project/project.yaml` re-evaluates it. Nothing silently changes based on interactive input. | +| III. TOML-First Architecture | No | N/A | No control definitions. No TOML schema changes. | +| IV. Never Guess User Values | Yes | PASS + REINFORCED | This feature exists precisely to elicit human confirmation. The interactive resolver produces answers tagged `authority: "asserted"` -- a human said this, not a heuristic. Empty and whitespace-only inputs (interactive OR programmatic per FR-006a) collapse to skip so no resolver author can accidentally record an assertion of emptiness. The audit trail (`resolution_trail`) makes every resolver attempt visible so an auditor can see how each value was obtained. | +| V. Sieve Pipeline Integrity | No | N/A | This feature runs downstream of the sieve (in the collect phase). The 4-phase pipeline is unchanged. | + +**No violations.** No Complexity Tracking entries required. + +Two positive observations: +- The `QuestionResolver` Protocol seam extends feature 026's fleet-operator framing without a rewrite. Future adapters (A2A, GitHub issue comments, Slack, webhook) plug in as external packages registering via entry point; no darnit-core change per adapter. +- The `resolution_trail` per-question audit surface is a genuine Constitution IV artifact: "how was this value obtained" is now recoverable from the report alone. That property matters more as third-party resolvers accumulate. + +## Project Structure + +### Documentation (this feature) + +```text +specs/027-interactive-resolvers/ ++-- spec.md # /speckit-specify + /speckit-clarify output ++-- plan.md # this file ++-- research.md # Phase 0: architectural decisions ++-- data-model.md # Phase 1: QuestionResolver, Answer, ResolutionTrailEntry ++-- quickstart.md # Phase 1: how to run + verify locally ++-- contracts/ +| +-- question-resolver-protocol.md # Protocol shape + registration contract +| +-- interactive-resolver-behavior.md # /dev/tty, prompt payload, empty/EOF/Ctrl+C +| +-- resolution-trail-schema.md # `resolution_trail` field in HarnessReport JSON ++-- checklists/ +| +-- requirements.md # spec-quality checklist (exists) ++-- tasks.md # /speckit-tasks output (later) +``` + +### Source Code (repository root) + +Everything ships in `darnit-core`. No new package. + +```text +packages/darnit/src/darnit/harness/ ++-- question_resolvers.py # NEW: QuestionResolver Protocol, Answer, ResolutionTrailEntry ++-- interactive_resolver.py # NEW: InteractiveTerminalResolver (POSIX /dev/tty) ++-- resolver_discovery.py # NEW: entry-point discovery for `darnit.question_resolvers` ++-- driver.py # UPDATED: HarnessRun.question_resolvers field, resolver-chain +| # invocation in _collect_unanswered, resolution_trail ++-- report.py # UPDATED: HarnessReport.resolvers_used, PendingFeedbackEntry +| # gains resolution_trail; markdown/json emit trail ++-- exit_codes.py # UNCHANGED ++-- answer_sources.py # UNCHANGED (AnswerSource remains passive-lookup only) + +packages/darnit/src/darnit/cli.py # UPDATED: cmd_harness gains --interactive flag; wires + # discovery + terminal resolver into HarnessRun + +packages/darnit/pyproject.toml # UPDATED: entry-point group declaration for + # `darnit.question_resolvers`; a + # darnit-core-supplied "interactive_terminal" + # entry-point registration for MVP + +tests/darnit/harness/ ++-- test_question_resolvers.py # NEW: Protocol conformance, Answer validation ++-- test_interactive_resolver.py # NEW: prompt format, empty/EOF/Ctrl+C, streams injectable ++-- test_resolver_discovery.py # NEW: entry-point discovery via importlib.metadata ++-- test_resolution_trail.py # NEW: trail population, outcome enum, ordering ++-- test_driver.py # UPDATED: resolver chain invocation + no-reaudit invariant ++-- test_cli.py # UPDATED: --interactive flag; fail-fast on non-TTY ++-- test_report.py # UPDATED: resolution_trail in JSON + Markdown output ++-- fixtures/ + +-- mock_resolver_pkg/ # NEW: external-to-harness package that registers a + # QuestionResolver via entry point; used by + # test_resolver_discovery + test_driver to + # enforce SC-002 (no edits under harness/) +``` + +**Structure Decision**: Additive-only. Three new modules in `packages/darnit/src/darnit/harness/`. Two of those (`question_resolvers.py`, `resolver_discovery.py`) are the reusable substrate; the third (`interactive_resolver.py`) is the reference implementation of the Protocol. `driver.py`, `report.py`, `cli.py` receive small, well-scoped extensions. The fixture package `tests/darnit/harness/fixtures/mock_resolver_pkg/` exists specifically to make SC-002 mechanically enforceable -- a resolver defined outside the harness tree that the CI test suite proves is discoverable. + +## Complexity Tracking + +No violations. This section left intentionally empty. diff --git a/specs/027-interactive-resolvers/quickstart.md b/specs/027-interactive-resolvers/quickstart.md new file mode 100644 index 0000000..d751cff --- /dev/null +++ b/specs/027-interactive-resolvers/quickstart.md @@ -0,0 +1,188 @@ +# Quickstart: Interactive Question Resolvers + +**Feature**: 027-interactive-resolvers | **For**: fleet operators running `darnit harness` interactively, third-party resolver authors adding a custom answer source. + +## Operator: run interactively against a repo + +```bash +# From the darnit workspace +uv run darnit harness /path/to/repo --level 3 --interactive +``` + +What you should see: + +1. Ordinary audit progress lines on stderr (`[N/M] control_id `) for each control, exactly as they appear in a non-interactive run. +2. When the audit finishes, if there are pending feedback questions, exactly ONE line on stderr: + + ``` + harness: starting interactive collection (3 pending questions) + ``` + +3. A prompt on your terminal (via /dev/tty, physically separate from stderr): + + ``` + [1 of 3] + OSPS-GV-01.01 + Who is the security contact for this project? + Help: A person or team that receives vulnerability reports. + > _ + ``` + +4. Type an answer and hit Enter. The prompt moves to `[2 of 3]`. +5. Hit Enter without typing to skip. That question stays pending in the report. +6. Ctrl+C at any point stops further prompting but keeps everything you've already answered. +7. One closing line on stderr: + + ``` + harness: finished interactive collection: 2 answered, 1 skipped + ``` + +8. The report is written per the `--output` and `--format` flags. Each pending question that was offered to the resolver chain shows its `resolution_trail` in the report (JSON always; Markdown when non-empty). + +## Operator: combine `--answers` file and `--interactive` + +`--answers` runs first (values you already know from the yaml file), then interactive fills in the rest: + +```bash +uv run darnit harness /path/to/repo --level 3 --answers ~/known-values.yaml --interactive +``` + +The report's `answer_sources_used` lists both sources; each answer records which source produced it in its `origin` field. + +## Fail-fast behavior + +Piped stdin under `--interactive`: + +```bash +echo "foo" | uv run darnit harness /path/to/repo --interactive +``` + +Expected: exit code 2 within 2 seconds. Stderr summary: + +``` +harness: setup_error: interactive channel unavailable (stdin is not a TTY), exit 2 +``` + +No control runs. This is deliberate -- if a CI runner accidentally sets `--interactive`, we want a loud failure, not a silent skip-everything. + +## Third-party resolver author: write a custom resolver + +Ship a package with a `QuestionResolver`: + +```python +# my_pkg/resolvers.py +from darnit.harness.question_resolvers import Answer, QuestionResolver + +class GHIssueCommentResolver: + name = "gh_issue_comment" + + async def resolve(self, question) -> Answer | None: + # Look up the question's control_id in a GitHub issue, + # scrape the latest maintainer comment, etc. + answer_text = await self._fetch_from_gh(question) + if not answer_text: + return None # nothing found; harness will try the next resolver + return Answer( + value=answer_text, + origin=f"gh_issue_comment:{question.control_id}", + ) + +def build() -> QuestionResolver: + return GHIssueCommentResolver() +``` + +Declare the entry point in your `pyproject.toml`: + +```toml +[project.entry-points."darnit.question_resolvers"] +gh_issue_comment = "my_pkg.resolvers:build" +``` + +Install your package into the same venv as darnit; the harness will discover the resolver automatically at CLI startup. + +Verify: + +```bash +uv pip install /path/to/my_pkg +uv run darnit harness /path/to/repo --interactive --level 3 +``` + +The `harness: starting interactive collection` line will be preceded by an INFO log naming the resolvers configured for this run, including `gh_issue_comment`. The report's `resolvers_used` will list both. + +## Library consumer: inject a resolver directly + +For tests or embedded uses: + +```python +from darnit.harness.driver import HarnessRun +from darnit.harness.question_resolvers import Answer + +class AlwaysAnswerResolver: + name = "always" + async def resolve(self, question): + return Answer(value="constant", origin="always") + +run = HarnessRun( + local_path="/path/to/repo", + level=3, + question_resolvers=[AlwaysAnswerResolver()], +) +report = await run.run() +``` + +No entry-point discovery happens; only the explicitly passed resolvers run. Useful for reproducible test fixtures. + +## Verifying the resolution trail + +For any pending question in the report, the `resolution_trail` shows which resolvers were offered the question and how each responded: + +```json +{ + "pending_feedback": [ + { + "control_id": "OSPS-GV-01.01", + "context_key": "security_contact", + "question": "Who is the security contact for this project?", + "answered": true, + "answer": "security@example.com", + "resolution_trail": [ + {"resolver_name": "gh_issue_comment", "outcome": "skipped", "error_summary": null}, + {"resolver_name": "interactive_terminal", "outcome": "answered", "error_summary": null} + ] + } + ] +} +``` + +An auditor reads this and knows: GitHub had no comment for this question, and the operator typed the answer at the terminal. + +## Common gotchas + +- **`--interactive` in a CI job**: will fail with exit code 2. Use `--answers ` for non-interactive answer collection. +- **Container with no `/dev/tty` node**: same failure, different underlying cause (stdin might be a TTY but `/dev/tty` open fails). Stderr summary distinguishes. +- **Multiple entry points with the same name**: last-write wins in `importlib.metadata` discovery. Don't ship two packages that both register `interactive_terminal`. +- **A resolver hangs indefinitely**: the MVP has no per-resolver timeout for non-interactive resolvers. Third-party resolver authors are responsible for their own timeouts; the harness's total-run timeout (`--total-run-timeout-s`, from feature 026) is the ultimate backstop. +- **The report shows an empty `resolution_trail`**: means the AnswerSource chain (project.yaml + `--answers`) answered the question before it reached the resolver chain. Normal, not a bug. + +## Running the tests + +```bash +uv run pytest tests/darnit/harness/ -q +``` + +Feature-027-specific tests live in: + +- `tests/darnit/harness/test_question_resolvers.py` +- `tests/darnit/harness/test_interactive_resolver.py` +- `tests/darnit/harness/test_resolver_discovery.py` +- `tests/darnit/harness/test_resolution_trail.py` + +Plus additions to `test_driver.py`, `test_cli.py`, `test_report.py`. + +External-fixture package for SC-002 enforcement: + +``` +tests/darnit/harness/fixtures/mock_resolver_pkg/ +``` + +This lives OUTSIDE `packages/darnit/src/darnit/harness/` and registers a resolver via entry point. If a test asserts it is discoverable via `importlib.metadata.entry_points(group="darnit.question_resolvers")` and invoked without any changes under `packages/darnit/src/darnit/harness/`, SC-002 is mechanically enforced. diff --git a/specs/027-interactive-resolvers/research.md b/specs/027-interactive-resolvers/research.md new file mode 100644 index 0000000..b131452 --- /dev/null +++ b/specs/027-interactive-resolvers/research.md @@ -0,0 +1,131 @@ +# Phase 0 Research: Interactive Question Resolvers + +**Feature**: 027-interactive-resolvers | **Date**: 2026-08-08 + +All five load-bearing decisions were resolved in `/speckit-clarify` and are recorded in `spec.md`'s Clarifications section (2026-08-07 session). This research file covers the *technical* how -- the residual mechanics that Phase 1's design work needs to sit on top of. + +## R1. Protocol shape + registration mechanism + +**Decision**: `QuestionResolver` is a `@runtime_checkable` Protocol with (a) a `name: str` class or instance attribute and (b) an async `resolve(question: FeedbackQuestion) -> Answer | None` method. Registration is hybrid per the clarify session: + +- **Entry point** group: `darnit.question_resolvers`. Format matches `darnit.implementations` (existing pattern): `entry-point-name = "package.module:factory_callable"`. The factory returns an instance implementing the Protocol. +- **Direct injection**: `HarnessRun(question_resolvers=[MyResolver(), ...])`. Bypasses discovery entirely -- used by tests and library consumers. + +The CLI resolves an operator-visible flag (`--interactive`) into a resolver by name: the terminal resolver is registered by darnit-core itself as an entry point named `interactive_terminal`, so `--interactive` is equivalent to "look up the `interactive_terminal` entry point and put it at the head of the chain." + +**Rationale**: Matches darnit's existing extension pattern (`ComplianceImplementation`). Allows third-party packages to register without a code change to darnit-core (Constitution I, SC-002). Direct injection is the natural surface for tests -- no need to construct a wheel to test a resolver. + +**Alternatives considered**: +- Direct injection only: rejected because a third party can't ship a wheel that drops into a fleet operator's environment; they'd need to write a wrapper. Fails the SC-002 extensibility test. +- Config-file registration (`.baseline.toml` resolver list by module path): rejected as another config surface with no offsetting benefit; entry points already give us packaged discovery. +- Auto-discovery scanning `sys.path` for `resolvers/*.py`: rejected as too magical; entry points are declared intent. + +## R2. Entry-point discovery mechanics + +**Decision**: Use `importlib.metadata.entry_points(group="darnit.question_resolvers")`. Guard against the Python 3.9 vs 3.10+ API drift by pinning to the 3.10+ shape (returns `EntryPoints` selection object). darnit already requires 3.11+ so this is safe. + +Discovery result: a list of `EntryPoint` objects. For each, `ep.load()` returns the factory callable. Call it with no arguments; expect a `QuestionResolver` instance back. `isinstance(instance, QuestionResolver)` verifies conformance (Protocol is `@runtime_checkable`). + +Discovery runs at CLI startup, before any control iteration. Failures during `ep.load()` for a single entry point log a WARNING with the entry-point name and continue (don't crash the whole run on one malformed third-party wheel). Failure during `isinstance` check is likewise a warning + skip. + +**Rationale**: Matches how `darnit.implementations` is discovered in `packages/darnit/src/darnit/core/discovery.py`. Same behavior on failure (log + continue). No new dependency; `importlib.metadata` is stdlib. + +**Alternatives considered**: +- `pkg_resources` (setuptools legacy): rejected -- deprecated, slower, adds an implicit runtime dep on setuptools. +- Eager import at module load time: rejected -- would tie test isolation to global state; lazy discovery at CLI startup is cleaner. + +## R3. `/dev/tty` mechanics + +**Decision**: Open `/dev/tty` in mode `"r+"` with unbuffered binary or line-buffered text. Concretely: `open("/dev/tty", "r+", buffering=1)` (line-buffered text mode). Read one line at a time via `.readline()`; strip trailing newline; treat empty-after-strip as skip. + +Availability check: at `InteractiveTerminalResolver` construction (or at the first `resolve()` call), attempt `open("/dev/tty", "r+")`. On `OSError` / `FileNotFoundError`, raise a subclass of `HarnessSetupError` so the caller (`cmd_harness`) can translate to exit code 2 with an intelligible stderr summary. + +**Rationale**: `/dev/tty` is the POSIX-standard private operator channel. `readline()` returns the empty string on EOF, giving us a clean Ctrl+D signal (treat identically to Ctrl+C -- stop asking, keep collected). `open` failure names the platform reason directly. + +Ctrl+C handling: `readline()` raises `KeyboardInterrupt`. The resolver catches it, closes the /dev/tty handle, and re-raises a small sentinel exception (`InteractiveAborted`) that the driver's collect loop catches and treats as "stop further prompts, preserve collected answers." Existing signal handlers are not overridden; we just react to the natural KeyboardInterrupt. + +**Alternatives considered**: +- `input()` (writes to stdout, reads from stdin): rejected -- would pollute the report stream (Q2 of clarify). +- `getpass.getpass` (reads from `/dev/tty` on POSIX): rejected -- silent echo, wrong UX for these prompts which are not secrets. +- `curses` full-screen prompt: rejected -- vast overkill and forces a stdlib module that some minimal Python builds omit. +- Raw `os.open("/dev/tty", os.O_RDWR)` + manual read/write: rejected -- gives us nothing over the `open()` file-object wrapper and forces us to reimplement line buffering. + +## R4. Testing `/dev/tty` -- inject streams + +**Decision**: `InteractiveTerminalResolver.__init__` accepts optional `input_stream` and `output_stream` parameters (default: both `None`, meaning "open `/dev/tty`"). Tests pass `io.StringIO` for both. Production code path never sees `StringIO`. + +Prompt-format regression: a golden-file test writes a scripted resolver run to a `StringIO` output stream and asserts the byte-for-byte prompt payload matches an expected fixture. Cheap way to lock in the position-indicator + control-id + question-text + help-text ordering. + +**Rationale**: Same shape as feature 026's `harness_run_factory` fixture -- constructor-time injection lets tests exercise the code without touching real terminals. Matches Python community norms for testing CLI apps. + +**Alternatives considered**: +- Monkey-patch `builtins.open` in tests: rejected -- global patch is fragile; interacts badly with pytest's own capture machinery. +- Use `pexpect` to script a pseudo-terminal: rejected -- brings in a heavyweight test dep; the injectable-stream approach gives 95% of the coverage at 5% of the cost. + +## R5. Progress-line suppression during interactive collect + +**Decision**: The driver's `_collect_unanswered` is the phase where interactive prompts fire. Ordinary per-control `[N/M]` audit-progress lines have already stopped by this point (the audit + LLM continuation loop both finish before collect begins). But the driver DOES emit progress lines from within `_llm_continuation_loop`. We ensure no such lines are emitted from inside the collect phase itself -- collect is silent on `darnit.harness` except for the two bookends specified in FR-013a. + +Implementation shape: `_collect_unanswered` opens with `logger.info("harness: starting interactive collection (%d pending)", n)`, iterates the resolver chain, and closes with `logger.info("harness: finished interactive collection: %d answered, %d skipped, %d aborted", ...)`. Nothing between those two lines writes to `darnit.harness`. + +**Rationale**: Simplest possible implementation of FR-013a. No log-level fiddling, no filter push/pop, no signal to progress emitters to hush. The audit-progress emitters are already quiescent by the time collect starts; we just have to keep collect itself quiet. + +**Alternatives considered**: +- Install a logging filter for the duration of collect: rejected -- more state, more surface area, no benefit given the audit-progress emitters are already done. +- Route interactive-collect progress to a separate logger name: rejected -- adds an axis of configuration for no operator-visible benefit. + +## R6. Resolution trail construction and error redaction + +**Decision**: For each pending question the driver visits: + +1. Iterate resolvers in registered order. +2. Call `await resolver.resolve(question)` inside a try/except. + - On `Answer(...)` return with non-empty value: append `ResolutionTrailEntry(resolver_name=resolver.name, outcome="answered")`, apply the answer, stop iteration for this question. + - On `Answer(...)` with empty/whitespace value OR `None` return: append `ResolutionTrailEntry(resolver_name=resolver.name, outcome="skipped")`, continue. + - On any exception (except `InteractiveAborted`): capture `str(exc)`, redact via feature 026's `_redact_secrets`, append `ResolutionTrailEntry(resolver_name=resolver.name, outcome="errored", error_summary=redacted[:200])`, continue. + - On `InteractiveAborted` from the interactive resolver: append `ResolutionTrailEntry(resolver_name=resolver.name, outcome="skipped")` (Ctrl+C is a skip for THIS question), stop the whole collection loop (do not offer remaining pending questions to any resolver). + +3. If iteration completes with no `answered` entry, the question stays pending in the report with the full trail attached. + +Error summary is truncated to 200 characters (research-time choice; adjustable in the plan phase if we prefer a different bound). Redaction reuses the exact `_redact_secrets` regex table from feature 026 so credential leakage is handled consistently across the harness. + +**Rationale**: The trail is written incrementally -- every resolver visit produces exactly one entry. Ordering matches invocation order. Error summaries are bounded so a runaway third-party resolver with a 10KB stack trace can't bloat the report. + +**Alternatives considered**: +- Emit the whole exception `__traceback__`: rejected -- privacy risk, size risk. Summary + truncation is the right conservative default. +- Collect exceptions into a sidecar file: rejected -- splits the audit trail across two artifacts, opposite of FR-015a's intent. + +## R7. Interaction with `HarnessRun` public API + +**Decision**: Add `question_resolvers: list[QuestionResolver] = field(default_factory=list)` to the `HarnessRun` dataclass. Add a `HarnessRun.build_default_resolver_chain(interactive: bool) -> list[QuestionResolver]` classmethod that: + +- If `interactive` is True: opens the `interactive_terminal` entry point from `darnit.question_resolvers` and puts it first. +- Then appends every OTHER entry point in the `darnit.question_resolvers` group in the order returned by `importlib.metadata.entry_points()` (stable across a given interpreter session, undefined across installs; documented as such). + +Direct-injection callers construct their own list; the factory exists only for the CLI path. + +**Rationale**: Symmetric with feature 026's `build_default_resolver` (for `AnswerSource`). Keeps the CLI wiring in `cmd_harness` short and lets tests bypass discovery by passing `question_resolvers=[MockResolver()]` directly. + +**Alternatives considered**: +- Auto-append discovered resolvers even when `--interactive` isn't passed: rejected -- surprising behavior; a fleet operator running non-interactive shouldn't have a random third-party resolver start prompting them. Non-interactive default = empty chain unless the operator asks for one. +- Merge `AnswerSource` and `QuestionResolver` behind one Protocol: rejected already in clarify (Q1 discussed the semantic difference); noting here for the record. + +## R8. Backwards compatibility with existing 026 tests + +**Decision**: Every feature-026 test that constructs a `HarnessRun` today does so WITHOUT `question_resolvers`. The default (empty list) means no resolver phase runs; `_collect_unanswered` behaves exactly as it does today for those cases. The "no re-audit after collect" invariant test (test_answered_question_does_not_change_control_status_in_mvp) is unchanged and MUST still pass. + +Adding `question_resolvers` to the driver is additive; the report gains fields (`resolvers_used`, `resolution_trail`) but they default to empty lists / not-emitted so existing golden-file tests only need to be updated if they assert on exact JSON shape. Where they do, we update the assertion to match the new (superset) shape and note the reason in the test. + +**Rationale**: Feature 027 is genuinely additive to the harness surface; no existing behavior changes when the new features aren't invoked. This is the litmus test for a well-scoped addition. + +**Alternatives considered**: +- Only emit `resolution_trail` when non-empty: rejected -- makes JSON-schema conformance testing fussy (field is sometimes present, sometimes not). +- Version the report schema: rejected as over-engineering; a Pydantic model with new fields is the natural evolution. + +## Summary of Phase 0 outcome + +- No NEEDS CLARIFICATION remaining from the spec (all resolved in clarify). +- Every technical unknown for Phase 1 design work has a concrete decision above. +- No new runtime dependencies. stdlib + existing deps only. +- Test surface: injectable streams for `/dev/tty`, external fixture package for entry-point discovery, `MockQuestionResolver` for driver-level chain tests, `_redact_secrets` reuse for error-trail sanitization. +- Constitution IV interaction: interactive answer = `authority: "asserted"`; "no re-audit after collect" MVP policy from feature 026 stays intact. diff --git a/specs/027-interactive-resolvers/spec.md b/specs/027-interactive-resolvers/spec.md new file mode 100644 index 0000000..6e0f0ea --- /dev/null +++ b/specs/027-interactive-resolvers/spec.md @@ -0,0 +1,145 @@ +# Feature Specification: Interactive Question Resolvers + +**Feature Branch**: `027-interactive-resolvers` + +**Created**: 2026-08-07 + +**Status**: Draft + +**Input**: User description: "Add an interactive question-resolver mechanism to `darnit harness` so an operator at the terminal can answer feedback questions live, alongside the existing file-based `--answers` flow." + +## Clarifications + +### Session 2026-08-07 + +- Q: How do third parties register a `QuestionResolver`? -> A: Hybrid -- Python entry points for third-party packages (matching darnit's existing `darnit.implementations` discovery pattern) AND direct injection into `HarnessRun.question_resolvers` for tests and inline library use. +- Q: Where does the interactive prompt write its output? -> A: `/dev/tty` for the MVP (matches the git/ssh/sudo private-operator-channel pattern; isolated from stdout/stderr). The prompt output channel MUST be designed as a pluggable seam so future variants can route prompts to event streams, log sinks, or other observability channels without requiring a Protocol change. `/dev/tty` is the default; configurability is post-MVP. +- Q: What does the operator see for progress during interactive collect? -> A: Bookends only. The prompt payload on `/dev/tty` includes an `[N of M]` position indicator. Stderr gets exactly one "starting interactive collection" line before the first prompt and one "finished interactive collection: X answered, Y skipped" line after the last. No per-question stderr progress line during collect; ordinary `[N/M]` audit-progress lines are suppressed for the duration of the interactive phase. +- Q: What semantics does `Answer("")` (or whitespace-only) have when returned by a programmatic resolver? -> A: Treated as skip, symmetric with interactive UX. A resolver that means "I have no answer for this" returns None; `Answer("")` and `Answer(" ")` are collapsed to skip so the question stays pending. This is a resolver-contract rule, enforced at the harness layer so no resolver author can accidentally record an assertion of emptiness. +- Q: Should the report record which resolvers DECLINED a question, or only which one answered? -> A: Full trail. For each pending question, the report captures a `resolution_trail` list containing one entry per resolver that was offered the question, with an outcome enum: `answered`, `skipped`, or `errored`. This is the Constitution IV audit-trail property: an auditor can see not just the final answer, but every resolver that was tried and why each one didn't produce the value. + +## User Scenarios & Testing *(mandatory)* + +### User Story 1 - Operator answers questions live at the terminal (Priority: P1) + +A fleet-quality auditor runs `darnit harness some-repo --interactive` and the harness pauses to ask each unanswered feedback question in turn. The operator types an answer (or hits Enter to skip). Each answered value lands in the report with an origin string so a later reviewer can see how the value was obtained. + +**Why this priority**: The whole point of the feature. Feature 026 shipped batch answer collection via `--answers `; the operator experience of filling in answers by hand into a YAML file, then re-running the audit, is friction that keeps interactive users off the harness. This story replaces "write YAML, re-run" with "type the answer, next question." + +**Independent Test**: Run the harness against a fixture repo with two pending questions on a TTY-attached terminal; answer one; skip the other. The report shows one answered question with `origin: "interactive_terminal"`, one still-pending question, and no additional YAML file was needed. + +**Acceptance Scenarios**: + +1. **Given** a repo with one pending question and stdin is a TTY, **When** the operator runs `darnit harness --interactive` and types an answer at the prompt, **Then** the report records the answer with `origin: "interactive_terminal"` and no unanswered questions remain for that control. +2. **Given** the same setup, **When** the operator hits Enter without typing anything, **Then** the question stays in the report's pending-feedback list unchanged. +3. **Given** two pending questions, **When** the operator answers the first and hits Ctrl+C at the second, **Then** the report contains the first answer and lists the second question as still pending; the process exits with a documented exit code (audit outcome, not internal error). +4. **Given** stdin is NOT a TTY (piped from a file, running under CI), **When** the operator passes `--interactive`, **Then** the harness fails fast with a clear setup-error message identifying the missing TTY (exit code SETUP_ERROR, in under 2 seconds). + +--- + +### User Story 2 - Third-party author writes a custom resolver (Priority: P2) + +An engineer at a downstream org wants their fleet audit to fetch answers from an internal Slack workflow instead of prompting at the terminal. They write a class with a `name` attribute and an `async def resolve(question) -> Answer | None` method, register it with the harness, and rerun. The harness routes pending questions through their resolver without any change to `darnit.harness.driver`. + +**Why this priority**: The QuestionResolver Protocol is the load-bearing contract of the feature. If P1 lands but the Protocol is not usable by third parties without forking the driver, the feature has failed at its extensibility goal. Interactive resolution is one concrete implementation of the Protocol; the Protocol itself is the deliverable. + +**Independent Test**: A test that defines an in-repo `MockQuestionResolver` returning a fixed answer, injects it into `HarnessRun.question_resolvers`, and asserts the answer appears in the report with the mock's `name` in `origin`. The test must not touch `driver.py` internals. + +**Acceptance Scenarios**: + +1. **Given** a `MockQuestionResolver` class that satisfies the Protocol, **When** it is added to a `HarnessRun`'s resolver list, **Then** pending questions are offered to it and its returned answers appear in the report. +2. **Given** two resolvers registered in order (interactive first, mock second), **When** the interactive resolver returns None for a question (operator skipped), **Then** the mock resolver is offered the same question. +3. **Given** a resolver's `resolve()` raises an exception, **When** the harness processes a pending question, **Then** the exception is caught, logged with the resolver name, and the harness continues to offer the question to the next registered resolver (or leaves it pending if none remain). + +--- + +### User Story 3 - Combining `--answers` file and interactive mode (Priority: P3) + +An operator has a `answers.yaml` covering the values that are known ahead of time (e.g. `security_contact: security@example.com`) and wants to answer the rest live at the terminal. They pass both `--answers answers.yaml --interactive`. Only questions the file did not cover are prompted. + +**Why this priority**: Composition of the two mechanisms is the natural workflow but adds no new safety or extensibility properties beyond P1/P2. A user could achieve the same result by running twice; the primary value is ergonomics. + +**Independent Test**: Fixture repo with three pending questions; `answers.yaml` covering one; run with `--answers ... --interactive` on a fake TTY that answers one prompt and skips the other. The report shows one answer from the file (origin `--answers `), one from interactive (origin `interactive_terminal`), and one still pending. + +**Acceptance Scenarios**: + +1. **Given** an `--answers` file covers one question and one is uncovered, **When** the harness runs with both `--answers` and `--interactive`, **Then** the operator is prompted only for the uncovered question. +2. **Given** the same setup, **When** viewing the final report, **Then** each answered question shows its own origin (file vs. interactive). + +--- + +### Edge Cases + +- **Empty input on Enter**: treated as skip. The question stays pending. Empty input is never recorded as an asserted answer. +- **Whitespace-only input**: also treated as skip. Rationale: a whitespace-only answer is almost certainly a mis-keystroke; recording it as an assertion produces a value that will fail downstream validation anyway, but with confusing provenance ("a human said ''"). Skip is safer and honest. +- **Ctrl+C mid-question**: interpreted as "stop asking further questions." Already-collected answers remain in the report. The harness continues to report assembly (does not treat this as an internal error). +- **Ctrl+D (EOF) mid-question**: same behavior as Ctrl+C -- stop asking, keep what was collected. +- **Very long answer** (over 10KB): accepted. There is no product-level cap; the resolver contract does not constrain answer size. Downstream consumers may truncate for display. +- **A resolver's `resolve()` hangs**: covered by per-resolver timeout (see FR-011). The interactive resolver has no built-in timeout by default (a human at a terminal may take arbitrary time); a fleet operator may set one via configuration if desired. +- **Non-TTY stdin with `--interactive`**: setup error, fail fast. Never silently degrade to skipping all questions. +- **`/dev/tty` unavailable with `--interactive`** (e.g., detached process, unusual container / chroot without the device node): setup error, fail fast. Same class as non-TTY stdin. The interactive resolver's private operator channel is a hard requirement in the MVP. +- **`--interactive` and `--output `**: coexist. Prompts still go to stdin/stdout; the final report goes to the file. +- **Multiple resolvers claim to answer the same question**: first non-None wins. Ordering is registered order; interactive is registered first (immediately after `--answers` in the source chain). +- **Empty pending-questions list at collect time**: interactive resolver is not invoked (nothing to ask). This is not an error; the operator sees no prompt. + +## Requirements *(mandatory)* + +### Functional Requirements + +- **FR-001**: The system MUST provide a `QuestionResolver` Protocol distinct from `AnswerSource`. `QuestionResolver` represents an active, potentially-async answer producer (asks a human, calls an external service, opens an issue) rather than a passive lookup. +- **FR-002**: The Protocol MUST expose at minimum a stable identifier (`name`) and an async `resolve` method that takes one pending feedback question and returns either an answer with provenance or nothing. +- **FR-003**: The harness MUST run registered resolvers downstream of the existing `AnswerSource` chain: `.project/project.yaml` -> `--answers ` -> registered resolvers in registration order. Any question left uncovered by the source chain is offered to each resolver in turn until one returns an answer or the list is exhausted. +- **FR-004**: An `InteractiveTerminalResolver` MUST be provided as the reference implementation. It writes prompts to and reads answers from `/dev/tty` (the private operator channel used by git, ssh, and sudo), so the report stream on stdout and the progress/exit-summary stream on stderr from feature 026 both remain uncontaminated. The prompt payload MUST include, at minimum, the control identifier, the question text, and any control-level help text available. +- **FR-004a**: The prompt output channel MUST be a resolver-internal seam (not baked into the Protocol contract). A future variant that routes prompts to an event stream, log sink, WebSocket, or other observability channel MUST be possible without changing `QuestionResolver` itself. `/dev/tty` is the MVP default; alternative output channels are post-MVP. +- **FR-005**: The interactive resolver MUST refuse to run when stdin is not a TTY OR when `/dev/tty` is not openable (e.g., detached process, chroot without the device). This case is a setup error, not a silent degrade. +- **FR-006**: Empty input (including whitespace-only input) at the interactive prompt MUST be treated as "skip" -- the question stays pending; nothing is asserted. +- **FR-006a**: A resolver that returns an `Answer` whose `value` is empty or whitespace-only MUST be treated identically to a resolver that returns None: the question stays pending; no assertion is recorded. This applies uniformly across all resolvers (interactive, programmatic, future A2A / GH-issue / Slack). "I have no answer" is the single canonical way to skip a question; asserting an empty value is not a supported semantic in the MVP. +- **FR-007**: Ctrl+C or EOF during interactive collection MUST stop prompting for further questions and preserve already-collected answers. This case is not an internal error; the report is still assembled and returned. +- **FR-008**: Every answer produced by a resolver MUST carry an origin string identifying which resolver produced it (e.g. `"interactive_terminal"`, `"gh_issue_42_comment"`, `"a2a_agent_xyz"`). The origin MUST appear in the final report against each answered question. +- **FR-009**: An answer produced by a resolver MUST be tagged with `authority: "asserted"` in the report. Consumers can distinguish it from `dispositive` (observed) or `suggestive` (inferred) provenance. +- **FR-010**: The CLI MUST support a `--interactive` flag on `darnit harness`. It defaults off. Passing it registers the interactive resolver at the head of the resolver chain. +- **FR-011**: The system MUST support a per-resolver timeout mechanism. The interactive resolver's default is no timeout (a human may take arbitrary time); resolver authors and operators MAY configure explicit bounds. +- **FR-012**: A resolver whose `resolve()` raises an exception MUST NOT crash the harness. The error MUST be logged with the resolver's name and the harness MUST continue to offer the same question to any remaining resolvers. +- **FR-013**: Answer values MUST NOT appear verbatim in progress log lines. Log lines may reference the question by control id and context key, but the operator-supplied value belongs in the report, not the log stream. +- **FR-013a**: During interactive collection, the harness MUST emit exactly two bookend lines on stderr: one "starting interactive collection (N pending questions)" line before the first prompt, and one "finished interactive collection: X answered, Y skipped, Z aborted-via-interrupt" line after the last prompt or after Ctrl+C. Ordinary per-control `[N/M]` progress lines MUST be suppressed for the duration of the interactive phase so they do not collide with the prompt. +- **FR-013b**: Each interactive prompt payload written to `/dev/tty` MUST include a position indicator (e.g. `[2 of 5]`), the control identifier, the question text, and any control-level help text available. The position indicator lives in the prompt payload -- NOT in stderr -- so an operator sees exactly one place per prompt where "where am I in this collection?" is answered. +- **FR-014**: The Protocol MUST be discoverable via two mechanisms: (a) Python entry points under a dedicated group (e.g. `darnit.question_resolvers`) so third-party packages can register a resolver by shipping a wheel with an entry-point declaration, matching darnit's existing pattern for framework implementations; and (b) direct injection into `HarnessRun.question_resolvers` at construction time, for tests and inline library use. Neither mechanism requires editing files under `packages/darnit/src/darnit/harness/`. +- **FR-015**: The report MUST record which resolvers were configured for the run, in the same shape as it already records `answer_sources_used`. This is provenance for the audit trail. +- **FR-015a**: For every pending question that reaches the resolver chain, the report MUST include a `resolution_trail` list capturing one entry per resolver that was offered the question. Each entry MUST include the resolver's `name` and an `outcome` value from a small closed set: `answered` (resolver returned a valid non-empty answer), `skipped` (resolver returned None, or an empty-or-whitespace-only Answer per FR-006a), or `errored` (resolver raised an exception per FR-012). `errored` entries MUST also include a truncated exception summary; `answered` entries MUST reference the `Answer.origin` on the accompanying answer. Trail entries appear in the order resolvers were offered the question, so a reader can reconstruct the chain. +- **FR-016**: The existing MVP policy that answer collection does NOT trigger re-audit (feature 026 data-model.md) applies unchanged to interactively collected answers. This spec does not introduce automatic re-audit on interactive input. + +### Key Entities + +- **QuestionResolver**: A pluggable active answerer for a pending feedback question. Has a stable name, an async resolve method, and may fail, time out, or return None ("I can't answer this one"). +- **Answer**: The value returned by a resolver, together with its origin string (which resolver produced it) and its authority tag (`asserted` in every case that flows from this feature). The value MUST be a non-empty, non-whitespace-only string; empty and whitespace-only `Answer` objects are collapsed to skip (equivalent to returning None) by the harness before landing in the report. There is no supported way in the MVP for a resolver to assert an empty value. +- **FeedbackQuestion**: The pending question the sieve produced during audit (already present in feature 026 data-model). Feature 027 adds no new fields. +- **ResolverChain**: The ordered list of resolvers configured on a `HarnessRun`. Composed by the CLI when parsing flags; injectable directly for library/test use. +- **ResolutionTrailEntry**: One entry in the per-question audit trail. Carries the resolver's `name` and an `outcome` (`answered` | `skipped` | `errored`). `errored` entries carry a truncated exception summary; `answered` entries reference the `Answer.origin` of the answer that was accepted. Ordered by the sequence in which resolvers were offered the question. + +## Success Criteria *(mandatory)* + +### Measurable Outcomes + +- **SC-001**: An operator sitting at a terminal can answer five pending questions in under three minutes, end-to-end, from harness invocation to report assembly. Baseline for comparison: the file-round-trip workflow (edit YAML, re-run audit) takes longer than five minutes for the same five questions. +- **SC-002**: A third-party developer can add a new resolver (mock, real, or otherwise) that satisfies the Protocol without modifying any file in `packages/darnit/src/darnit/harness/`. This is enforceable by a test that adds a resolver defined outside that directory tree and asserts it is invoked. +- **SC-003**: Every answer surfaced through a `QuestionResolver` in the final report carries `authority: "asserted"`. No path through this feature emits an answer tagged `dispositive` or `suggestive`. Enforced by test. +- **SC-004**: If the operator hits Ctrl+C after answering some questions, the report still contains those answers. No answer is lost due to interrupt handling. Enforced by test using a scripted resolver that raises `KeyboardInterrupt` after N answers. +- **SC-005**: Running `darnit harness ... --interactive` when no operator channel is available (stdin is not a TTY, OR `/dev/tty` is not openable) fails within 2 seconds with exit code `SETUP_ERROR` and a stderr summary that names the missing channel as the cause. No control is ever executed before this failure. +- **SC-006**: For every answered question in a report, an auditor can determine which resolver produced it by reading a single `origin` field. This is verifiable via schema check on report output. +- **SC-007**: A resolver that raises an exception on `resolve()` does not affect other resolvers or other questions. Verified by a test that registers two resolvers where the first always raises; the second is still invoked and its answers appear in the report. +- **SC-008**: During an interactive run with N pending questions, stderr contains exactly two harness-emitted collection-related lines: the "starting interactive collection" bookend and the "finished interactive collection" bookend. Verified by capturing stderr across a scripted interactive run and asserting the count. +- **SC-009**: For every pending question in the report, an auditor can reconstruct the full resolver chain that was attempted: which resolvers were offered the question, in what order, and with what outcome for each. Verified by a test that registers three resolvers (first errors, second skips, third answers) against one pending question and asserts the `resolution_trail` contains exactly those three entries in order with outcomes `errored`, `skipped`, `answered`. + +## Assumptions + +- The primary interactive medium is the operator's terminal (stdin/stdout). Future resolvers may use other channels (Slack, GitHub, A2A) but this feature ships one reference resolver only. +- The interactive resolver uses stdlib I/O. No dependency is added on `rich`, `click`, `prompt_toolkit`, or similar libraries. Terminal ergonomics beyond "print prompt, read line" are out of scope for the MVP. +- The interactive resolver's prompt output channel is `/dev/tty` in the MVP. A future post-MVP variant may route prompts to an event stream, log sink, or other observability channel for headless / audited operator flows. FR-004a captures this as a design constraint; the specific alternative channels are out of scope for this feature. +- The `AnswerSource` -> `QuestionResolver` two-phase model is the correct shape. Passive lookup and active resolution are semantically distinct enough to warrant separate Protocols. If future evolution merges them, that is a spec change, not a refactor. +- The "no re-audit after collect" MVP policy from feature 026 remains in effect. A separate feature (not this one) may introduce re-audit-on-fresh-answer. +- Progress lines and exit-summary contracts from feature 026 remain unchanged. Interactive prompts appear on stdout; existing stderr contract for exit summaries is untouched. +- CI environments will not use `--interactive`. The non-TTY fail-fast is a safety net, not a common code path. +- Every entry point for creating a `HarnessRun` will provide a way to inject `question_resolvers`, including from the CLI, from the Python API, and (later) via configuration files. The exact injection surface is a plan-phase concern. +- The Constitution IV property ("Never Guess User Values") is not weakened by this feature. A human answering a prompt is confirmation, which promotes a value from candidate to usable -- an asserted answer is the point at which the human explicitly speaks. It never authorizes concluding a value on the human's behalf. +- Constitution IV requires a confirmation to record when it was made, by whom, and which candidate it was based on. This feature does NOT persist interactive answers (feature 026's "no re-audit after collect" MVP policy applies unchanged). The record-when/by-whom/which-candidate requirement therefore applies to a future persistence step (writing the value to `.project/project.yaml`), not to this feature's in-memory capture. Feature 027 is complete without those fields; a follow-up feature that persists interactively supplied values MUST add them. +- Existing tests for feature 026 (batch collection, `--answers`, no re-audit invariant) MUST continue to pass without modification. This feature is additive to the answer-collection surface. diff --git a/specs/027-interactive-resolvers/tasks.md b/specs/027-interactive-resolvers/tasks.md new file mode 100644 index 0000000..e9a5d5d --- /dev/null +++ b/specs/027-interactive-resolvers/tasks.md @@ -0,0 +1,332 @@ +--- +description: "Tasks for feature 027: Interactive Question Resolvers -- extensible resolver Protocol for `darnit harness`" +--- + +# Tasks: Interactive Question Resolvers + +**Input**: Design documents from `specs/027-interactive-resolvers/` + +**Prerequisites**: plan.md (loaded), spec.md (loaded, 5 clarifications), research.md (loaded, 8 decisions), data-model.md (loaded), contracts/{question-resolver-protocol,interactive-resolver-behavior,resolution-trail-schema}.md (loaded), quickstart.md (loaded) + +**Tests**: Test tasks included. Every FR and SC has explicit test coverage. SC-002 (extensibility -- resolver defined outside `packages/darnit/src/darnit/harness/`), SC-003 (interactive answers = `authority: "asserted"`), SC-004 (Ctrl+C preserves collected answers), SC-005 (non-TTY / no-/dev/tty fail-fast), SC-007 (resolver-exception isolation), SC-008 (exactly two bookend lines), SC-009 (three-outcome trail) are load-bearing. + +**Organization**: Tasks are grouped by user story per spec.md. Feature 026's invariants (especially "no re-audit after collect") MUST continue to pass unchanged. + +**Branch base**: `026-harness-with-stage1` (PR #365, still open at time of writing). Rebase to `main` after PR #365 merges. Do NOT branch this from `main` directly -- feature 027 depends on 026's `HarnessRun`, `PendingFeedbackEntry`, and `_redact_secrets`. + +## Format: `[ID] [P?] [Story?] Description` + +- **[P]**: Parallelizable with other [P] tasks in the same phase (different files, no deps on unfinished tasks) +- **[Story]**: Which user story (US1, US2, US3) +- File paths are exact and repository-relative + +--- + +## Phase 1: Setup (Shared Infrastructure) + +**Purpose**: Create the external-fixture package and update `pyproject.toml` so entry-point discovery has something concrete to find. No new runtime dependencies (this feature is stdlib-only on the production surface). + +- [X] T001 Create `tests/darnit/harness/fixtures/mock_resolver_pkg/` directory. Add a minimal `pyproject.toml` declaring `[project] name = "mock-resolver-pkg"` and a `[project.entry-points."darnit.question_resolvers"]` block that registers `mock_answer = "mock_resolver_pkg.resolvers:build_answer"` and `mock_error = "mock_resolver_pkg.resolvers:build_error"`. Include `[tool.hatch.build.targets.wheel] packages = ["mock_resolver_pkg"]`. + +- [X] T002 [P] Create `tests/darnit/harness/fixtures/mock_resolver_pkg/mock_resolver_pkg/__init__.py` (empty) and `resolvers.py`. Implement `AnsweringResolver` (returns `Answer(value="fixed", origin="mock_answer")`) and `ErroringResolver` (raises `RuntimeError("fixture failure")`). Add module-level `build_answer()` and `build_error()` factory functions returning fresh instances. Import `Answer` and `QuestionResolver` from `darnit.harness.question_resolvers` (dep on Phase 2). + +- [X] T003 [P] Update `packages/darnit/pyproject.toml` to declare `[project.entry-points."darnit.question_resolvers"] interactive_terminal = "darnit.harness.interactive_resolver:build"`. Note: Python entry-point declarations are LAZY -- the referenced module is only loaded when `importlib.metadata` iterates entry points AND the caller invokes `ep.load()`. Declaring a target module that will be created later in Phase 3 (T008) is not an install-time error. Discovery gracefully skips broken entry points per contract QR-16. + +**Checkpoint**: External fixture package installable via `uv pip install -e tests/darnit/harness/fixtures/mock_resolver_pkg`; darnit-core's own `interactive_terminal` entry point is declared. + +--- + +## Phase 2: Foundational (Blocking Prerequisites) + +**Purpose**: Build the shared Protocol, entities, and exception types that ALL three user stories depend on. Each task creates a self-contained module. + +**CRITICAL**: No user-story tasks can proceed until this phase is complete. + +- [X] T004 Create `packages/darnit/src/darnit/harness/question_resolvers.py`. Define: + - `Answer` (Pydantic `BaseModel` with `extra="forbid"`; fields `value: str`, `origin: str`, `authority: Literal["asserted"] = "asserted"`; per data-model.md section 2). The `Literal["asserted"]` with a fixed default enforces FR-009 / SC-003 at the model level -- resolver authors physically cannot construct an `Answer` with a different authority; Pydantic raises a validation error at construction time. + - `ResolutionTrailEntry` (Pydantic `BaseModel` with `extra="forbid"`; fields `resolver_name: str`, `outcome: Literal["answered", "skipped", "errored"]`, `error_summary: str | None = None`; cross-field validator: `error_summary` required iff `outcome == "errored"`; per data-model.md section 3) + - `QuestionResolver` (`@runtime_checkable` Protocol with `name: str` and `async def resolve(question) -> Answer | None`; per data-model.md section 1 + contract QR-1..QR-4) + - `InteractiveAborted` (Exception subclass; docstring per data-model.md section 8) + - `__all__` tuple exporting the four names above. + Docstring: cite feature 027 spec + contracts. + +- [X] T005 [P] Create `tests/darnit/harness/test_question_resolvers.py`. Tests: + - `Answer` accepts non-empty strings; rejects `extra` keys (Pydantic extra=forbid). + - `Answer(value="v", origin="o")` produces `authority == "asserted"` by default (SC-003 at the model layer). + - `Answer(value="v", origin="o", authority="dispositive")` raises `ValidationError` (Literal enforcement). + - `Answer(value="v", origin="o", authority="suggestive")` raises `ValidationError`. + - `ResolutionTrailEntry` accepts each of the three outcomes. + - `ResolutionTrailEntry` requires `error_summary` when `outcome == "errored"` (cross-field validation fails otherwise). + - `ResolutionTrailEntry` forbids `error_summary` when `outcome` is `"answered"` or `"skipped"`. + - A test class with `name` and async `resolve` passes `isinstance(x, QuestionResolver)`. + - A test class MISSING `resolve` fails `isinstance` (proves the Protocol shape). + - Serialization round-trip for `Answer.model_dump_json()` and `ResolutionTrailEntry.model_dump_json()` (baseline for schema stability). Assert `authority: "asserted"` appears in the serialized `Answer` JSON. + +- [X] T006 [P] Create `tests/darnit/harness/test_protocol_conformance.py`. Contract tests QR-1..QR-27 from `contracts/question-resolver-protocol.md`: + - QR-1..QR-4: `name` + `resolve` shape + `isinstance` recognition + - QR-5, QR-6: `None` return -> skip semantics; `Answer(value="x")` -> answered semantics (verified via `MockQuestionResolver` and driver's collect path -- deferred to T023 for the driver-level part; T006 covers the boundary) + - QR-9, QR-10: exception passes through as `errored`; `KeyboardInterrupt` NOT caught inside `resolve()` of programmatic resolvers + - QR-26: Protocol shape v1 is exactly what's exported from `question_resolvers.py` + Uses `MockAnsweringResolver`, `MockSkippingResolver`, `MockErroringResolver` fixtures defined in `tests/darnit/harness/conftest.py` (see T007). + +- [X] T007 [P] Update `tests/darnit/harness/conftest.py` to add three feature-027 fixtures: + - `mock_answering_resolver` -- yields a fresh instance of a resolver that returns `Answer(value="mock-answer", origin="mock_answering")` + - `mock_skipping_resolver` -- yields one that returns `None` + - `mock_erroring_resolver(exception_message="mock error")` -- factory fixture; yields a resolver whose `resolve()` raises `RuntimeError(exception_message)` + Every fixture's resolver satisfies `isinstance(r, QuestionResolver)` (i.e., has `name` and async `resolve`). + +**Checkpoint**: `uv run pytest tests/darnit/harness/test_question_resolvers.py tests/darnit/harness/test_protocol_conformance.py -q` passes. Entities and Protocol are locked; downstream code can import from `darnit.harness.question_resolvers`. + +--- + +## Phase 3: User Story 1 -- Operator answers live at the terminal (P1) 🎯 MVP + +**Goal**: `darnit harness --interactive` at a TTY prompts the operator per pending question, records answers with `origin: "interactive_terminal"`, and preserves them across Ctrl+C. + +**Independent Test**: Run the harness against a fixture repo with two pending questions on a scripted "TTY" (test-injectable streams); answer one, skip the other. The report shows one answered question with `origin: "interactive_terminal"`, one still-pending question, and no additional YAML file was needed. + +### Implementation for US1 + +- [X] T008 [US1] Create `packages/darnit/src/darnit/harness/interactive_resolver.py`. Implement `InteractiveTerminalResolver` per contract `interactive-resolver-behavior.md`: + - `name = "interactive_terminal"` (class attribute) + - `__init__(self, input_stream=None, output_stream=None)` -- store streams; do NOT open `/dev/tty` here + - `_open_tty()` -- lazily open `/dev/tty` in mode `"r+", buffering=1` on first `resolve()`. On `OSError` / `FileNotFoundError`, raise `HarnessSetupError` (imported from `darnit.harness.driver`) with message "interactive channel unavailable (/dev/tty not openable)". + - `_format_prompt(question, position, total)` -- produce the exact byte sequence specified by IR-10 (blank line, `[N of M]`, control_id line, question text, optional `Help: ...` indented, `> ` chevron with no newline). + - `async def resolve(question)` -- accepts an optional out-of-band `position` and `total` (see T009 for how the driver threads these in); writes prompt via `_format_prompt`; calls `readline()`; on `KeyboardInterrupt` raise `InteractiveAborted`; on empty `readline()` return raise `InteractiveAborted` (EOF/Ctrl+D); strip result; empty-after-strip -> return `None`; non-empty -> return `Answer(value=stripped, origin="interactive_terminal")`. + - `close()` -- close `/dev/tty` handle idempotently. Subsequent `resolve()` raises `RuntimeError("resolver is closed")`. + - Module-level `def build() -> InteractiveTerminalResolver: return InteractiveTerminalResolver()` (entry-point factory). + - `__all__ = ("InteractiveTerminalResolver", "build")`. + +- [X] T009 [US1] Update `packages/darnit/src/darnit/harness/driver.py`: + - Add `question_resolvers: list[QuestionResolver] = field(default_factory=list)` to `HarnessRun` dataclass (per data-model.md section 7). + - Add `per_resolver_timeout_s: float | None = None` to `HarnessRun` dataclass (per data-model.md section 7). Enforces FR-011. + - Extend `_collect_unanswered` per research.md R6: + - If `question_resolvers` is empty, current 026 behavior is preserved. + - Otherwise: emit bookend log line `harness: starting interactive collection (%d pending questions)` on `darnit.harness` INFO. + - Iterate remaining pending questions. For each, iterate `question_resolvers` in order. Thread `(position, total)` into the interactive resolver via a private branch (`isinstance(r, InteractiveTerminalResolver)` -> pass extra args; other resolvers get the question alone -- see design note in T009's docstring). + - Wrap each `resolver.resolve(...)` call in `asyncio.wait_for(..., timeout=self.per_resolver_timeout_s)` when `per_resolver_timeout_s is not None`. On `TimeoutError`, append `ResolutionTrailEntry(outcome="errored", error_summary=f"resolver timed out after {self.per_resolver_timeout_s}s")` and continue to next resolver. + - Catch: `Answer` with non-empty stripped value -> record answered; append `ResolutionTrailEntry(outcome="answered")`; set the parent `PendingFeedbackEntry.answer_authority = "asserted"` (per data-model.md section 5); break inner loop for this question. + - `None` or `Answer` with empty/whitespace-only value -> append `ResolutionTrailEntry(outcome="skipped")`; continue to next resolver. + - `InteractiveAborted` -> append `ResolutionTrailEntry(outcome="skipped")`; break BOTH loops (stop offering further questions to any resolver). + - Any other `Exception` -> log warning; append `ResolutionTrailEntry(outcome="errored", error_summary=_redact_secrets(str(exc))[:200])`; continue to next resolver. + - Close any resolver that exposes a `close()` method after collection. + - Emit closing bookend `harness: finished interactive collection: %d answered, %d skipped, %d aborted` on INFO. + - No `darnit.harness` log record emitted BETWEEN the two bookends contains any resolver-supplied value (FR-013 preservation). The bookends themselves reference counts only, never values. + - MUST NOT re-audit any control (feature 026 no-reaudit invariant preserved). + +- [X] T010 [US1] Update `packages/darnit/src/darnit/harness/report.py`: + - Add `resolvers_used: list[str] = Field(default_factory=list)` to `HarnessReport`. + - Add `resolution_trail: list[ResolutionTrailEntry] = Field(default_factory=list)` to `PendingFeedbackEntry`. + - Add `answer_authority: Literal["asserted"] | None = None` to `PendingFeedbackEntry` per data-model.md section 5. Cross-field model validator: `answer_authority == "asserted"` required when `answered == True`; `answer_authority is None` required otherwise. + - `to_json()` emits `resolvers_used`, `resolution_trail`, and `answer_authority` unconditionally (empty arrays / None when unused). + - `to_markdown()`: + - Emits a "Resolvers used" bullet list section immediately after "Answer sources used" ONLY if `resolvers_used` is non-empty. + - For each `PendingFeedbackEntry` with a non-empty `resolution_trail`, renders a nested "Resolution trail:" list per contract `resolution-trail-schema.md` section 6. + - Answered entries display their `answer_authority` inline (e.g., `Answered: security@example.com (asserted)`) so a human reader sees the provenance without diving into the JSON. + +- [X] T011 [US1] Update `packages/darnit/src/darnit/cli.py` (`cmd_harness`): + - Add `--interactive` boolean flag (default `False`) to the subparser. + - Availability guard BEFORE `HarnessRun.run()` is invoked: if `--interactive`, verify (a) `sys.stdin.isatty()` returns True AND (b) `open("/dev/tty", "r+")` succeeds. Failure -> exit 2 within 2s with stderr summary `harness: setup_error: interactive channel unavailable (), exit 2` per contract IR-7/IR-8/IR-9. + - On success: construct the resolver chain via `HarnessRun.build_default_resolver_chain(interactive=True)` (see T017 -- for MVP-first ordering, T011 constructs the interactive resolver directly if T017 not yet done, then swaps to the classmethod). + - No new required arguments beyond `--interactive`. + +### Tests for US1 + +- [X] T012 [P] [US1] Create `tests/darnit/harness/test_interactive_resolver.py`. Cover contract IR-1..IR-31: + - IR-1: `resolver.name == "interactive_terminal"` + - IR-4/IR-5: with test-injected streams, prompt output lands in `output_stream` and NOTHING is written to `sys.stdout` or `sys.stderr`. Assert both are empty via `capsys`. + - IR-6: (mocking) `open("/dev/tty", ...)` raises -> `HarnessSetupError` from first `resolve()` call. + - IR-10: golden-file test -- construct a `FeedbackQuestion(control_id="OSPS-GV-01.01", context_key="security_contact", question="Who is the security contact?", ...)`, call `_format_prompt(question, position=2, total=5)`, assert output exactly matches `tests/darnit/harness/fixtures/prompt_golden.txt`. Create the golden file as part of this task. + - IR-11: prompt must NOT contain any answer from a previous question OR the string of any env var; test with a distinctive `ANTHROPIC_API_KEY` set. + - IR-13..IR-16: input handling -- typed answer, empty input (skip), whitespace-only input (skip), leading/trailing whitespace stripped. + - IR-17/IR-18: `KeyboardInterrupt` from `readline()` -> `InteractiveAborted`; empty-string return (EOF) -> `InteractiveAborted`. + - IR-22: `close()` twice is a no-op; `resolve()` after `close()` -> `RuntimeError`. + +- [X] T013 [P] [US1] Create `tests/darnit/harness/test_resolution_trail.py`. Cover SC-009, contract RT-1..RT-14: + - RT-1: every `PendingFeedbackEntry` in a report has a `resolution_trail` field (may be empty). + - RT-6..RT-9: `outcome == "errored"` requires `error_summary`; only one `"answered"` entry per trail and it is the last. + - SC-009: register three resolvers (first errors, second skips, third answers) against one pending question; assert trail contains three entries in that order with the expected outcomes. + - RT-10 + RT-11: an erroring resolver whose `str(exc)` contains `sk-ant-fake-KEY-1234567890` produces a trail entry whose `error_summary` DOES NOT contain the key literal (redacted); length <= 200. + +- [X] T014 [P] [US1] Update `tests/darnit/harness/test_driver.py`. Add a new class `TestQuestionResolverChain`: + - Injecting a `MockAnsweringResolver` into `HarnessRun.question_resolvers` -> pending question resolved; report's `answered` field is True; report's `answer_authority == "asserted"` (SC-003 end-to-end assertion); trail entry has `outcome="answered"`; `origin` is the resolver's own value. + - Injecting a `MockSkippingResolver` FOLLOWED by a `MockAnsweringResolver` -> question resolved by the second; trail has two entries in order (skipped, answered). + - Injecting a `MockErroringResolver` FOLLOWED by a `MockAnsweringResolver` -> question resolved by the second; trail has two entries (errored, answered); error resolver's exception does NOT propagate to caller (SC-007). + - Injecting resolvers but NO pending questions -> chain is not invoked; bookend lines are NOT emitted; report's `resolvers_used` still lists them. + - **Bookend count (SC-008)**: With N pending questions and an injected `MockAnsweringResolver`, capture all `darnit.harness` log records via `caplog`. Assert exactly one record starting with `harness: starting interactive collection (` and exactly one starting with `harness: finished interactive collection: `. Assert ZERO records between them match the `[N/M]` per-control progress-line pattern from feature 026. + - **Programmatic empty-Answer skip (FR-006a / M1)**: A resolver that returns `Answer(value="", origin="x")` -> caught at the driver layer, trail entry has `outcome="skipped"` (NOT `"answered"`); question stays pending; `answer_authority` remains `None`. Same test with `Answer(value=" ", origin="x")` (whitespace-only). + - **No values in progress logs (FR-013 / M2)**: A `MockAnsweringResolver` that returns `Answer(value="DISTINCTIVE-VALUE-XYZ-123", origin="mock")` -> assert NO `darnit.harness` log record contains the literal `DISTINCTIVE-VALUE-XYZ-123`. Mirrors feature 026's API-key-redaction pattern (`test_api_key_never_appears_in_stderr`). + - **Per-resolver timeout (FR-011)**: With `HarnessRun.per_resolver_timeout_s=0.05` and a resolver whose `resolve()` awaits `asyncio.sleep(0.5)` -> trail entry has `outcome="errored"`, `error_summary` contains the substring `timed out`, the driver moves on to the next resolver (or leaves pending if no more). + - The existing `test_answered_question_does_not_change_control_status_in_mvp` test (feature 026 no-reaudit invariant) MUST still pass unmodified when a resolver chain provides the answer -- add a variant asserting the same invariant with a resolver-provided answer. + +- [X] T015 [P] [US1] Update `tests/darnit/harness/test_cli.py`. Add a new class `TestInteractiveFlag`: + - SC-005: `--interactive` under non-TTY stdin (piped from a `StringIO`-backed input; use `monkeypatch.setattr(sys, "stdin", ...)`) -> exit 2 in <2s; stderr summary contains `interactive channel unavailable` and either `stdin is not a TTY` or `/dev/tty not openable`; ZERO `[N/M]` progress lines emitted before the setup_error line. + - SC-005 variant: `--interactive` with stdin-is-TTY but `open("/dev/tty", ...)` mocked to raise -> exit 2 with the `/dev/tty not openable` variant of the error message. + - `--interactive` with a mocked TTY and injected `MockAnsweringResolver` via `HarnessRun.question_resolvers=[...]` (test-only path bypassing the CLI's resolver-chain construction) -> exit 0; report's `resolvers_used` contains `"interactive_terminal"`. + +- [X] T016 [P] [US1] Update `tests/darnit/harness/test_report.py`. Add: + - JSON emission includes `resolvers_used: []`, per-`PendingFeedbackEntry.resolution_trail: []`, and `answer_authority: null` when unused (RT-1, RT-2). + - JSON round-trip: build a `HarnessReport` with a trail containing all three outcomes AND one answered entry with `answer_authority: "asserted"`; `HarnessReport.model_validate_json(r.to_json())` reproduces it. + - Markdown emission: with `resolvers_used=["interactive_terminal"]`, a "Resolvers used" section appears. With an empty `resolvers_used`, no such section. + - Markdown emission: with a non-empty `resolution_trail`, the "Resolution trail:" nested list renders per contract section 6. + - Markdown emission: an answered entry shows `answer_authority` inline (e.g., `Answered: security@example.com (asserted)`). + - **Reconstructibility (SC-006 / M3)**: Build a `HarnessReport` with three trail entries produced by three named resolvers; serialize to JSON via `to_json()`; parse the JSON via `json.loads` (NOT via `HarnessReport.model_validate_json`, to simulate an external consumer that doesn't have the Pydantic model); iterate `pending_feedback[0].resolution_trail` and reconstruct the resolver chain as an ordered list of `(name, outcome)` pairs. Assert the reconstruction matches the input exactly. Proves that a report reader can trace how each value was obtained using only the JSON. + - Pydantic cross-field validator on `PendingFeedbackEntry`: `answered=True` with `answer_authority=None` raises ValidationError. `answered=False` with `answer_authority="asserted"` also raises. + +**Checkpoint**: `uv run darnit harness /path/to/repo --interactive` at a real TTY prompts the operator; skipped/answered questions are captured in the report; Ctrl+C preserves collected answers. US1 is independently shippable if we stop here. + +--- + +## Phase 4: User Story 2 -- Third-party author writes a custom resolver (P2) + +**Goal**: A resolver defined OUTSIDE `packages/darnit/src/darnit/harness/` is discovered via Python entry points and invoked by the harness with no code changes under that directory. + +**Independent Test**: SC-002 -- add a resolver as the fixture package (Phase 1's `mock_resolver_pkg`), install it via `uv pip install -e ...`, run the harness, and observe the resolver being invoked. The test file lives outside `packages/darnit/src/darnit/harness/`; assertions do not modify anything under that directory. + +### Implementation for US2 + +- [X] T017 [US2] Create `packages/darnit/src/darnit/harness/resolver_discovery.py`. Implement per research.md R2: + - `def discover_registered_resolvers() -> dict[str, QuestionResolver]`: + - Call `importlib.metadata.entry_points(group="darnit.question_resolvers")`. + - For each entry: `ep.load()` inside try/except; on success, call the returned factory (zero args); verify `isinstance(instance, QuestionResolver)`; on any failure log a WARNING with the entry-point name and skip. + - Return `{ep.name: instance}` dict. + - `def build_default_resolver_chain(interactive: bool) -> list[QuestionResolver]` (this is the module-level function; the `HarnessRun.build_default_resolver_chain` classmethod delegates): + - Discover all registered resolvers. + - If `interactive` is True: put the `interactive_terminal` resolver first (raise `HarnessSetupError` if not discovered). + - Append every OTHER resolver in `importlib.metadata` discovery order. + - Return the list. + - Docstring cites contract QR-14..QR-16 and R2. + +- [X] T018 [US2] Update `packages/darnit/src/darnit/harness/driver.py`: + - Add `@classmethod def build_default_resolver_chain(cls, interactive: bool)` that delegates to `resolver_discovery.build_default_resolver_chain`. + +- [X] T019 [US2] Update `packages/darnit/src/darnit/cli.py` (`cmd_harness`): + - Replace the direct `InteractiveTerminalResolver()` construction from T011 (if used) with `HarnessRun.build_default_resolver_chain(interactive=args.interactive)`. + - Emit an INFO log line at CLI startup listing the resolvers configured for this run: `harness: resolvers configured: [interactive_terminal, gh_issue_comment, ...]`. This lands on `darnit.harness` before the audit begins. + +### Tests for US2 + +- [X] T020 [P] [US2] Create `tests/darnit/harness/test_resolver_discovery.py`: + - `discover_registered_resolvers()` returns a dict whose keys include `interactive_terminal` (registered by darnit-core itself in `pyproject.toml`). + - With the `mock_resolver_pkg` fixture package installed (add a session-scope fixture that runs `uv pip install -e tests/darnit/harness/fixtures/mock_resolver_pkg` OR uses `importlib.metadata`-monkeypatching per pytest recipe), `discover_registered_resolvers()` returns `mock_answer` and `mock_error` alongside `interactive_terminal`. + - A broken entry point (module that raises `ImportError` on load, mocked via `unittest.mock.patch` on `importlib.metadata`) is skipped with a WARNING but does not crash discovery; other entry points still register. + - `build_default_resolver_chain(interactive=True)` returns a list where `interactive_terminal` is first. + - `build_default_resolver_chain(interactive=False)` returns a list where `interactive_terminal` is NOT present (only third-party resolvers). + +- [X] T021 [P] [US2] Create `tests/darnit/harness/test_extensibility_sc002.py` (SC-002 enforcement): + - Assert that no file under `packages/darnit/src/darnit/harness/` was modified to enable the `mock_resolver_pkg` fixture's resolver to be invoked. Concretely: import the fixture's resolver, register it via a `HarnessRun.question_resolvers=[fixture_resolver]` injection, run the harness against a repo with one pending question, and assert the resolver's answer appears in the report. + - The test itself lives in `tests/`, not in `packages/darnit/src/darnit/harness/`. + - Include a directory-hash sanity check that lists files under `packages/darnit/src/darnit/harness/` and asserts none were modified during test execution (using `os.path.getmtime` before/after -- optional, use if the CI test flakiness allows). + +**Checkpoint**: A wheel outside darnit-core can register a `QuestionResolver` via entry point and be invoked by the harness. SC-002 mechanically holds. + +--- + +## Phase 5: User Story 3 -- Combining `--answers` file and `--interactive` (P3) + +**Goal**: `--answers` file first, `--interactive` fills gaps. Each answered question shows its own origin in the report. + +**Independent Test**: Fixture repo with three pending questions; `answers.yaml` covers one; run with `--answers ... --interactive` on a mock TTY that answers one prompt and skips the other. The report shows one answer from the file (origin `--answers `), one from interactive (origin `interactive_terminal`), and one still pending. + +### Implementation for US3 + +- [X] T022 [US3] Verify the AnswerSource -> QuestionResolver ordering in `_collect_unanswered` (contract QR-19). Read the updated `_collect_unanswered` from T009 and produce a one-paragraph review comment in the PR description confirming (a) the AnswerSource pass runs first, (b) only questions still-pending after that pass are offered to `question_resolvers`, and (c) the ordering is documented in an inline code comment referencing QR-19. If ordering is WRONG, open a bug against T009 and block T023/T024 until fixed. Deliverable: the review paragraph (in the PR desc) OR a follow-up commit fixing the ordering. + +### Tests for US3 + +- [X] T023 [P] [US3] Update `tests/darnit/harness/test_driver.py`. Add a class `TestComposition`: + - Seed `.project/project.yaml` with `security_contact: from_project@example.com` (one question answered by AnswerSource). + - Seed an `--answers` file with `code_of_conduct_url: from_answers` (another question answered by AnswerSource; two-source ordering). + - Inject `MockAnsweringResolver` (would answer any question with `"mock"`, origin `"mock"`). + - Assert: the two questions covered by AnswerSource are answered with the file/project origins; the third gets answered by the resolver with origin `"mock"`; the report's `answer_sources_used` and `resolvers_used` both appear; `resolution_trail` is EMPTY for the two AnswerSource-answered questions (resolver was never offered them) and has exactly one `answered` entry for the third. + +- [X] T024 [P] [US3] Update `tests/darnit/harness/test_cli.py`. Add: + - `--answers --interactive` invocation with two pending questions -- one covered by the file, one uncovered. With test-injected TTY streams answering the uncovered one, assert the report shows both answered, each with their own origin. + +**Checkpoint**: File + interactive compose. US3 is verified. + +--- + +## Phase 6: Polish & Cross-Cutting + +**Purpose**: Docs, quickstart validation, and a final end-to-end sanity pass. + +- [X] T025 [P] Update `packages/darnit/src/darnit/harness/__init__.py` to re-export the new public names: `QuestionResolver`, `Answer`, `ResolutionTrailEntry`, `InteractiveAborted`, `InteractiveTerminalResolver`, `discover_registered_resolvers`, `build_default_resolver_chain`. Preserve existing 026 re-exports. + +- [X] T026 [P] Add feature 027 entry to `CLAUDE.md`'s "Recent Changes" section (top of the list). One-paragraph summary describing the QuestionResolver Protocol seam, the InteractiveTerminalResolver reference implementation, and the `--interactive` CLI flag. + +- [X] T027 [P] Run `uv run ruff check .` and `uv run ruff format --check .` on the whole workspace. Fix any lint issues introduced by the new code. + +- [X] T028 [P] Run `uv run python scripts/validate_sync.py --verbose` and address any handler-name / spec-sync failures. Feature 027 doesn't add controls but a validate_sync pass keeps us honest. + +- [ ] T029 [P] Manual quickstart verification against a real repo. Run through `specs/027-interactive-resolvers/quickstart.md` end-to-end on a repo with at least three pending questions; capture the run output; verify: two bookend lines on stderr, position indicators in prompts, Ctrl+C preserves answers, report contains `resolution_trail` for each answered question. Not automated; produces a manual-sign-off note added to the PR description. + +- [X] T030 [P] Full test sweep: `uv run pytest tests/ -q`. Expected: 2533 + new feature-027 tests all pass, 15 skipped (feature 026 baseline). No regressions in feature 025 or feature 026 tests. + +- [ ] T031 Write the PR description. Structure per `feedback_no_ai_signoff.md`: no Co-Authored-By trailer, no Generated with Claude Code footer. Include a summary, the two new commits' rationale, test plan, and links to spec.md + plan.md. + +--- + +## Dependencies & Story Completion Order + +``` +Phase 1 (T001-T003) --setup-- + | + v +Phase 2 (T004-T007) --foundational: Protocol + entities-- + | + +-----+---------------------+ + v v v + Phase 3 (T008-T016) Phase 4 (T017-T021) [independent of each other after Phase 2] + US1 -- MVP US2 -- extensibility + | | | + +-----+---------------------+ + v + Phase 5 (T022-T024) --composition-- + | + v + Phase 6 (T025-T031) --polish-- +``` + +- **Phase 1 tasks are parallelizable** (T001, T002 [P], T003 [P]) but T002 depends on Phase 2's `question_resolvers.py` for its imports. Reorder as: T001, T003 first; T002 after T004. +- **Phase 2 tasks**: T004 first; T005, T006, T007 all [P] after T004. +- **Phase 3 tasks**: T008, T009, T010, T011 must be sequential (they touch overlapping files); T012-T016 are [P] tests that run in parallel after their subjects exist. +- **Phase 4 tasks**: T017 first; T018, T019 depend on T017; T020, T021 are [P] tests after implementation. +- **Phase 5 tasks**: T022 is a code-inspection task; T023, T024 are [P] tests. +- **Phase 6 tasks**: T025-T030 mostly [P]; T031 is last (needs the full picture). + +## Parallel Execution Examples + +Within Phase 3, once T008-T011 have landed the implementation, run T012, T013, T014, T015, T016 in parallel: + +```bash +uv run pytest tests/darnit/harness/test_interactive_resolver.py \ + tests/darnit/harness/test_resolution_trail.py \ + tests/darnit/harness/test_driver.py::TestQuestionResolverChain \ + tests/darnit/harness/test_cli.py::TestInteractiveFlag \ + tests/darnit/harness/test_report.py \ + -q -n auto +``` + +## Implementation Strategy + +**MVP-first order**: Phase 1 -> Phase 2 -> Phase 3 (US1 delivers the operator-visible value). US1 is independently shippable at that point; US2 and US3 layer on top without changing US1's contract. + +**Time boxing**: Phase 3 is the largest slice (~5 implementation tasks + 5 test tasks). Phases 4 and 5 combined are smaller than Phase 3. Total estimated size: ~600-800 lines net production + ~600 lines tests. Feature 026 shipped ~1500 net production + ~1600 tests; feature 027 is meaningfully smaller because it composes on top of 026 rather than adding a whole new subsystem. + +**Test coverage matrix** (each SC has at least one test task): + +| Success Criterion | Test task(s) | +|---|---| +| SC-001 (5 questions in <3 min) | T029 (manual verification) | +| SC-002 (external resolver, no harness edits) | T021 | +| SC-003 (interactive answer = asserted) | T005 (model-layer), T014 (end-to-end), T016 (report shape) | +| SC-004 (Ctrl+C preserves answers) | T012 | +| SC-005 (non-TTY fail-fast) | T015 | +| SC-006 (origin reconstructible) | T013, T016 (external-consumer reconstruction) | +| SC-007 (resolver exception isolation) | T014 | +| SC-008 (exactly two bookend lines) | T014 (bookend-count assertion) | +| SC-009 (three-outcome trail) | T013 | +| FR-006a (programmatic empty Answer skip) | T005 (unit), T014 (driver-level) | +| FR-011 (per-resolver timeout mechanism) | T014 (timeout test with per_resolver_timeout_s=0.05) | +| FR-013 (no answer values in progress logs) | T014 (no-values-in-logs test) | diff --git a/tests/darnit/harness/conftest.py b/tests/darnit/harness/conftest.py index 975a1a6..199d2a7 100644 --- a/tests/darnit/harness/conftest.py +++ b/tests/darnit/harness/conftest.py @@ -21,6 +21,7 @@ from darnit.core.llm_step import LLMJudgment, MockLLMStep from darnit.harness.answer_sources import AnswerResolver from darnit.harness.driver import HarnessRun +from darnit.harness.question_resolvers import Answer, QuestionResolver FIXTURES_DIR = Path(__file__).parent / "fixtures" @@ -34,6 +35,25 @@ def _ensure_api_key(monkeypatch: pytest.MonkeyPatch) -> None: monkeypatch.setenv("ANTHROPIC_API_KEY", "test-key-not-real") +@pytest.fixture(autouse=True) +def _stub_framework_pending(monkeypatch: pytest.MonkeyPatch) -> None: + """Default: stub `HarnessRun._enumerate_framework_pending` to `[]`. + + Feature 027 QuestionResolver tests (and other harness tests) assemble + synthesized fake results and count how many end up answered/pending. + Once PR #365 wired the framework's own `get_pending_context` into + `_collect_unanswered`, running against a real .baseline.toml fixture + starts emitting real per-key questions -- polluting those counts. + Tests that want the enumerator active un-stub via `monkeypatch.undo()` + or set `run._enumerate_framework_pending = `. + """ + monkeypatch.setattr( + HarnessRun, + "_enumerate_framework_pending", + lambda self: [], + ) + + @pytest.fixture def mock_llm_step() -> MockLLMStep: """Canned LLMJudgment: yes / high confidence / plausible reasoning.""" @@ -120,3 +140,62 @@ def _factory( ) return _factory + + +# --------------------------------------------------------------------------- +# Feature 027: QuestionResolver fixtures +# --------------------------------------------------------------------------- + + +class _MockAnsweringResolver: + """Returns a fixed Answer to every question.""" + + def __init__(self, name: str = "mock_answering", value: str = "mock-answer") -> None: + self.name = name + self._value = value + + async def resolve(self, question: object) -> Answer | None: + return Answer(value=self._value, origin=self.name) + + +class _MockSkippingResolver: + """Returns None to every question.""" + + def __init__(self, name: str = "mock_skipping") -> None: + self.name = name + + async def resolve(self, question: object) -> Answer | None: + return None + + +class _MockErroringResolver: + """Raises RuntimeError on every resolve().""" + + def __init__( + self, + name: str = "mock_erroring", + exception_message: str = "mock error", + ) -> None: + self.name = name + self._msg = exception_message + + async def resolve(self, question: object) -> Answer | None: + raise RuntimeError(self._msg) + + +@pytest.fixture +def mock_answering_resolver() -> QuestionResolver: + return _MockAnsweringResolver() + + +@pytest.fixture +def mock_skipping_resolver() -> QuestionResolver: + return _MockSkippingResolver() + + +@pytest.fixture +def mock_erroring_resolver() -> Callable[..., QuestionResolver]: + def _factory(exception_message: str = "mock error") -> QuestionResolver: + return _MockErroringResolver(exception_message=exception_message) + + return _factory diff --git a/tests/darnit/harness/fixtures/mock_resolver_pkg/mock_resolver_pkg/__init__.py b/tests/darnit/harness/fixtures/mock_resolver_pkg/mock_resolver_pkg/__init__.py new file mode 100644 index 0000000..e69de29 diff --git a/tests/darnit/harness/fixtures/mock_resolver_pkg/mock_resolver_pkg/resolvers.py b/tests/darnit/harness/fixtures/mock_resolver_pkg/mock_resolver_pkg/resolvers.py new file mode 100644 index 0000000..55c629e --- /dev/null +++ b/tests/darnit/harness/fixtures/mock_resolver_pkg/mock_resolver_pkg/resolvers.py @@ -0,0 +1,37 @@ +"""Fixture QuestionResolver implementations for feature 027 SC-002 enforcement. + +This package lives OUTSIDE `packages/darnit/src/darnit/harness/` on purpose: +SC-002 requires that a resolver defined outside that directory can be discovered +and invoked without modifying anything under it. See spec.md SC-002 and +test_extensibility_sc002.py. +""" + +from __future__ import annotations + +from darnit.harness.question_resolvers import Answer, QuestionResolver + + +class AnsweringResolver: + """Returns a fixed Answer to every question.""" + + name = "mock_answer" + + async def resolve(self, question: object) -> Answer | None: + return Answer(value="fixed", origin="mock_answer") + + +class ErroringResolver: + """Raises on every resolve() call.""" + + name = "mock_error" + + async def resolve(self, question: object) -> Answer | None: + raise RuntimeError("fixture failure") + + +def build_answer() -> QuestionResolver: + return AnsweringResolver() + + +def build_error() -> QuestionResolver: + return ErroringResolver() diff --git a/tests/darnit/harness/fixtures/mock_resolver_pkg/pyproject.toml b/tests/darnit/harness/fixtures/mock_resolver_pkg/pyproject.toml new file mode 100644 index 0000000..c3730f8 --- /dev/null +++ b/tests/darnit/harness/fixtures/mock_resolver_pkg/pyproject.toml @@ -0,0 +1,16 @@ +[build-system] +requires = ["hatchling"] +build-backend = "hatchling.build" + +[project] +name = "mock-resolver-pkg" +version = "0.0.1" +description = "Test fixture: external QuestionResolver package for feature 027 SC-002 enforcement." +requires-python = ">=3.11" + +[project.entry-points."darnit.question_resolvers"] +mock_answer = "mock_resolver_pkg.resolvers:build_answer" +mock_error = "mock_resolver_pkg.resolvers:build_error" + +[tool.hatch.build.targets.wheel] +packages = ["mock_resolver_pkg"] diff --git a/tests/darnit/harness/test_cli.py b/tests/darnit/harness/test_cli.py index 9cca147..537a318 100644 --- a/tests/darnit/harness/test_cli.py +++ b/tests/darnit/harness/test_cli.py @@ -308,3 +308,84 @@ def test_api_key_never_appears_in_stderr( assert secret not in r.getMessage(), f"API key leaked into log record: {r.getMessage()!r}" # Report body also key-clean. assert secret not in stdout1 + + +# --------------------------------------------------------------------------- +# Feature 027: --interactive flag (T015) +# --------------------------------------------------------------------------- + + +class TestInteractiveFlag: + """Cover SC-005 (fail-fast) and CLI-visible behavior of --interactive.""" + + def test_interactive_with_non_tty_stdin_fails_fast( + self, + minimal_llm_repo_tree: Path, + mock_llm_step: MockLLMStep, + capsys: pytest.CaptureFixture[str], + caplog: pytest.LogCaptureFixture, + monkeypatch: pytest.MonkeyPatch, + ) -> None: + """SC-005: piped stdin under --interactive -> exit 2 in <2s.""" + # Ensure isatty returns False (default in pytest, but be explicit). + import sys + monkeypatch.setattr(sys.stdin, "isatty", lambda: False) + + start = time.monotonic() + exit_code, _stdout, _stderr, records = _invoke_cli( + [str(minimal_llm_repo_tree), "--interactive", "--level", "1"], + capsys, + caplog, + mock_llm=mock_llm_step, + ) + elapsed = time.monotonic() - start + + assert exit_code == int(HarnessExitCode.SETUP_ERROR) + assert elapsed < 2.0, f"fail-fast bound exceeded: {elapsed:.3f}s" + + summary_msgs = [r.getMessage() for r in records if "setup_error" in r.getMessage()] + assert len(summary_msgs) >= 1 + assert any("interactive channel unavailable" in m for m in summary_msgs) + assert any("stdin is not a TTY" in m for m in summary_msgs) + + # SC-005 also asserts: ZERO progress lines before the setup_error. + progress_pattern = re.compile(r"\[\d+/\d+\]") + progress_lines = [ + r.getMessage() for r in records + if progress_pattern.search(r.getMessage()) + ] + assert progress_lines == [] + + def test_interactive_with_devtty_unavailable_fails_fast( + self, + minimal_llm_repo_tree: Path, + mock_llm_step: MockLLMStep, + capsys: pytest.CaptureFixture[str], + caplog: pytest.LogCaptureFixture, + monkeypatch: pytest.MonkeyPatch, + ) -> None: + """SC-005 variant: stdin is a TTY but /dev/tty is not openable.""" + import builtins + import sys + + monkeypatch.setattr(sys.stdin, "isatty", lambda: True) + real_open = builtins.open + + def _fail_open(path: object, *args: object, **kwargs: object) -> object: + if path == "/dev/tty": + raise OSError("no tty in test env") + return real_open(path, *args, **kwargs) # type: ignore[arg-type] + + monkeypatch.setattr(builtins, "open", _fail_open) + + exit_code, _stdout, _stderr, records = _invoke_cli( + [str(minimal_llm_repo_tree), "--interactive", "--level", "1"], + capsys, + caplog, + mock_llm=mock_llm_step, + ) + assert exit_code == int(HarnessExitCode.SETUP_ERROR) + + summary_msgs = [r.getMessage() for r in records if "setup_error" in r.getMessage()] + assert len(summary_msgs) >= 1 + assert any("/dev/tty not openable" in m for m in summary_msgs) diff --git a/tests/darnit/harness/test_driver.py b/tests/darnit/harness/test_driver.py index 9cb3107..f6e2c44 100644 --- a/tests/darnit/harness/test_driver.py +++ b/tests/darnit/harness/test_driver.py @@ -231,9 +231,13 @@ def test_answered_question_does_not_change_control_status_in_mvp( }, ] - updated, pending, ctx_values = run._collect_unanswered(fake_results) + # Feature 027: _collect_unanswered is now async and returns a 4-tuple + # (results, pending_feedback, answered_feedback, context_values). + updated, pending, answered, ctx_values = _run( + run._collect_unanswered(fake_results), + ) - # (a) status unchanged + # (a) status unchanged (feature 026 invariant preserved) assert updated[0]["status"] == "FAIL" # (b) answer captured on the question + in context_values assert updated[0]["feedback_questions"][0]["answered"] is True @@ -245,6 +249,368 @@ def test_answered_question_does_not_change_control_status_in_mvp( # generally NOT empty on a real fixture -- assert instead that the # question we answered isn't in it. assert "security_contact" not in {e.context_key for e in pending} + # (d) feature 027: answer captured in answered_feedback with origin + answered_for_security_contact = [ + a for a in answered if a.context_key == "security_contact" + ] + assert len(answered_for_security_contact) == 1 + entry = answered_for_security_contact[0] + assert entry.control_id == "STAGE1-REF-SECURITY-01" + assert entry.answer == "sec@example.com" + assert entry.origin == "mock" + assert entry.authority == "asserted" + + +# --------------------------------------------------------------------------- +# Feature 027: QuestionResolver chain tests (T014) +# --------------------------------------------------------------------------- + + +class TestQuestionResolverChain: + """Cover SC-003, SC-007, SC-008, FR-006a, FR-011, FR-013 at the driver layer.""" + + def _make_fake_results(self, context_keys: list[str]) -> list[dict]: + """Build fake result dicts each with one pending feedback question.""" + return [ + { + "id": f"CTRL-{i:02d}", + "status": "FAIL", + "authority": "dispositive", + "level": 1, + "feedback_questions": [ + { + "control_id": f"CTRL-{i:02d}", + "context_key": key, + "question": f"Question for {key}?", + "answered": False, + }, + ], + } + for i, key in enumerate(context_keys) + ] + + def test_answering_resolver_resolves_question_with_asserted_authority( + self, + minimal_llm_repo_tree: Path, + harness_run_factory: Callable[..., HarnessRun], + mock_answering_resolver: object, + ) -> None: + """SC-003 end-to-end: answered question carries authority='asserted' in the report.""" + run = harness_run_factory(str(minimal_llm_repo_tree)) + run.question_resolvers = [mock_answering_resolver] + + results = self._make_fake_results(["security_contact"]) + _updated, pending, answered, ctx = _run(run._collect_unanswered(results)) + + assert pending == [] + assert len(answered) == 1 + assert answered[0].authority == "asserted" + assert answered[0].origin == "mock_answering" + assert ctx["security_contact"] == "mock-answer" + assert len(answered[0].resolution_trail) == 1 + assert answered[0].resolution_trail[0].outcome == "answered" + + def test_skipping_then_answering_produces_two_trail_entries( + self, + minimal_llm_repo_tree: Path, + harness_run_factory: Callable[..., HarnessRun], + mock_skipping_resolver: object, + mock_answering_resolver: object, + ) -> None: + """Trail ordering: skipping resolver appears BEFORE the answering one.""" + run = harness_run_factory(str(minimal_llm_repo_tree)) + run.question_resolvers = [mock_skipping_resolver, mock_answering_resolver] + + results = self._make_fake_results(["security_contact"]) + _updated, pending, answered, _ctx = _run(run._collect_unanswered(results)) + + assert pending == [] + assert len(answered) == 1 + trail = answered[0].resolution_trail + assert len(trail) == 2 + assert trail[0].resolver_name == "mock_skipping" + assert trail[0].outcome == "skipped" + assert trail[1].resolver_name == "mock_answering" + assert trail[1].outcome == "answered" + + def test_erroring_resolver_isolated_from_answering( + self, + minimal_llm_repo_tree: Path, + harness_run_factory: Callable[..., HarnessRun], + mock_erroring_resolver: Callable[..., object], + mock_answering_resolver: object, + ) -> None: + """SC-007: errored resolver does not stop chain; second resolver answers.""" + run = harness_run_factory(str(minimal_llm_repo_tree)) + run.question_resolvers = [ + mock_erroring_resolver(exception_message="mock error x"), + mock_answering_resolver, + ] + + results = self._make_fake_results(["security_contact"]) + _updated, pending, answered, _ctx = _run(run._collect_unanswered(results)) + + assert pending == [] + assert len(answered) == 1 + trail = answered[0].resolution_trail + assert len(trail) == 2 + assert trail[0].outcome == "errored" + assert trail[0].error_summary is not None + assert "mock error x" in trail[0].error_summary + assert trail[1].outcome == "answered" + + def test_resolvers_configured_but_no_pending_questions( + self, + minimal_llm_repo_tree: Path, + harness_run_factory: Callable[..., HarnessRun], + mock_answering_resolver: object, + caplog: pytest.LogCaptureFixture, + ) -> None: + """When there are 0 pending questions, no bookend lines are emitted.""" + import logging + + run = harness_run_factory(str(minimal_llm_repo_tree)) + run.question_resolvers = [mock_answering_resolver] + + caplog.set_level(logging.INFO, logger="darnit.harness") + results: list[dict] = [] # no results = no questions + _updated, pending, answered, _ctx = _run(run._collect_unanswered(results)) + + assert pending == [] + assert answered == [] + collection_lines = [ + r.getMessage() for r in caplog.records + if "interactive collection" in r.getMessage() + ] + assert collection_lines == [] + + def test_bookend_lines_appear_exactly_once_each( + self, + minimal_llm_repo_tree: Path, + harness_run_factory: Callable[..., HarnessRun], + mock_answering_resolver: object, + caplog: pytest.LogCaptureFixture, + ) -> None: + """SC-008: exactly two bookend lines per interactive collection phase.""" + import logging + import re + + run = harness_run_factory(str(minimal_llm_repo_tree)) + run.question_resolvers = [mock_answering_resolver] + + caplog.set_level(logging.INFO, logger="darnit.harness") + results = self._make_fake_results(["k1", "k2", "k3"]) + _run(run._collect_unanswered(results)) + + starting = [ + r.getMessage() for r in caplog.records + if "starting interactive collection" in r.getMessage() + ] + finished = [ + r.getMessage() for r in caplog.records + if "finished interactive collection" in r.getMessage() + ] + assert len(starting) == 1, f"expected 1 starting bookend, got {starting}" + assert len(finished) == 1, f"expected 1 finished bookend, got {finished}" + assert "3 pending" in starting[0] + + # No per-control [N/M] progress lines from feature 026 during collect + progress_pattern = re.compile(r"\[\d+/\d+\]") + between = [ + r.getMessage() for r in caplog.records + if progress_pattern.search(r.getMessage()) + ] + assert between == [] + + def test_programmatic_empty_answer_collapsed_to_skip( + self, + minimal_llm_repo_tree: Path, + harness_run_factory: Callable[..., HarnessRun], + ) -> None: + """FR-006a / M1: Answer('') and Answer(' ') collapse to skip at the driver.""" + from darnit.harness.question_resolvers import Answer + + class _EmptyReturningResolver: + name = "empty_returning" + + async def resolve(self, question: object) -> Answer | None: + return Answer(value=" ", origin="empty_returning") + + run = harness_run_factory(str(minimal_llm_repo_tree)) + run.question_resolvers = [_EmptyReturningResolver()] + + results = self._make_fake_results(["k1"]) + _updated, pending, answered, _ctx = _run(run._collect_unanswered(results)) + + assert answered == [] + assert len(pending) == 1 + assert len(pending[0].resolution_trail) == 1 + assert pending[0].resolution_trail[0].outcome == "skipped" + + def test_answer_values_never_appear_in_log_records( + self, + minimal_llm_repo_tree: Path, + harness_run_factory: Callable[..., HarnessRun], + caplog: pytest.LogCaptureFixture, + ) -> None: + """FR-013 / M2: resolver-supplied values must not leak to any log line.""" + import logging + + from darnit.harness.question_resolvers import Answer + + distinct_value = "DISTINCTIVE-VALUE-XYZ-123-NEVER-IN-LOGS" + + class _DistinctiveResolver: + name = "distinctive" + + async def resolve(self, question: object) -> Answer | None: + return Answer(value=distinct_value, origin="distinctive") + + run = harness_run_factory(str(minimal_llm_repo_tree)) + run.question_resolvers = [_DistinctiveResolver()] + + caplog.set_level(logging.DEBUG, logger="darnit.harness") + results = self._make_fake_results(["k1"]) + _run(run._collect_unanswered(results)) + + for record in caplog.records: + assert distinct_value not in record.getMessage(), ( + f"leaked value into log: {record.getMessage()!r}" + ) + + def test_per_resolver_timeout_records_errored_entry( + self, + minimal_llm_repo_tree: Path, + harness_run_factory: Callable[..., HarnessRun], + mock_answering_resolver: object, + ) -> None: + """FR-011: a slow resolver hits the timeout, gets 'errored' outcome, + and the driver moves on to the next resolver.""" + import asyncio as _asyncio + + from darnit.harness.question_resolvers import Answer + + class _SlowResolver: + name = "slow" + + async def resolve(self, question: object) -> Answer | None: + await _asyncio.sleep(0.5) + return Answer(value="never-returned", origin="slow") + + run = harness_run_factory(str(minimal_llm_repo_tree)) + run.question_resolvers = [_SlowResolver(), mock_answering_resolver] + run.per_resolver_timeout_s = 0.05 + + results = self._make_fake_results(["k1"]) + _updated, pending, answered, _ctx = _run(run._collect_unanswered(results)) + + assert pending == [] + assert len(answered) == 1 + trail = answered[0].resolution_trail + assert len(trail) == 2 + assert trail[0].resolver_name == "slow" + assert trail[0].outcome == "errored" + assert trail[0].error_summary is not None + assert "timed out" in trail[0].error_summary + assert trail[1].outcome == "answered" + + +# --------------------------------------------------------------------------- +# US3: composition of --answers file with resolver chain (T023) +# --------------------------------------------------------------------------- + + +class TestComposition: + def test_answer_source_wins_before_resolver_chain( + self, + minimal_llm_repo_tree: Path, + harness_run_factory: Callable[..., HarnessRun], + mock_answering_resolver: object, + ) -> None: + """QR-19: AnswerSource pass runs BEFORE QuestionResolver chain. + A question answered by an AnswerSource never reaches the resolvers.""" + from darnit.harness.answer_sources import AnswerResolver + from tests.darnit.harness.test_answer_sources import MockAnswerSource + + answer_resolver = AnswerResolver() + answer_resolver.add( + MockAnswerSource( + "project_yaml", {"security_contact": "from_project@example.com"}, + ), + ) + answer_resolver.add( + MockAnswerSource( + "answers_file", {"code_of_conduct_url": "from_answers.md"}, + ), + ) + + run = harness_run_factory(str(minimal_llm_repo_tree)) + run.answer_resolver = answer_resolver + run.question_resolvers = [mock_answering_resolver] + + # Three questions: two covered by AnswerSource, one uncovered. + results = [ + { + "id": "CTRL-01", + "status": "FAIL", + "authority": "dispositive", + "level": 1, + "feedback_questions": [ + { + "control_id": "CTRL-01", + "context_key": "security_contact", + "question": "sec?", + "answered": False, + }, + ], + }, + { + "id": "CTRL-02", + "status": "FAIL", + "authority": "dispositive", + "level": 1, + "feedback_questions": [ + { + "control_id": "CTRL-02", + "context_key": "code_of_conduct_url", + "question": "coc?", + "answered": False, + }, + ], + }, + { + "id": "CTRL-03", + "status": "FAIL", + "authority": "dispositive", + "level": 1, + "feedback_questions": [ + { + "control_id": "CTRL-03", + "context_key": "release_process", + "question": "release?", + "answered": False, + }, + ], + }, + ] + _updated, pending, answered, ctx = _run(run._collect_unanswered(results)) + + # All three answered + assert pending == [] + assert len(answered) == 3 + # Distinct origins per question + by_key = {e.context_key: e for e in answered} + assert by_key["security_contact"].origin == "project_yaml" + assert by_key["code_of_conduct_url"].origin == "answers_file" + assert by_key["release_process"].origin == "mock_answering" + + # Resolver chain was ONLY offered the uncovered question -- verified + # by the trail (empty for AnswerSource-answered, populated for the + # resolver-answered one). + assert by_key["security_contact"].resolution_trail == [] + assert by_key["code_of_conduct_url"].resolution_trail == [] + assert len(by_key["release_process"].resolution_trail) == 1 + assert by_key["release_process"].resolution_trail[0].outcome == "answered" # --------------------------------------------------------------------------- diff --git a/tests/darnit/harness/test_extensibility_sc002.py b/tests/darnit/harness/test_extensibility_sc002.py new file mode 100644 index 0000000..122b5a0 --- /dev/null +++ b/tests/darnit/harness/test_extensibility_sc002.py @@ -0,0 +1,98 @@ +"""SC-002 enforcement: a resolver defined OUTSIDE the harness dir works +(feature 027 T021). + +The whole extensibility contract of feature 027 lives here: a resolver that +lives outside `packages/darnit/src/darnit/harness/` must be discoverable and +invoked by the harness with NO edits under that directory. + +We prove this by: + 1. Importing a `QuestionResolver` from + `tests/darnit/harness/fixtures/mock_resolver_pkg/mock_resolver_pkg/resolvers.py` + -- a location strictly outside `packages/darnit/src/darnit/harness/`. + 2. Injecting it into `HarnessRun.question_resolvers` and running the collect + phase. + 3. Asserting the injected resolver's answer landed in the report. + +Bonus proof: the fixture's `pyproject.toml` declares entry-points; a downstream +consumer that `uv pip install`s the fixture package would get the same resolver +via discovery. That path is covered by `test_resolver_discovery.py`; here we +only cover the direct-injection surface which is even stronger (proves the +Protocol works for any external code, discovery-mediated or not). +""" + +from __future__ import annotations + +import asyncio +import sys +from collections.abc import Callable +from pathlib import Path + +from darnit.harness.driver import HarnessRun +from darnit.harness.question_resolvers import QuestionResolver + +# Make the fixture package importable without installing it. +_FIXTURE_ROOT = ( + Path(__file__).parent / "fixtures" / "mock_resolver_pkg" +).resolve() +if str(_FIXTURE_ROOT) not in sys.path: + sys.path.insert(0, str(_FIXTURE_ROOT)) + + +def _run(coro): + return asyncio.new_event_loop().run_until_complete(coro) + + +class TestExternalResolverExtensibility: + def test_resolver_from_outside_harness_dir_is_invoked( + self, + minimal_llm_repo_tree: Path, + harness_run_factory: Callable[..., HarnessRun], + ) -> None: + """SC-002: import a resolver from a directory OUTSIDE the harness + subtree; use it via direct injection; assert its answer appears in + the report.""" + # Import the fixture (proves the module lives outside the harness dir). + from mock_resolver_pkg.resolvers import ( # type: ignore[import-not-found] + build_answer, + ) + + # Sanity: the resolver's module path is under the fixture dir, NOT + # under the harness dir. If somebody moves it, this assertion fails + # loudly. + instance = build_answer() + module_file = sys.modules[type(instance).__module__].__file__ or "" + assert "packages/darnit/src/darnit/harness" not in module_file, ( + f"fixture resolver leaked into harness dir: {module_file}" + ) + assert isinstance(instance, QuestionResolver) + + # Now: run the driver's collect phase with this external resolver. + run = harness_run_factory(str(minimal_llm_repo_tree)) + run.question_resolvers = [instance] + + fake_results = [ + { + "id": "CTRL-01", + "status": "FAIL", + "authority": "dispositive", + "level": 1, + "feedback_questions": [ + { + "control_id": "CTRL-01", + "context_key": "security_contact", + "question": "Who?", + "answered": False, + }, + ], + }, + ] + _updated, pending, answered, ctx = _run(run._collect_unanswered(fake_results)) + + # Extensibility contract: external resolver's answer appears here. + assert pending == [] + assert len(answered) == 1 + assert answered[0].origin == "mock_answer" + assert ctx["security_contact"] == "fixed" + # SC-003: authority=asserted preserved through the external Protocol + # boundary. + assert answered[0].authority == "asserted" diff --git a/tests/darnit/harness/test_interactive_resolver.py b/tests/darnit/harness/test_interactive_resolver.py new file mode 100644 index 0000000..aeedc38 --- /dev/null +++ b/tests/darnit/harness/test_interactive_resolver.py @@ -0,0 +1,211 @@ +"""Tests for InteractiveTerminalResolver (feature 027 T012). + +Covers contract IR-1..IR-31 from contracts/interactive-resolver-behavior.md. + +Streams are injected via `input_stream` / `output_stream` constructor args so +tests don't touch /dev/tty. IR-6 is verified by mocking `open` at module level. +""" + +from __future__ import annotations + +import asyncio +import io +from typing import Any + +import pytest + +from darnit.harness.driver import HarnessSetupError +from darnit.harness.interactive_resolver import InteractiveTerminalResolver +from darnit.harness.question_resolvers import Answer, InteractiveAborted + + +def _run(coro): + return asyncio.new_event_loop().run_until_complete(coro) + + +class _Question: + def __init__( + self, + control_id: str = "OSPS-GV-01.01", + question: str = "Who is the security contact?", + help_md: str = "", + ) -> None: + self.control_id = control_id + self.question = question + self.help_md = help_md + + +class TestIR1Name: + def test_name_is_interactive_terminal(self) -> None: + assert InteractiveTerminalResolver.name == "interactive_terminal" + assert InteractiveTerminalResolver().name == "interactive_terminal" + + +class TestIR4And5StreamsIsolatedFromStdoutStderr: + def test_prompt_lands_only_on_injected_output_stream( + self, capsys: pytest.CaptureFixture[str], + ) -> None: + """IR-4 + IR-5: prompt goes to injected stream, NOT stdout/stderr.""" + in_stream = io.StringIO("security@example.com\n") + out_stream = io.StringIO() + r = InteractiveTerminalResolver( + input_stream=in_stream, output_stream=out_stream, + ) + _run(r.resolve(_Question(), position=1, total=1)) + captured = capsys.readouterr() + assert captured.out == "" + assert captured.err == "" + assert "Who is the security contact?" in out_stream.getvalue() + + +class TestIR6FailsFastWhenTtyUnavailable: + def test_open_devtty_failure_raises_setup_error( + self, monkeypatch: pytest.MonkeyPatch, + ) -> None: + """IR-6: /dev/tty unavailable -> HarnessSetupError.""" + import builtins + + real_open = builtins.open + + def _fail_open(path: Any, *args: Any, **kwargs: Any) -> Any: + if path == "/dev/tty": + raise OSError("no tty in test env") + return real_open(path, *args, **kwargs) + + monkeypatch.setattr(builtins, "open", _fail_open) + r = InteractiveTerminalResolver() + with pytest.raises(HarnessSetupError, match="/dev/tty not openable"): + _run(r.resolve(_Question())) + + +class TestIR10PromptFormat: + def test_prompt_contains_position_control_id_question(self) -> None: + """IR-10: position header, control_id, question text, chevron.""" + out_stream = io.StringIO() + r = InteractiveTerminalResolver( + input_stream=io.StringIO("\n"), output_stream=out_stream, + ) + _run( + r.resolve( + _Question( + control_id="OSPS-GV-01.01", + question="Who is the security contact?", + help_md="A person who handles vulnerability reports.", + ), + position=2, + total=5, + ), + ) + prompt = out_stream.getvalue() + assert "[2 of 5]" in prompt + assert "OSPS-GV-01.01" in prompt + assert "Who is the security contact?" in prompt + assert "Help: A person who handles vulnerability reports." in prompt + assert prompt.endswith("> ") + + def test_prompt_without_help_omits_help_line(self) -> None: + out_stream = io.StringIO() + r = InteractiveTerminalResolver( + input_stream=io.StringIO("\n"), output_stream=out_stream, + ) + _run(r.resolve(_Question(help_md=""), position=1, total=1)) + assert "Help:" not in out_stream.getvalue() + + +class TestIR11PromptDoesNotLeakSecrets: + def test_prompt_does_not_contain_api_key( + self, monkeypatch: pytest.MonkeyPatch, + ) -> None: + """IR-11: ANTHROPIC_API_KEY must never appear in the prompt payload.""" + secret = "sk-ant-DISTINCTIVE-XYZ-9zz" + monkeypatch.setenv("ANTHROPIC_API_KEY", secret) + out_stream = io.StringIO() + r = InteractiveTerminalResolver( + input_stream=io.StringIO("\n"), output_stream=out_stream, + ) + _run(r.resolve(_Question(), position=1, total=1)) + assert secret not in out_stream.getvalue() + + +class TestIR13Through16InputHandling: + def test_typed_answer_returns_answer(self) -> None: + r = InteractiveTerminalResolver( + input_stream=io.StringIO("security@example.com\n"), + output_stream=io.StringIO(), + ) + result = _run(r.resolve(_Question())) + assert isinstance(result, Answer) + assert result.value == "security@example.com" + assert result.origin == "interactive_terminal" + assert result.authority == "asserted" + + def test_empty_input_returns_none(self) -> None: + r = InteractiveTerminalResolver( + input_stream=io.StringIO("\n"), + output_stream=io.StringIO(), + ) + result = _run(r.resolve(_Question())) + assert result is None + + def test_whitespace_only_input_returns_none(self) -> None: + r = InteractiveTerminalResolver( + input_stream=io.StringIO(" \t \n"), + output_stream=io.StringIO(), + ) + result = _run(r.resolve(_Question())) + assert result is None + + def test_leading_trailing_whitespace_stripped(self) -> None: + r = InteractiveTerminalResolver( + input_stream=io.StringIO(" value-with-spaces \n"), + output_stream=io.StringIO(), + ) + result = _run(r.resolve(_Question())) + assert result is not None + assert result.value == "value-with-spaces" + + +class TestIR17And18InterruptHandling: + def test_keyboard_interrupt_raises_interactive_aborted( + self, monkeypatch: pytest.MonkeyPatch, + ) -> None: + """IR-17: readline() KeyboardInterrupt -> InteractiveAborted.""" + + class _InterruptingStream: + def readline(self) -> str: + raise KeyboardInterrupt + + r = InteractiveTerminalResolver( + input_stream=_InterruptingStream(), + output_stream=io.StringIO(), + ) + with pytest.raises(InteractiveAborted): + _run(r.resolve(_Question())) + + def test_eof_raises_interactive_aborted(self) -> None: + """IR-18: readline() returns empty (EOF/Ctrl+D) -> InteractiveAborted.""" + r = InteractiveTerminalResolver( + input_stream=io.StringIO(""), # empty, immediate EOF + output_stream=io.StringIO(), + ) + with pytest.raises(InteractiveAborted): + _run(r.resolve(_Question())) + + +class TestIR22CloseLifecycle: + def test_close_is_idempotent(self) -> None: + r = InteractiveTerminalResolver( + input_stream=io.StringIO(), + output_stream=io.StringIO(), + ) + r.close() + r.close() # second call is a no-op + + def test_resolve_after_close_raises(self) -> None: + r = InteractiveTerminalResolver( + input_stream=io.StringIO("v\n"), + output_stream=io.StringIO(), + ) + r.close() + with pytest.raises(RuntimeError, match="closed"): + _run(r.resolve(_Question())) diff --git a/tests/darnit/harness/test_protocol_conformance.py b/tests/darnit/harness/test_protocol_conformance.py new file mode 100644 index 0000000..946c301 --- /dev/null +++ b/tests/darnit/harness/test_protocol_conformance.py @@ -0,0 +1,88 @@ +"""Contract tests for QuestionResolver Protocol (feature 027 T006). + +Verifies contract QR-1..QR-27 from contracts/question-resolver-protocol.md +at the Protocol boundary. Driver-level assertions live in test_driver.py. +""" + +from __future__ import annotations + +import asyncio +from collections.abc import Callable + +from darnit.harness.question_resolvers import ( + Answer, + QuestionResolver, +) + + +def _run(coro): + return asyncio.new_event_loop().run_until_complete(coro) + + +class TestQR1ThroughQR4NameAndResolveShape: + """QR-1..QR-4: name attribute + async resolve + isinstance recognition.""" + + def test_mock_answering_conforms( + self, mock_answering_resolver: QuestionResolver, + ) -> None: + assert isinstance(mock_answering_resolver, QuestionResolver) + assert hasattr(mock_answering_resolver, "name") + assert isinstance(mock_answering_resolver.name, str) + + def test_mock_skipping_conforms( + self, mock_skipping_resolver: QuestionResolver, + ) -> None: + assert isinstance(mock_skipping_resolver, QuestionResolver) + + def test_mock_erroring_conforms( + self, mock_erroring_resolver: Callable[..., QuestionResolver], + ) -> None: + r = mock_erroring_resolver() + assert isinstance(r, QuestionResolver) + + +class TestQR5AndQR6ReturnSemantics: + """QR-5: None => skip. QR-6: Answer(non-empty) => answered.""" + + def test_none_return_produces_skip_semantic( + self, mock_skipping_resolver: QuestionResolver, + ) -> None: + result = _run(mock_skipping_resolver.resolve(question=None)) + assert result is None + + def test_answer_return_carries_expected_shape( + self, mock_answering_resolver: QuestionResolver, + ) -> None: + result = _run(mock_answering_resolver.resolve(question=None)) + assert isinstance(result, Answer) + assert result.value # non-empty + assert result.authority == "asserted" + + +class TestQR9ExceptionPropagation: + """QR-9: resolver exception is expected to propagate; the driver catches.""" + + def test_erroring_resolver_raises_from_resolve( + self, mock_erroring_resolver: Callable[..., QuestionResolver], + ) -> None: + r = mock_erroring_resolver(exception_message="propagates") + try: + _run(r.resolve(question=None)) + except RuntimeError as exc: + assert "propagates" in str(exc) + else: + raise AssertionError("resolver should have raised") + + +class TestQR26ProtocolShapeV1: + """QR-26: v1 exports exactly these public names from question_resolvers.""" + + def test_module_exports_the_four_public_names(self) -> None: + import darnit.harness.question_resolvers as qr + + assert set(qr.__all__) == { + "Answer", + "ResolutionTrailEntry", + "QuestionResolver", + "InteractiveAborted", + } diff --git a/tests/darnit/harness/test_question_resolvers.py b/tests/darnit/harness/test_question_resolvers.py new file mode 100644 index 0000000..4f1e78f --- /dev/null +++ b/tests/darnit/harness/test_question_resolvers.py @@ -0,0 +1,133 @@ +"""Tests for the QuestionResolver Protocol + entities (feature 027 T005). + +Covers data-model.md sections 1-3, 8. Contract QR-1..QR-4, QR-26. +""" + +from __future__ import annotations + +import json + +import pytest +from pydantic import ValidationError + +from darnit.harness.question_resolvers import ( + Answer, + InteractiveAborted, + QuestionResolver, + ResolutionTrailEntry, +) + + +class TestAnswer: + def test_accepts_non_empty_string(self) -> None: + a = Answer(value="v", origin="o") + assert a.value == "v" + assert a.origin == "o" + + def test_default_authority_is_asserted(self) -> None: + """SC-003 at the model layer: every Answer defaults to authority='asserted'.""" + a = Answer(value="v", origin="o") + assert a.authority == "asserted" + + def test_rejects_other_authority_dispositive(self) -> None: + """FR-009 enforcement: Literal['asserted'] blocks any other value.""" + with pytest.raises(ValidationError): + Answer(value="v", origin="o", authority="dispositive") # type: ignore[arg-type] + + def test_rejects_other_authority_suggestive(self) -> None: + with pytest.raises(ValidationError): + Answer(value="v", origin="o", authority="suggestive") # type: ignore[arg-type] + + def test_extra_fields_forbidden(self) -> None: + with pytest.raises(ValidationError): + Answer(value="v", origin="o", extra_field="nope") # type: ignore[call-arg] + + def test_json_roundtrip_includes_authority(self) -> None: + a = Answer(value="v", origin="o") + payload = json.loads(a.model_dump_json()) + assert payload == {"value": "v", "origin": "o", "authority": "asserted"} + + +class TestResolutionTrailEntry: + def test_accepts_answered(self) -> None: + e = ResolutionTrailEntry(resolver_name="r", outcome="answered") + assert e.outcome == "answered" + assert e.error_summary is None + + def test_accepts_skipped(self) -> None: + e = ResolutionTrailEntry(resolver_name="r", outcome="skipped") + assert e.outcome == "skipped" + + def test_accepts_errored_with_summary(self) -> None: + e = ResolutionTrailEntry( + resolver_name="r", outcome="errored", error_summary="boom", + ) + assert e.error_summary == "boom" + + def test_errored_without_summary_rejected(self) -> None: + """RT-6: outcome='errored' requires a non-empty error_summary.""" + with pytest.raises(ValidationError): + ResolutionTrailEntry(resolver_name="r", outcome="errored") + + def test_errored_with_empty_summary_rejected(self) -> None: + with pytest.raises(ValidationError): + ResolutionTrailEntry( + resolver_name="r", outcome="errored", error_summary="", + ) + + def test_answered_with_summary_rejected(self) -> None: + """The reverse invariant: only errored may carry a summary.""" + with pytest.raises(ValidationError): + ResolutionTrailEntry( + resolver_name="r", outcome="answered", error_summary="oops", + ) + + def test_skipped_with_summary_rejected(self) -> None: + with pytest.raises(ValidationError): + ResolutionTrailEntry( + resolver_name="r", outcome="skipped", error_summary="oops", + ) + + def test_json_roundtrip(self) -> None: + e = ResolutionTrailEntry( + resolver_name="r", outcome="errored", error_summary="boom", + ) + payload = json.loads(e.model_dump_json()) + assert payload == { + "resolver_name": "r", + "outcome": "errored", + "error_summary": "boom", + } + + +class TestQuestionResolverProtocol: + def test_class_with_name_and_resolve_conforms(self) -> None: + class Good: + name = "good" + + async def resolve(self, question: object) -> Answer | None: + return None + + assert isinstance(Good(), QuestionResolver) + + def test_class_missing_resolve_does_not_conform(self) -> None: + class Bad: + name = "bad" + + assert not isinstance(Bad(), QuestionResolver) + + def test_class_missing_name_does_not_conform(self) -> None: + class NoName: + async def resolve(self, question: object) -> Answer | None: + return None + + assert not isinstance(NoName(), QuestionResolver) + + +class TestInteractiveAborted: + def test_is_exception_subclass(self) -> None: + assert issubclass(InteractiveAborted, Exception) + + def test_can_be_raised_and_caught(self) -> None: + with pytest.raises(InteractiveAborted): + raise InteractiveAborted("test") diff --git a/tests/darnit/harness/test_report.py b/tests/darnit/harness/test_report.py index cc8b14d..11a12db 100644 --- a/tests/darnit/harness/test_report.py +++ b/tests/darnit/harness/test_report.py @@ -164,3 +164,138 @@ def test_markdown_hides_api_key( monkeypatch.setenv("ANTHROPIC_API_KEY", secret) md = sample_report.to_markdown() assert secret not in md + + +# --------------------------------------------------------------------------- +# Feature 027: resolvers_used + answered_feedback + resolution_trail (T016) +# --------------------------------------------------------------------------- + + +class TestFeature027ReportAdditions: + def _minimal_report(self, **overrides) -> HarnessReport: + base = { + "target": {"local_path": "/tmp"}, + "summary": HarnessSummary( + total=0, **{"pass": 0, "fail": 0, "warn": 0, "n_a": 0, "error": 0}, + ), + "controls": [], + "pending_feedback": [], + "answer_sources_used": [], + "llm_calls": {"total": 0, "provider": "mock"}, + } + base.update(overrides) + return HarnessReport(**base) + + def test_json_emits_resolvers_used_and_answered_feedback_when_empty( + self, + ) -> None: + """Both new fields serialize as empty arrays when unused (predictable shape).""" + report = self._minimal_report() + payload = json.loads(report.to_json()) + assert payload["resolvers_used"] == [] + assert payload["answered_feedback"] == [] + + def test_json_roundtrip_with_three_outcome_trail(self) -> None: + """Serialize an entry with all three trail outcomes; reparse and + assert the shape survives.""" + from darnit.harness.question_resolvers import ResolutionTrailEntry + from darnit.harness.report import AnsweredFeedbackEntry + + entry = AnsweredFeedbackEntry( + control_id="C", + context_key="k", + question="q", + answer="v", + origin="slack", + resolution_trail=[ + ResolutionTrailEntry( + resolver_name="gh", outcome="errored", error_summary="boom", + ), + ResolutionTrailEntry(resolver_name="term", outcome="skipped"), + ResolutionTrailEntry(resolver_name="slack", outcome="answered"), + ], + ) + report = self._minimal_report( + resolvers_used=["gh", "term", "slack"], + answered_feedback=[entry], + ) + js = report.to_json() + + # Roundtrip via Pydantic + reparsed = HarnessReport.model_validate_json(js) + assert reparsed.resolvers_used == ["gh", "term", "slack"] + assert len(reparsed.answered_feedback) == 1 + assert reparsed.answered_feedback[0].authority == "asserted" + assert [e.outcome for e in reparsed.answered_feedback[0].resolution_trail] == [ + "errored", + "skipped", + "answered", + ] + + def test_markdown_resolvers_used_section_only_when_non_empty(self) -> None: + empty = self._minimal_report() + assert "## Resolvers Used" not in empty.to_markdown() + + with_resolvers = self._minimal_report( + resolvers_used=["interactive_terminal"], + ) + md = with_resolvers.to_markdown() + assert "## Resolvers Used" in md + assert "- interactive_terminal" in md + + def test_markdown_answered_feedback_section_with_trail(self) -> None: + from darnit.harness.question_resolvers import ResolutionTrailEntry + from darnit.harness.report import AnsweredFeedbackEntry + + report = self._minimal_report( + resolvers_used=["interactive_terminal"], + answered_feedback=[ + AnsweredFeedbackEntry( + control_id="OSPS-GV-01.01", + context_key="security_contact", + question="Who is the security contact?", + answer="security@example.com", + origin="interactive_terminal", + resolution_trail=[ + ResolutionTrailEntry( + resolver_name="interactive_terminal", + outcome="answered", + ), + ], + ), + ], + ) + md = report.to_markdown() + assert "## Answered Feedback" in md + assert "OSPS-GV-01.01" in md + assert "security@example.com" in md + assert "origin: interactive_terminal" in md + assert "authority: asserted" in md + assert "Resolution trail:" in md + + def test_markdown_pending_feedback_shows_trail(self) -> None: + from darnit.harness.question_resolvers import ResolutionTrailEntry + from darnit.harness.report import PendingFeedbackEntry + + report = self._minimal_report( + resolvers_used=["r1"], + pending_feedback=[ + PendingFeedbackEntry( + control_id="OSPS-GV-01.01", + context_key="security_contact", + question="Who is the security contact?", + resolution_trail=[ + ResolutionTrailEntry( + resolver_name="r1", + outcome="errored", + error_summary="unavailable", + ), + ], + ), + ], + ) + md = report.to_markdown() + assert "## Pending Feedback" in md + assert "OSPS-GV-01.01" in md + assert "Resolution trail:" in md + assert "errored -- unavailable" in md diff --git a/tests/darnit/harness/test_resolution_trail.py b/tests/darnit/harness/test_resolution_trail.py new file mode 100644 index 0000000..19e947c --- /dev/null +++ b/tests/darnit/harness/test_resolution_trail.py @@ -0,0 +1,269 @@ +"""Tests for the resolution_trail contract (feature 027 T013). + +Covers SC-009 (three-outcome trail), contract RT-1..RT-14 from +contracts/resolution-trail-schema.md, and redaction/truncation semantics. +""" + +from __future__ import annotations + +import asyncio +import json +from collections.abc import Callable +from pathlib import Path + +from darnit.harness.driver import HarnessRun +from darnit.harness.question_resolvers import ( + Answer, + ResolutionTrailEntry, +) + + +def _run(coro): + return asyncio.new_event_loop().run_until_complete(coro) + + +class TestRT1EveryPendingHasTrail: + def test_pending_entry_has_resolution_trail_field(self) -> None: + from darnit.harness.report import PendingFeedbackEntry + + entry = PendingFeedbackEntry( + control_id="X", context_key="k", question="q?", + ) + # Default: empty list, never absent + assert entry.resolution_trail == [] + + +class TestRT6ToRT9CrossFieldInvariants: + def test_answered_last_when_present( + self, + minimal_llm_repo_tree: Path, + harness_run_factory: Callable[..., HarnessRun], + mock_skipping_resolver: object, + mock_answering_resolver: object, + ) -> None: + """RT-9: if any entry has outcome='answered', it is the LAST entry.""" + run = harness_run_factory(str(minimal_llm_repo_tree)) + run.question_resolvers = [mock_skipping_resolver, mock_answering_resolver] + + results = [ + { + "id": "CTRL", + "status": "FAIL", + "authority": "dispositive", + "level": 1, + "feedback_questions": [ + { + "control_id": "CTRL", + "context_key": "k", + "question": "q?", + "answered": False, + }, + ], + }, + ] + _updated, _pending, answered, _ctx = _run(run._collect_unanswered(results)) + + trail = answered[0].resolution_trail + answered_indices = [ + i for i, e in enumerate(trail) if e.outcome == "answered" + ] + assert len(answered_indices) == 1 + assert answered_indices[0] == len(trail) - 1 + + +class TestSC009ThreeOutcomeTrail: + def test_errored_then_skipped_then_answered( + self, + minimal_llm_repo_tree: Path, + harness_run_factory: Callable[..., HarnessRun], + mock_erroring_resolver: Callable[..., object], + mock_skipping_resolver: object, + mock_answering_resolver: object, + ) -> None: + """SC-009: three resolvers (errored, skipped, answered) produce + exactly that three-entry trail in that order.""" + run = harness_run_factory(str(minimal_llm_repo_tree)) + run.question_resolvers = [ + mock_erroring_resolver(exception_message="fixture boom"), + mock_skipping_resolver, + mock_answering_resolver, + ] + + results = [ + { + "id": "CTRL", + "status": "FAIL", + "authority": "dispositive", + "level": 1, + "feedback_questions": [ + { + "control_id": "CTRL", + "context_key": "k", + "question": "q?", + "answered": False, + }, + ], + }, + ] + _updated, _pending, answered, _ctx = _run(run._collect_unanswered(results)) + + trail = answered[0].resolution_trail + assert len(trail) == 3 + outcomes = [e.outcome for e in trail] + assert outcomes == ["errored", "skipped", "answered"] + + names = [e.resolver_name for e in trail] + assert names == ["mock_erroring", "mock_skipping", "mock_answering"] + + +class TestRT10RedactionOfErrorSummary: + def test_credential_material_scrubbed_from_error_summary( + self, + minimal_llm_repo_tree: Path, + harness_run_factory: Callable[..., HarnessRun], + mock_answering_resolver: object, + ) -> None: + """RT-10 + RT-11: sk-ant-* substrings in exception messages are + redacted before landing in trail entries.""" + secret = "sk-ant-fake-KEY-1234567890" + + class _LeakingErrorResolver: + name = "leaking" + + async def resolve(self, question: object) -> Answer | None: + raise RuntimeError(f"http 401 with token {secret}") + + run = harness_run_factory(str(minimal_llm_repo_tree)) + run.question_resolvers = [_LeakingErrorResolver(), mock_answering_resolver] + + results = [ + { + "id": "CTRL", + "status": "FAIL", + "authority": "dispositive", + "level": 1, + "feedback_questions": [ + { + "control_id": "CTRL", + "context_key": "k", + "question": "q?", + "answered": False, + }, + ], + }, + ] + _updated, _pending, answered, _ctx = _run(run._collect_unanswered(results)) + + trail = answered[0].resolution_trail + assert len(trail) == 2 + errored = trail[0] + assert errored.outcome == "errored" + assert errored.error_summary is not None + assert secret not in errored.error_summary + assert "REDACTED" in errored.error_summary + assert len(errored.error_summary) <= 200 + + +class TestRT2EmptyTrailWhenAnswerSourceCoversQuestion: + """RT-1 clarification: a question resolved by AnswerSource NEVER reaches + the resolver chain, so its resolution_trail (on AnsweredFeedbackEntry) + is empty. Answered by an ANSWER SOURCE, not a resolver.""" + + def test_answer_source_answered_question_has_empty_trail( + self, + minimal_llm_repo_tree: Path, + harness_run_factory: Callable[..., HarnessRun], + mock_answering_resolver: object, + ) -> None: + from darnit.harness.answer_sources import AnswerResolver + from tests.darnit.harness.test_answer_sources import MockAnswerSource + + resolver = AnswerResolver() + resolver.add(MockAnswerSource("mock_source", {"k": "from-source"})) + + run = harness_run_factory(str(minimal_llm_repo_tree)) + run.answer_resolver = resolver + # Resolvers configured but should NOT be invoked -- source answers first. + run.question_resolvers = [mock_answering_resolver] + + results = [ + { + "id": "CTRL", + "status": "FAIL", + "authority": "dispositive", + "level": 1, + "feedback_questions": [ + { + "control_id": "CTRL", + "context_key": "k", + "question": "q?", + "answered": False, + }, + ], + }, + ] + _updated, _pending, answered, ctx = _run(run._collect_unanswered(results)) + + assert ctx["k"] == "from-source" + assert len(answered) == 1 + assert answered[0].origin == "mock_source" + assert answered[0].resolution_trail == [] + + +class TestReconstructibilityViaJSON: + """M3 / SC-006: an external consumer reading the report JSON can + reconstruct the resolver chain from `resolution_trail` alone.""" + + def test_external_json_consumer_can_reconstruct_chain(self) -> None: + from darnit.harness.report import AnsweredFeedbackEntry, HarnessReport, HarnessSummary + + entry = AnsweredFeedbackEntry( + control_id="CTRL-01", + context_key="k", + question="q?", + answer="v", + origin="slack_dm", + resolution_trail=[ + ResolutionTrailEntry( + resolver_name="gh_issue_comment", + outcome="errored", + error_summary="HTTP 404", + ), + ResolutionTrailEntry( + resolver_name="interactive_terminal", outcome="skipped", + ), + ResolutionTrailEntry( + resolver_name="slack_dm", outcome="answered", + ), + ], + ) + report = HarnessReport( + target={"local_path": "/tmp"}, + summary=HarnessSummary( + total=0, **{"pass": 0}, fail=0, warn=0, n_a=0, error=0, + ), + controls=[], + pending_feedback=[], + answer_sources_used=[], + llm_calls={"total": 0, "provider": "mock"}, + resolvers_used=["gh_issue_comment", "interactive_terminal", "slack_dm"], + answered_feedback=[entry], + ) + + # Serialize and parse WITHOUT using the Pydantic model (external + # consumer simulation). + payload = json.loads(report.to_json()) + af = payload["answered_feedback"][0] + + reconstructed = [ + (e["resolver_name"], e["outcome"]) + for e in af["resolution_trail"] + ] + assert reconstructed == [ + ("gh_issue_comment", "errored"), + ("interactive_terminal", "skipped"), + ("slack_dm", "answered"), + ] + # Every trail entry has the origin recoverable from `resolver_name`. + assert af["origin"] == "slack_dm" + assert af["authority"] == "asserted" diff --git a/tests/darnit/harness/test_resolver_discovery.py b/tests/darnit/harness/test_resolver_discovery.py new file mode 100644 index 0000000..e276eba --- /dev/null +++ b/tests/darnit/harness/test_resolver_discovery.py @@ -0,0 +1,138 @@ +"""Tests for QuestionResolver entry-point discovery (feature 027 T020). + +Covers contract QR-14..QR-16 and research decision R2. +""" + +from __future__ import annotations + +from importlib.metadata import EntryPoint +from unittest.mock import patch + +import pytest + +from darnit.harness.driver import HarnessSetupError +from darnit.harness.question_resolvers import QuestionResolver +from darnit.harness.resolver_discovery import ( + ENTRY_POINT_GROUP, + build_default_resolver_chain, + discover_registered_resolvers, +) + + +class TestDiscovery: + def test_interactive_terminal_is_registered_by_darnit_core(self) -> None: + """darnit-core's own pyproject.toml registers `interactive_terminal`.""" + resolvers = discover_registered_resolvers() + assert "interactive_terminal" in resolvers + + def test_returned_instances_conform_to_protocol(self) -> None: + resolvers = discover_registered_resolvers() + for name, instance in resolvers.items(): + assert isinstance(instance, QuestionResolver), ( + f"resolver {name!r} does not satisfy the Protocol" + ) + + def test_broken_entry_point_is_skipped_with_warning( + self, caplog: pytest.LogCaptureFixture, + ) -> None: + """QR-16: an entry point that fails to load is logged and skipped; + other entry points still register.""" + import logging + + # Craft a real EntryPoint pointing at a nonexistent module. + broken_ep = EntryPoint( + name="broken_resolver", + value="darnit_nonexistent_module_xyz:factory", + group=ENTRY_POINT_GROUP, + ) + + # Also craft a good EntryPoint pointing at the interactive-terminal + # factory so the "other resolvers still register" claim is verifiable. + good_ep = EntryPoint( + name="_test_good", + value="darnit.harness.interactive_resolver:build", + group=ENTRY_POINT_GROUP, + ) + + class _FakeEntryPoints: + def __init__(self, items: list[EntryPoint]) -> None: + self._items = items + + def __iter__(self): + return iter(self._items) + + with patch( + "darnit.harness.resolver_discovery.metadata.entry_points", + return_value=_FakeEntryPoints([broken_ep, good_ep]), + ): + caplog.set_level(logging.WARNING, logger="darnit.harness.resolver_discovery") + resolvers = discover_registered_resolvers() + + # Broken skipped; good registered + assert "broken_resolver" not in resolvers + assert "_test_good" in resolvers + + # Warning was logged + warn_msgs = [ + r.getMessage() for r in caplog.records + if r.name == "darnit.harness.resolver_discovery" + ] + assert any("broken_resolver" in m for m in warn_msgs) + + def test_factory_returning_wrong_type_is_skipped_with_warning( + self, caplog: pytest.LogCaptureFixture, + ) -> None: + """QR-16: a factory that returns a non-Protocol object is skipped.""" + import logging + + class _MockEntryPoint: + """Mock that implements the minimal EntryPoint API we call.""" + + name = "wrong_type" + + def load(self): + # Return the factory; harness will call it and see object(). + return lambda: object() + + class _MockEntryPoints: + def __iter__(self): + return iter([_MockEntryPoint()]) + + with patch( + "darnit.harness.resolver_discovery.metadata.entry_points", + return_value=_MockEntryPoints(), + ): + caplog.set_level( + logging.WARNING, + logger="darnit.harness.resolver_discovery", + ) + resolvers = discover_registered_resolvers() + + assert "wrong_type" not in resolvers + assert any( + "does not satisfy the QuestionResolver Protocol" in r.getMessage() + for r in caplog.records + ) + + +class TestBuildDefaultResolverChain: + def test_interactive_true_puts_terminal_first(self) -> None: + chain = build_default_resolver_chain(interactive=True) + assert chain, "chain should be non-empty when interactive=True" + assert chain[0].name == "interactive_terminal" + + def test_interactive_false_omits_terminal(self) -> None: + """QR-21: --interactive controls whether the terminal is in the chain.""" + chain = build_default_resolver_chain(interactive=False) + names = [r.name for r in chain] + assert "interactive_terminal" not in names + + def test_interactive_true_raises_if_terminal_missing(self) -> None: + """Explicit failure mode when a fleet installs without the terminal + resolver's entry point (should be impossible in practice, defensive).""" + with patch( + "darnit.harness.resolver_discovery.discover_registered_resolvers", + return_value={}, + ): + with pytest.raises(HarnessSetupError): + build_default_resolver_chain(interactive=True)