fix(cli): mode-appropriate retry advice, and validate input files before any call (#1114)

Both from PR review, both real.

1. The retry-cap handler gave --brief-file runs an --answers-file message: it
   told them a "canned answer list cannot recover" and to "supply answers that
   address every part of the question". Brief mode has no list and takes a
   brief, so that advice is unactionable — and it contradicts this PR's own
   claim that --brief-file survives a rejection. Worse, can_desynchronise was
   added precisely to tell the two modes apart and then never used. It is now
   the branch: --answers-file is pointed at --brief-file, and --brief-file is
   told the brief probably does not cover the question, quoting it.

2. The input files were loaded after session.start_discovery(), which generates
   the opening question — a paid call taking minutes (#902). So a typo in a path
   cost a request before failing. This contradicted the comment three lines
   above the conflict check ("a flag conflict must not cost an API call"), which
   I had applied to the flags and not their contents. Both loads moved up; two
   tests assert create_provider is never called on a bad path.

The brief-mode message test exercises the handler directly. Driving it through
the CLI hung inside the discovery engine's own retry loop, which would have
meant mocking the whole engine to assert a message string.

Full suite: 6351 passed, 49 skipped.

Author: frankbria

codeframe/cli/app.py

@@ -196,13 +196,24 @@ def _handle_answer_attempts_exhausted(session, answers: "_AnswerSource", questio
         f"\n[yellow]That question has been answered {MAX_ANSWER_ATTEMPTS} times "
         f"without being accepted.[/yellow]"
     )
-    if not answers.interactive:
+    if answers.can_desynchronise:
         console.print(
             "[red]Error:[/red] giving up in non-interactive mode. The validator "
             "rejects partial answers, and AI-generated questions often have two "
-            "parts — a canned answer list cannot recover once it desynchronises. "
-            "Run interactively, or supply answers that address every part of the "
-            "question."
+            "parts — a canned --answers-file cannot recover once it "
+            "desynchronises. Use --brief-file, which answers the question "
+            "actually asked, or run interactively."
+        )
+        raise typer.Exit(1)
+    if not answers.interactive:
+        # --brief-file: there is no list to desynchronise, so repeated rejection
+        # means the brief does not cover what is being asked.
+        console.print(
+            "[red]Error:[/red] giving up in non-interactive mode. The stand-in "
+            "answered from the brief five times without being accepted, which "
+            "usually means the brief does not cover what this question asks:\n\n"
+            f"  {escape(question.get('text', ''))}\n\n"
+            "Extend the brief to cover it, or run interactively."
         )
         raise typer.Exit(1)
 
@@ -1741,6 +1752,12 @@ def prd_generate(
         )
         raise typer.Exit(2)
 
+    # Same rule for the file contents. start_discovery() generates the opening
+    # question, which is a paid call taking minutes (#902) — a missing or
+    # malformed input file must not get that far.
+    canned = _load_answers_file(answers_file) if answers_file else None
+    brief = _load_brief_file(brief_file) if brief_file else None
+
     console.print(f"[dim]Using template: {escape(template_obj.name)}[/dim]")
 
     try:
@@ -1793,8 +1810,6 @@ def prd_generate(
         console.print("[dim]The AI will ask questions to understand your project.[/dim]")
         console.print("[dim]Type /help for available commands[/dim]\n")
 
-        canned = _load_answers_file(answers_file) if answers_file else None
-        brief = _load_brief_file(brief_file) if brief_file else None
         brief_provider = None
         if brief is not None:
             # The stand-in answerer uses the same resolved provider chain as

tests/cli/test_prd_generate_non_interactive_1114.py

@@ -25,6 +25,7 @@
     AnswersExhausted,
     _AnswerSource,
     _load_answers_file,
+    _handle_answer_attempts_exhausted,
     _load_brief_file,
     app,
 )
@@ -342,3 +343,75 @@ def test_a_non_utf8_brief_file(self, tmp_path):
         with pytest.raises(typer.BadParameter) as exc:
             _load_brief_file(path)
         assert "UTF-8" in str(exc.value)
+
+
+class TestReviewFindings:
+    """Two findings from PR review, both real."""
+
+    def test_a_bad_input_file_costs_no_llm_call(self, workspace_dir, tmp_path):
+        """start_discovery() generates the opening question — a paid, slow call.
+
+        Validation of the input file was happening after it, so a typo in a path
+        cost a request and minutes of wall time before failing.
+        """
+        with patch("codeframe.core.llm_resolution.create_provider") as create_provider:
+            result = runner.invoke(
+                app,
+                [
+                    "prd", "generate", "-w", str(workspace_dir),
+                    "--answers-file", str(tmp_path / "does-not-exist.json"),
+                ],
+                env={"ANTHROPIC_API_KEY": "test-key"},
+            )
+
+        assert result.exit_code != 0
+        create_provider.assert_not_called()
+
+    def test_a_bad_brief_file_costs_no_llm_call(self, workspace_dir, tmp_path):
+        with patch("codeframe.core.llm_resolution.create_provider") as create_provider:
+            result = runner.invoke(
+                app,
+                [
+                    "prd", "generate", "-w", str(workspace_dir),
+                    "--brief-file", str(tmp_path / "nope.md"),
+                ],
+                env={"ANTHROPIC_API_KEY": "test-key"},
+            )
+
+        assert result.exit_code != 0
+        create_provider.assert_not_called()
+
+    def test_brief_mode_gets_brief_advice_not_answers_advice(self, capsys):
+        """The retry-cap message must match the mode.
+
+        --brief-file has no canned list, so telling the user their "answer list
+        desynchronised" and to "supply answers" is unactionable. Exercised on the
+        handler directly: driving it through the CLI would mean mocking the whole
+        discovery engine, which is not what this finding is about.
+        """
+        import typer
+
+        source = _AnswerSource(None, "A brief.", MagicMock())
+        question = {"text": "What problem, and what inspired it?"}
+
+        with pytest.raises(typer.Exit):
+            _handle_answer_attempts_exhausted(MagicMock(), source, question)
+
+        out = capsys.readouterr().out
+        assert "brief" in out.lower()
+        assert "--answers-file" not in out
+        assert "canned" not in out.lower()
+        # It should quote the question the brief failed to cover.
+        assert "What problem" in out
+
+    def test_answers_file_mode_still_gets_answers_advice(self, capsys):
+        import typer
+
+        source = _AnswerSource(["a", "b"])
+
+        with pytest.raises(typer.Exit):
+            _handle_answer_attempts_exhausted(MagicMock(), source, {"text": "q"})
+
+        out = capsys.readouterr().out
+        assert "--answers-file" in out
+        assert "--brief-file" in out, "it should point at the mode that recovers"