fix(core): six low-severity PRD, discovery and template defects (#961)

1. create_new_version's error path masked the real exception. A bare
   cursor.execute("ROLLBACK") raises "cannot rollback - no transaction is
   active" whenever the failure happened before BEGIN took effect, and that
   replacement propagated instead of the original fault. The rollback is now
   best-effort and logged; the original always re-raises.

2. Legacy PRDs vanished from `cf prd list`. The chain_id backfill ran only
   WHERE parent_id IS NULL, so every legacy *child* kept a NULL chain_id — and
   because SQL's NULL = NULL is never true, list_chains' INNER JOIN dropped
   those rows entirely, with no error. Two fixes: a recursive-CTE backfill that
   walks each row to its root (so children join their parent's chain instead of
   forming spurious one-row chains), re-run on upgrade via SCHEMA_VERSION 4->5;
   and COALESCE(chain_id, parent_id, id) in list_chains so a read stays correct
   for anything the migration cannot resolve, such as a child whose parent row
   is gone.

3. submit_answer validated against a None question. A completed or
   freshly-loaded session passed self._current_question — None — into
   _validate_answer, asking the model to judge an answer against no question.
   Now raises DiscoveryError naming which case it is.

4. reset_discovery without a session_id closed EVERY non-completed session in
   the workspace, contradicting its own docstring and destroying in-flight work
   on unrelated PRDs. Now targets only the most recent one.

5. apply_template silently dropped out-of-range dependency indices via
   `if 0 <= idx < len(created_tasks)`, producing a task graph that looked fine
   and executed in the wrong order. Indices are now validated BEFORE any task
   is created, so a bad template raises with the offending index and leaves no
   half-built set of tasks behind.

6. `cf prd stress-test` had no handler for provider failures. It caught
   StressTestError but not an auth/network/rate-limit error from the LLM, which
   escaped as a traceback — on a command whose whole body is a multi-call LLM
   run. Both LLM calls (the stress test and the ambiguity refine) now report an
   actionable message and exit 1; the refine handler also states plainly that
   no answers were saved.

Closes #961

Author: frankbria

codeframe/cli/app.py

@@ -1800,6 +1800,19 @@ def prd_stress_test(
         # the CLI shows a traceback where every other failure here is a red line.
         console.print(f"[red]Error:[/red] {e}")
         raise typer.Exit(1)
+    except Exception as e:
+        # A provider failure (auth, network, rate limit) is not a StressTestError
+        # and used to escape as a raw traceback (#961). This is a multi-call LLM
+        # run, so it is one of the likelier ways the command ends.
+        console.print(
+            f"[red]Error:[/red] PRD stress test failed while calling the LLM "
+            f"provider: {e}"
+        )
+        console.print(
+            "[dim]Check your API key and network connection, then retry. "
+            "Use --llm-provider/--llm-model to try a different model.[/dim]"
+        )
+        raise typer.Exit(1)
 
     # Show ambiguity report
     if result.ambiguities:
@@ -1828,9 +1841,22 @@ def prd_stress_test(
 
         # Update PRD with resolved answers
         console.print("[dim]Updating PRD with resolved ambiguities...[/dim]")
-        updated_content = resolve_ambiguities_into_prd(
-            record.content, result.ambiguities, provider,
-        )
+        try:
+            updated_content = resolve_ambiguities_into_prd(
+                record.content, result.ambiguities, provider,
+            )
+        except Exception as e:
+            # Second LLM call of the command, and it had no handler at all
+            # (#961). Failing here must not lose the answers silently, nor
+            # print a traceback.
+            console.print(
+                f"[red]Error:[/red] Could not apply your answers to the PRD: {e}"
+            )
+            console.print(
+                "[dim]Your answers were not saved and no new version was "
+                "created. Re-run the command to try again.[/dim]"
+            )
+            raise typer.Exit(1)
         # resolve_ambiguities_into_prd returns the ORIGINAL content when the
         # LLM rewrite looks truncated. Creating a version from that would
         # discard the answers the user just typed while printing "✓ PRD updated

codeframe/core/prd.py

@@ -7,6 +7,7 @@
 """
 
 import json
+import logging
 import re
 import uuid
 from dataclasses import dataclass
@@ -16,6 +17,8 @@
 
 from codeframe.core.workspace import Workspace, get_db_connection
 
+logger = logging.getLogger(__name__)
+
 
 def _utc_now() -> datetime:
     """Get current UTC time as timezone-aware datetime."""
@@ -322,18 +325,28 @@ def list_chains(workspace: Workspace) -> list[PrdRecord]:
     conn = get_db_connection(workspace)
     cursor = conn.cursor()
 
-    # Get the latest version for each unique chain_id
+    # Get the latest version for each unique chain.
+    #
+    # Grouping and joining on COALESCE(chain_id, parent_id, id) rather than
+    # chain_id alone (#961): a legacy row can still carry a NULL chain_id, and
+    # SQL's `NULL = NULL` is never true — so the INNER JOIN silently dropped
+    # those PRDs from the list entirely, with no error. The upgrade path
+    # backfills them, but this keeps the read correct for anything it misses
+    # (e.g. a child whose parent row is gone).
     cursor.execute(
         """
         SELECT p.id, p.workspace_id, p.title, p.content, p.metadata, p.created_at,
                p.version, p.parent_id, p.change_summary, p.chain_id
         FROM prds p
         INNER JOIN (
-            SELECT chain_id, MAX(version) as max_version
+            SELECT COALESCE(chain_id, parent_id, id) AS grp,
+                   MAX(version) as max_version
             FROM prds
             WHERE workspace_id = ?
-            GROUP BY chain_id
-        ) latest ON p.chain_id = latest.chain_id AND p.version = latest.max_version
+            GROUP BY COALESCE(chain_id, parent_id, id)
+        ) latest
+          ON COALESCE(p.chain_id, p.parent_id, p.id) = latest.grp
+         AND p.version = latest.max_version
         WHERE p.workspace_id = ?
         ORDER BY p.created_at DESC
         """,
@@ -610,7 +623,19 @@ def create_new_version(
             chain_id=chain_id,
         )
     except Exception:
-        cursor.execute("ROLLBACK")
+        # The rollback is best-effort cleanup, never the story (#961). A bare
+        # cursor.execute("ROLLBACK") raises "cannot rollback - no transaction
+        # is active" whenever the failure happened before BEGIN took effect or
+        # after the transaction already ended — and that replacement exception
+        # propagated instead of the real one, hiding the actual fault.
+        try:
+            cursor.execute("ROLLBACK")
+        except Exception:
+            logger.warning(
+                "Rollback failed while unwinding create_new_version; the "
+                "original error is re-raised.",
+                exc_info=True,
+            )
         raise
     finally:
         conn.close()

codeframe/core/prd_discovery.py

@@ -419,12 +419,29 @@ def submit_answer(self, answer_text: str) -> dict[str, Any]:
 
         Raises:
             ValidationError: If answer is empty
+            DiscoveryError: If the session is already complete, or has no
+                question outstanding
         """
         answer_text = answer_text.strip()
 
         if not answer_text:
             raise ValidationError("Please provide an answer.")
 
+        # There is nothing to answer once discovery has finished (#961). Without
+        # this, a completed or freshly-loaded session passed self._current_question
+        # — None — straight into _validate_answer, which then asked the model to
+        # judge an answer against no question at all.
+        if self._is_complete:
+            raise DiscoveryError(
+                "This discovery session is already complete; there is no "
+                "question to answer. Start a new session to continue."
+            )
+        if not self._current_question:
+            raise DiscoveryError(
+                "No question is currently outstanding for this session. "
+                "Call get_current_question() first."
+            )
+
         # Validate with AI
         validation = self._validate_answer(self._current_question, answer_text)
 
@@ -1174,12 +1191,21 @@ def reset_discovery(
             (_utc_now().isoformat(), session_id, workspace.id),
         )
     else:
-        # Reset most recent non-completed session
+        # Reset ONLY the most recent non-completed session (#961). This used to
+        # update every row matching `state != 'completed'`, so a reset aimed at
+        # the session in front of the user also closed unrelated in-flight
+        # sessions belonging to other PRDs — silently destroying that work,
+        # and contradicting this function's own docstring.
         cursor.execute(
             """
             UPDATE discovery_sessions
             SET state = 'completed', updated_at = ?
-            WHERE workspace_id = ? AND state != 'completed'
+            WHERE id = (
+                SELECT id FROM discovery_sessions
+                WHERE workspace_id = ? AND state != 'completed'
+                ORDER BY updated_at DESC, created_at DESC
+                LIMIT 1
+            )
             """,
             (_utc_now().isoformat(), workspace.id),
         )

codeframe/core/templates.py

@@ -181,6 +181,22 @@ def apply_template(
         issue_number=issue_number,
     )
 
+    # Validate dependency indices BEFORE creating anything (#961). These used
+    # to be filtered with `if 0 <= idx < len(created_tasks)`, so a template
+    # naming a task that does not exist produced a graph that looked fine and
+    # then executed in the wrong order — silently. Checking up front also means
+    # a bad template leaves no half-built set of tasks behind.
+    for position, task_dict in enumerate(task_dicts):
+        for idx in task_dict.get("depends_on_indices", []) or []:
+            if not isinstance(idx, int) or not (0 <= idx < len(task_dicts)):
+                raise ValueError(
+                    f"Template '{template_id}' task #{position} "
+                    f"('{task_dict.get('title', '?')}') declares dependency "
+                    f"index {idx!r}, which is out of range for a template with "
+                    f"{len(task_dicts)} tasks (valid: 0..{len(task_dicts) - 1}). "
+                    "Fix the template's depends_on_indices."
+                )
+
     # Create tasks using v2 API
     created_tasks: list[tuple[tasks.Task, list[int]]] = []
     for task_dict in task_dicts:
@@ -195,17 +211,13 @@ def apply_template(
         )
         created_tasks.append((task, task_dict.get("depends_on_indices", [])))
 
-    # Wire up dependencies using indices -> actual task IDs
+    # Wire up dependencies using indices -> actual task IDs. Every index was
+    # range-checked above, so no filtering is needed (and none should happen —
+    # silently dropping one is the defect this replaced).
     for task, dep_indices in created_tasks:
         if dep_indices:
-            # Map 0-based indices to actual task IDs
-            depends_on_ids = [
-                created_tasks[idx][0].id
-                for idx in dep_indices
-                if 0 <= idx < len(created_tasks)
-            ]
-            if depends_on_ids:
-                tasks.update_depends_on(workspace, task.id, depends_on_ids)
+            depends_on_ids = [created_tasks[idx][0].id for idx in dep_indices]
+            tasks.update_depends_on(workspace, task.id, depends_on_ids)
 
     created_task_ids = [task.id for task, _ in created_tasks]
 

codeframe/core/workspace.py

@@ -36,7 +36,8 @@ def _utc_now() -> datetime:
 # migration path exactly once. Gates #733: steady-state loads skip all DDL.
 # 3: batch_runs.config_reloads (#957).
 # 4: batch_runs.cloud_timeout_minutes (#959).
-SCHEMA_VERSION = 4
+# 5: prds.chain_id backfill for legacy child rows (#961).
+SCHEMA_VERSION = 5
 
 # Per-workspace config file written by the Settings page (issue #556).
 # Owned by the UI layer today; kept here so a future core consumer can
@@ -647,13 +648,32 @@ def _ensure_schema_upgrades(db_path: Path) -> None:
             conn.commit()
         if "chain_id" not in prd_columns:
             cursor.execute("ALTER TABLE prds ADD COLUMN chain_id TEXT")
-            # Backfill chain_id for existing PRDs (set to their own id if no parent)
-            cursor.execute("""
-                UPDATE prds SET chain_id = id
-                WHERE chain_id IS NULL AND parent_id IS NULL
-            """)
             conn.commit()
 
+        # Backfill any PRD still missing a chain_id (#961). This runs on every
+        # upgrade, not only when the column is first added: the original
+        # backfill above set chain_id only WHERE parent_id IS NULL, so every
+        # legacy *child* row kept a NULL chain_id — and because SQL's
+        # `NULL = NULL` never matches, list_chains' join dropped those PRDs
+        # from the workspace entirely. The recursive CTE walks each row up to
+        # its root so children join their parent's chain rather than forming
+        # spurious one-row chains. Rows whose parent is missing keep their own
+        # id (COALESCE), which is the best available answer.
+        cursor.execute("""
+            WITH RECURSIVE root_of(id, root) AS (
+                SELECT id, id FROM prds WHERE parent_id IS NULL
+                UNION ALL
+                SELECT p.id, r.root
+                FROM prds p JOIN root_of r ON p.parent_id = r.id
+            )
+            UPDATE prds
+            SET chain_id = COALESCE(
+                (SELECT root FROM root_of WHERE root_of.id = prds.id), id
+            )
+            WHERE chain_id IS NULL
+        """)
+        conn.commit()
+
         # Add depends_on column to prds table if it doesn't exist
         # Re-check prd_columns as it may have changed
         cursor.execute("PRAGMA table_info(prds)")