diff --git a/tests/test_session.py b/tests/test_session.py index 62079734..cd91f612 100644 --- a/tests/test_session.py +++ b/tests/test_session.py @@ -144,12 +144,21 @@ class TestChatSessionConstruction: class TestPlanExec: """Tests for _exec_plan: unique session-scoped plan file and existing-plan injection.""" - def _run_plan(self, session, prompt, agent_return="# Plan\n\nDo the thing."): + _VALID_PLAN = ( + "## Goal\n\nDo the thing.\n\n" + "## Current State\n\nFile foo.py has bar().\n\n" + "## Plan\n\n1. Edit foo.py line 10.\n\n" + "## Risks\n\nNone." + ) + + def _run_plan(self, session, prompt, agent_return=None): """Invoke _exec_plan with _run_agent patched to avoid LLM calls. Returns (call_id_returned, content_returned, captured_messages) where captured_messages is the agent_messages list passed to _run_agent. """ + if agent_return is None: + agent_return = self._VALID_PLAN captured = {} def fake_run_agent(messages, **kwargs): @@ -175,10 +184,9 @@ class TestPlanExec: """Written plan file contains the agent's output verbatim.""" monkeypatch.chdir(tmp_path) session = _make_session() - plan_content = "## Goal\n\nAdd a new endpoint." - self._run_plan(session, "add endpoint", agent_return=plan_content) + self._run_plan(session, "add endpoint") plan_file = tmp_path / f".plan-{session._ws_id}.md" - assert plan_file.read_text() == plan_content + assert plan_file.read_text() == self._VALID_PLAN def test_two_sessions_produce_different_files(self, tmp_db, tmp_path, monkeypatch): """Two ChatSession instances never collide on the same plan file.""" @@ -262,10 +270,292 @@ class TestPlanExec: """_exec_plan returns (call_id, agent_output).""" monkeypatch.chdir(tmp_path) session = _make_session() - agent_output = "## Goal\n\nBuild it." - call_id, content, _ = self._run_plan(session, "do stuff", agent_return=agent_output) + call_id, content, _ = self._run_plan(session, "do stuff") assert call_id == "test-call-1" - assert content == agent_output + assert content == self._VALID_PLAN + + def test_exec_plan_retries_on_garbage(self, tmp_db, tmp_path, monkeypatch): + """When _run_agent returns garbage, _exec_plan retries once.""" + monkeypatch.chdir(tmp_path) + session = _make_session() + good_plan = ( + "## Goal\n\nAdd feature X.\n\n" + "## Current State\n\nFile foo.py has bar().\n\n" + "## Plan\n\n1. Edit foo.py:bar()\n\n" + "## Risks\n\nNone." + ) + call_count = 0 + + def fake_run_agent(messages, **kwargs): + nonlocal call_count + call_count += 1 + if call_count == 1: + return "Sure, do the thing." + return good_plan + + item = {"call_id": "c1", "prompt": "add feature X"} + with patch.object(session, "_run_agent", side_effect=fake_run_agent): + _, content = session._exec_plan(item) + + assert call_count == 2 + assert "## Goal" in content + + def test_exec_plan_warning_on_double_failure(self, tmp_db, tmp_path, monkeypatch): + """When both attempts produce garbage, content gets a warning prefix.""" + monkeypatch.chdir(tmp_path) + session = _make_session() + + def fake_run_agent(messages, **kwargs): + return "nope" + + item = {"call_id": "c1", "prompt": "add feature X"} + with patch.object(session, "_run_agent", side_effect=fake_run_agent): + _, content = session._exec_plan(item) + + assert content.startswith("[Warning:") + + def test_retry_continues_agent_conversation(self, tmp_db, tmp_path, monkeypatch): + """Retry appends coaching to the same agent_messages list.""" + monkeypatch.chdir(tmp_path) + session = _make_session() + captured_messages: list[list] = [] + + def fake_run_agent(messages, **kwargs): + captured_messages.append(list(messages)) + if len(captured_messages) == 1: + return "garbage" + return ( + "## Goal\n\nDone.\n\n## Current State\n\nx\n\n## Plan\n\n1. x\n\n## Risks\n\nNone." + ) + + item = {"call_id": "c1", "prompt": "add feature X"} + with patch.object(session, "_run_agent", side_effect=fake_run_agent): + session._exec_plan(item) + + assert len(captured_messages) == 2 + # Second call should have more messages (coaching appended) + assert len(captured_messages[1]) > len(captured_messages[0]) + # Last user message in second call is the coaching message + assert "did not follow" in captured_messages[1][-1]["content"] + + +# --------------------------------------------------------------------------- +# Plan validation +# --------------------------------------------------------------------------- + + +class TestPlanValidation: + """Tests for ChatSession._validate_plan quality gate.""" + + GOOD_PLAN = ( + "## Goal\n\nAdd authentication to the API.\n\n" + "## Current State\n\nFile server.py:45 has no auth middleware.\n\n" + "## Plan\n\n1. Add AuthMiddleware to server.py.\n" + "2. Create auth.py with JWT verification.\n\n" + "## Risks\n\nToken expiry handling may need tuning." + ) + + def test_valid_plan_passes(self): + valid, issues = ChatSession._validate_plan(self.GOOD_PLAN, "add auth") + assert valid + assert issues == [] + + def test_too_short_fails(self): + valid, issues = ChatSession._validate_plan("Do the thing.", "do stuff") + assert not valid + assert any("too short" in i for i in issues) + + def test_no_sections_fails(self): + content = "A" * 150 # long enough but no sections + valid, issues = ChatSession._validate_plan(content, "build it") + assert not valid + assert any("missing plan sections" in i for i in issues) + + def test_echo_detection(self): + goal = "deliver a simpsons quote from a specific episode" + content = "Deliver a Simpsons quote from a specific episode" + valid, issues = ChatSession._validate_plan(content, goal) + assert not valid + assert any("echo" in i for i in issues) + + def test_refusal_detection(self): + content = "I cannot create a plan for this task because " + "x" * 100 + valid, issues = ChatSession._validate_plan(content, "do stuff") + assert not valid + assert any("refusal" in i for i in issues) + + def test_partial_sections_passes(self): + """2 out of 4 sections is enough to pass.""" + content = ( + "## Goal\n\nFix the bug in parsing.\n\n" + "## Plan\n\n1. Edit parser.py line 42.\n" + "2. Add boundary check.\n" + "This is enough detail to proceed with confidence." + ) + valid, issues = ChatSession._validate_plan(content, "fix bug") + assert valid + + def test_one_section_fails(self): + """Only 1 out of 4 sections is not enough.""" + content = ( + "## Goal\n\nFix the bug.\n\n" + "We should probably edit parser.py and add some checks " + "to the boundary handling code path for safety." + ) + valid, issues = ChatSession._validate_plan(content, "fix bug") + assert not valid + assert any("missing plan sections" in i for i in issues) + + +# --------------------------------------------------------------------------- +# Plan refinement loop +# --------------------------------------------------------------------------- + + +class TestPlanRefinement: + """Tests for the iterative plan refinement loop in _execute_tools.""" + + GOOD_PLAN = TestPlanValidation.GOOD_PLAN + + def test_feedback_triggers_refinement(self, tmp_db, tmp_path, monkeypatch): + """User feedback causes _refine_plan to run, then approval exits.""" + monkeypatch.chdir(tmp_path) + session = _make_session() + refine_called = [] + + review_responses = iter(["add error handling", ""]) + session.ui = MagicMock(spec_set=NullUI) + session.ui.on_plan_review.side_effect = lambda c: next(review_responses) + session.ui.on_info = MagicMock() + session.ui.on_state_change = MagicMock() + + revised = self.GOOD_PLAN + "\n\n3. Add error handling." + + def fake_refine(content, goal, feedback): + refine_called.append(feedback) + return revised + + with patch.object(session, "_refine_plan", side_effect=fake_refine): + items = [ + { + "func_name": "create_plan", + "call_id": "c1", + "prompt": "add auth", + } + ] + results = [("c1", self.GOOD_PLAN)] + # Manually invoke the post-plan gate portion of _execute_tools. + # We test the loop by calling the gate code directly. + session.auto_approve = False + + original_goal = items[0].get("prompt", "") + output = results[0][1] + refinement_round = 0 + while refinement_round < session._MAX_PLAN_REFINEMENTS: + resp = session.ui.on_plan_review(output) + if resp.lower() in ("n", "no", "reject"): + break + elif resp: + output = session._refine_plan(output, original_goal, resp) + refinement_round += 1 + else: + break + + assert len(refine_called) == 1 + assert refine_called[0] == "add error handling" + assert "error handling" in output + + def test_reject_skips_refinement(self, tmp_db, tmp_path, monkeypatch): + """Rejection exits immediately without calling _refine_plan.""" + monkeypatch.chdir(tmp_path) + session = _make_session() + session.ui = MagicMock(spec_set=NullUI) + session.ui.on_plan_review.return_value = "reject" + + with patch.object(session, "_refine_plan") as mock_refine: + output = self.GOOD_PLAN + resp = session.ui.on_plan_review(output) + if resp.lower() in ("n", "no", "reject"): + output += "\n\n---\nUser REJECTED" + elif resp: + output = session._refine_plan(output, "g", resp) + + mock_refine.assert_not_called() + assert "REJECTED" in output + + def test_approve_skips_refinement(self, tmp_db, tmp_path, monkeypatch): + """Empty response (enter) approves without refinement.""" + monkeypatch.chdir(tmp_path) + session = _make_session() + session.ui = MagicMock(spec_set=NullUI) + session.ui.on_plan_review.return_value = "" + + with patch.object(session, "_refine_plan") as mock_refine: + output = self.GOOD_PLAN + resp = session.ui.on_plan_review(output) + if resp.lower() in ("n", "no", "reject"): + output += "\n\n---\nUser REJECTED" + elif resp: + output = session._refine_plan(output, "g", resp) + + mock_refine.assert_not_called() + assert "REJECTED" not in output + + def test_max_refinement_rounds(self, tmp_db, tmp_path, monkeypatch): + """Loop stops after _MAX_PLAN_REFINEMENTS rounds with a final review.""" + monkeypatch.chdir(tmp_path) + session = _make_session() + session.ui = MagicMock(spec_set=NullUI) + session.ui.on_plan_review.return_value = "more detail please" + session.ui.on_info = MagicMock() + + refine_count = 0 + + def fake_refine(content, goal, feedback): + nonlocal refine_count + refine_count += 1 + return content + f"\n(revision {refine_count})" + + with patch.object(session, "_refine_plan", side_effect=fake_refine): + output = self.GOOD_PLAN + original_goal = "add auth" + refinement_round = 0 + while True: + resp = session.ui.on_plan_review(output) + if ( + resp.lower() in ("n", "no", "reject") + or not resp + or refinement_round >= session._MAX_PLAN_REFINEMENTS + ): + break + output = session._refine_plan(output, original_goal, resp) + refinement_round += 1 + + assert refine_count == session._MAX_PLAN_REFINEMENTS + # User gets one extra review call after max rounds (the final prompt) + assert session.ui.on_plan_review.call_count == session._MAX_PLAN_REFINEMENTS + 1 + + def test_refine_plan_message_structure(self, tmp_db, tmp_path, monkeypatch): + """_refine_plan passes system + prior plan + feedback to _run_agent.""" + monkeypatch.chdir(tmp_path) + session = _make_session() + captured = {} + + def fake_run_agent(messages, **kwargs): + captured["messages"] = list(messages) + return self.GOOD_PLAN + + with patch.object(session, "_run_agent", side_effect=fake_run_agent): + session._refine_plan(self.GOOD_PLAN, "add auth", "add tests too") + + msgs = captured["messages"] + assert msgs[0]["role"] == "system" + assert msgs[1]["role"] == "assistant" + assert msgs[1]["tool_calls"][0]["function"]["name"] == "create_plan" + assert msgs[2]["role"] == "tool" + assert msgs[2]["content"] == self.GOOD_PLAN + assert msgs[3]["role"] == "user" + assert "add tests too" in msgs[3]["content"] # --------------------------------------------------------------------------- diff --git a/turnstone/cli.py b/turnstone/cli.py index c3a2ad8c..036d3dd4 100644 --- a/turnstone/cli.py +++ b/turnstone/cli.py @@ -204,7 +204,8 @@ class TerminalUI(SessionUI): try: prompt_text = ( f" \001{BOLD}\002Plan ready.\001{RESET}\002 " - f"\001{DIM}\002[enter to approve, or give feedback]\001{RESET}\002 " + f"\001{DIM}\002[enter to approve, feedback to amend, " + f"ctrl-c to reject]\001{RESET}\002 " ) resp = input(prompt_text).strip() except EOFError: diff --git a/turnstone/core/session.py b/turnstone/core/session.py index af5af7fc..f5cbd011 100644 --- a/turnstone/core/session.py +++ b/turnstone/core/session.py @@ -1551,28 +1551,73 @@ class ChatSession: with concurrent.futures.ThreadPoolExecutor(max_workers=4) as pool: results = list(pool.map(run_one, items)) - # Post-plan gate: prompt user on main thread after plan completes + # Post-plan gate: iterative review loop. When the user gives + # feedback the plan agent re-runs and the revised plan is shown + # again, up to _MAX_PLAN_REFINEMENTS rounds. for i, item in enumerate(items): - if ( - item.get("func_name") == "create_plan" - and not item.get("error") - and not item.get("denied") - and not self.auto_approve - ): - cid, output = results[i] - assert isinstance(output, str) # plan always returns text - # Let the UI present the plan for review - self._emit_state("attention") - resp = self.ui.on_plan_review(output) - self._emit_state("running") - if resp.lower() in ("n", "no", "reject"): - output += ( - "\n\n---\nUser REJECTED this plan. Do not proceed " - "with implementation. Ask the user what they want instead." - ) - elif resp: - output += f"\n\n---\nUser feedback on this plan: {resp}" - results[i] = (cid, output) + if item.get("func_name") != "create_plan" or item.get("error") or item.get("denied"): + continue + + cid, output = results[i] + assert isinstance(output, str) # plan always returns text + plan_path = f".plan-{self._ws_id}.md" + + if not self.auto_approve: + original_goal = item.get("prompt", "") + + refinement_round = 0 + while True: + self._emit_state("attention") + resp = self.ui.on_plan_review(output) + self._emit_state("running") + + if resp.lower() in ("n", "no", "reject"): + output += ( + "\n\n---\nUser REJECTED this plan. Do not " + "proceed with implementation. Ask the user " + "what they want instead." + ) + break + elif not resp: + break # empty response = approve + elif refinement_round >= self._MAX_PLAN_REFINEMENTS: + self.ui.on_info("[plan] max refinement rounds reached") + break + else: + # Re-run plan agent with user feedback. + # Strip any internal warning prefix so the + # agent sees the raw plan content. + raw = output + _warn = "[Warning: plan may be incomplete or poorly structured]\n\n" + if raw.startswith(_warn): + raw = raw[len(_warn) :] + try: + output = self._refine_plan( + raw, + original_goal, + resp, + ) + refinement_round += 1 + except (KeyboardInterrupt, GenerationCancelled): + output += "\n\n---\n(plan refinement interrupted)" + break + except Exception as e: + self.ui.on_info(f"[plan refinement error] {e}") + output += f"\n\n---\nUser feedback: {resp}" + break + # Loop continues → show revised plan to user + + # Write final version to disk (overwrites initial write) + try: + with open(plan_path, "w") as f: + f.write(output) + except OSError: + pass + + # Always include file path in the tool result so the + # outer model knows where the plan lives on disk. + output += f"\n\n---\nPlan saved to `{plan_path}`" + results[i] = (cid, output) return results, user_feedback @@ -2811,6 +2856,61 @@ class ChatSession: "and functions in every step." ) + _MIN_PLAN_LENGTH = 100 + _PLAN_REQUIRED_SECTIONS = ("## goal", "## current state", "## plan", "## risks") + _MIN_PLAN_SECTIONS = 2 + _MAX_PLAN_REFINEMENTS = 5 + + @staticmethod + def _validate_plan(content: str, goal: str) -> tuple[bool, list[str]]: + """Check if plan output meets minimum quality bar. + + Returns ``(valid, issues)`` where *issues* is a list of + human-readable problem descriptions (empty when valid). + """ + issues: list[str] = [] + stripped = content.strip() + stripped_lower = stripped.lower() + + # 1. Minimum length + if len(stripped) < ChatSession._MIN_PLAN_LENGTH: + issues.append( + f"too short ({len(stripped)} chars, minimum {ChatSession._MIN_PLAN_LENGTH})" + ) + + # 2. Section structure + found_sections = sum( + 1 for section in ChatSession._PLAN_REQUIRED_SECTIONS if section in stripped_lower + ) + if found_sections < ChatSession._MIN_PLAN_SECTIONS: + issues.append( + f"missing plan sections (found {found_sections}/" + f"{len(ChatSession._PLAN_REQUIRED_SECTIONS)}, " + f"need at least {ChatSession._MIN_PLAN_SECTIONS})" + ) + + # 3. Echo detection: plan is basically just the goal repeated + goal_stripped = goal.strip().lower() + if ( + goal_stripped + and len(stripped) < len(goal_stripped) * 2 + and goal_stripped in stripped_lower + ): + issues.append("plan appears to echo the goal without elaboration") + + # 4. Refusal detection + refusal_starts = ( + "i cannot", + "i'm sorry", + "i am sorry", + "error:", + "i can't", + ) + if any(stripped_lower.startswith(r) for r in refusal_starts): + issues.append("plan appears to be a refusal or error") + + return (len(issues) == 0, issues) + def _exec_plan(self, item: dict[str, Any]) -> tuple[str, str]: """Run a planning agent and write the result to .plan-.md.""" call_id, prompt = item["call_id"], item["prompt"] @@ -2853,6 +2953,41 @@ class ChatSession: self.ui.on_info(f"[plan error] {e}") return call_id, f"Plan error: {e}" + # Validate plan quality — retry once with coaching on failure + valid, issues = self._validate_plan(content, prompt) + if not valid: + self.ui.on_info(f"[plan] quality issues: {', '.join(issues)}") + preview = content[:200] + ("..." if len(content) > 200 else "") + coaching = ( + "Your previous response did not follow the required plan " + "format. A valid plan should include at least two of " + "these markdown sections:\n" + "## Goal (1-2 sentences)\n" + "## Current State (files/line numbers found)\n" + "## Plan (numbered steps with file names and functions)\n" + "## Risks (edge cases and unknowns)\n\n" + f'Your previous response was: "{preview}"\n\n' + "Please try again. Explore the codebase first, then write " + "the plan." + ) + agent_messages.append({"role": "user", "content": coaching}) + try: + content = self._run_agent( + agent_messages, + label="plan", + reasoning_effort="high", + ) + except (KeyboardInterrupt, GenerationCancelled): + return call_id, "(plan interrupted by user)" + except Exception as e: + self.ui.on_info(f"[plan retry error] {e}") + return call_id, f"Plan error: {e}" + + valid2, issues2 = self._validate_plan(content, prompt) + if not valid2: + self.ui.on_info(f"[plan] still has issues after retry: {', '.join(issues2)}") + content = "[Warning: plan may be incomplete or poorly structured]\n\n" + content + # Write to file separately — always return content even if write fails try: with open(plan_path, "w") as f: @@ -2863,6 +2998,60 @@ class ChatSession: return call_id, content + def _refine_plan( + self, + original_content: str, + original_goal: str, + feedback: str, + ) -> str: + """Re-run the plan agent incorporating user feedback.""" + tc_id = f"plan_refine_{uuid.uuid4().hex[:8]}" + agent_messages: list[dict[str, Any]] = [ + {"role": "system", "content": self._PLAN_IDENTITY}, + { + "role": "assistant", + "content": None, + "tool_calls": [ + { + "id": tc_id, + "type": "function", + "function": { + "name": "create_plan", + "arguments": json.dumps({"goal": original_goal}), + }, + } + ], + }, + { + "role": "tool", + "tool_call_id": tc_id, + "content": original_content, + }, + { + "role": "user", + "content": ( + "The user reviewed this plan and provided feedback:\n\n" + f"{feedback}\n\n" + "Please revise the plan accordingly. Keep the same " + "format (## Goal, ## Current State, ## Plan, ## Risks) " + "and address the feedback." + ), + }, + ] + + self.ui.on_info("[plan] revising based on feedback...") + content = self._run_agent( + agent_messages, + label="plan", + reasoning_effort="high", + ) + + valid, issues = self._validate_plan(content, original_goal) + if not valid: + self.ui.on_info(f"[plan] revised plan has issues: {', '.join(issues)}") + + return content + def _exec_remember(self, item: dict[str, Any]) -> tuple[str, str]: """Save a persistent memory.""" call_id, key, value = item["call_id"], item["key"], item["value"] diff --git a/turnstone/mq/bridge.py b/turnstone/mq/bridge.py index 8bc3df52..c395f555 100644 --- a/turnstone/mq/bridge.py +++ b/turnstone/mq/bridge.py @@ -709,6 +709,11 @@ class Bridge: def _wait_plan() -> None: try: raw_resp = self._broker.pop_response(request_id, timeout=self._approval_timeout) + # Clear pending entry *before* posting response so that + # a subsequent plan review event (from the refinement + # loop) is not skipped by the duplicate guard. + with self._lock: + self._pending_plan_reviews.pop(ws_id, None) if raw_resp: resp_msg = InboundMessage.from_json(raw_resp) feedback = getattr(resp_msg, "feedback", "") @@ -716,9 +721,16 @@ class Bridge: else: log.warning("Plan review timeout for ws %s — rejecting", ws_id) self._http.post("/v1/api/plan", json={"feedback": "reject", "ws_id": ws_id}) - finally: + except Exception: with self._lock: self._pending_plan_reviews.pop(ws_id, None) + # Best-effort rejection so the server doesn't hang + with contextlib.suppress(Exception): + self._http.post( + "/v1/api/plan", + json={"feedback": "reject", "ws_id": ws_id}, + ) + raise threading.Thread(target=self._run_in_context(_wait_plan), daemon=True).start() diff --git a/turnstone/shared_static/base.css b/turnstone/shared_static/base.css index 5a21af98..8ea3cbb6 100644 --- a/turnstone/shared_static/base.css +++ b/turnstone/shared_static/base.css @@ -28,6 +28,7 @@ --yellow: #fbbf24; --cyan: #67e8f9; --magenta: #c084fc; + --on-color: var(--bg); /* Glow variants for LED effects */ --green-glow: rgba(52, 211, 153, 0.25); @@ -66,6 +67,7 @@ --yellow: #b45309; --cyan: #0e7490; --magenta: #7c3aed; + --on-color: #ffffff; --green-glow: rgba(4, 120, 87, 0.25); --red-glow: rgba(220, 38, 38, 0.25); --yellow-glow: rgba(180, 83, 9, 0.25); diff --git a/turnstone/ui/static/app.js b/turnstone/ui/static/app.js index 7ac62d2a..e6686e1b 100644 --- a/turnstone/ui/static/app.js +++ b/turnstone/ui/static/app.js @@ -1570,20 +1570,45 @@ function scrollToBottom(force) { } // --- Plan review dialog --- +var _planContent = ""; function showPlanDialog(content) { + _planContent = content; document.getElementById("plan-content").textContent = content; - document.getElementById("plan-feedback").value = ""; + var feedbackEl = document.getElementById("plan-feedback"); + feedbackEl.value = ""; + _updatePlanRejectBtn(); + inputEl.disabled = true; + sendBtn.disabled = true; document.getElementById("plan-overlay").classList.add("active"); setTimeout(function () { - document.getElementById("plan-feedback").focus(); + feedbackEl.focus(); }, 50); } +function _updatePlanRejectBtn() { + var btn = document.getElementById("btn-plan-reject"); + var hasFeedback = + document.getElementById("plan-feedback").value.trim().length > 0; + btn.innerHTML = hasFeedback + ? 'Esc Amend' + : 'Esc Reject'; + btn.style.background = hasFeedback ? "var(--accent)" : ""; + btn.style.color = hasFeedback ? "var(--on-color)" : ""; + btn.onclick = function () { + resolvePlan(hasFeedback ? "" : "reject"); + }; +} + function resolvePlan(defaultFeedback) { let feedback = document.getElementById("plan-feedback").value.trim(); if (!feedback && defaultFeedback) feedback = defaultFeedback; document.getElementById("plan-overlay").classList.remove("active"); + inputEl.disabled = false; + sendBtn.disabled = false; inputEl.focus(); + + // Critical: fire the API call first — this unblocks the server. + // The inline rendering below is cosmetic and must never prevent it. authFetch("/v1/api/plan", { method: "POST", headers: { "Content-Type": "application/json" }, @@ -1591,6 +1616,65 @@ function resolvePlan(defaultFeedback) { }).catch(function (err) { addErrorMessage("Connection error: " + err.message); }); + + // Render plan inline in the chat (best-effort) + try { + var isReject = feedback === "reject"; + var isAmend = feedback && !isReject; + var action = isReject ? "rejected" : isAmend ? "amending" : "approved"; + _addInlinePlan(_planContent, action, feedback); + } catch (err) { + console.error("Failed to render inline plan:", err); + addInfoMessage("Plan " + action); + } + + // Show spinner while the model processes the plan result + setBusy(true); + addThinkingIndicator(); +} + +function _addInlinePlan(content, action, feedback) { + if (!content) return; + var wrapper = document.createElement("div"); + wrapper.className = "plan-inline"; + + var header = document.createElement("div"); + header.className = "plan-inline-header"; + var label = + action === "rejected" + ? "Plan rejected" + : action === "amending" + ? "Plan — amending" + : "Plan approved"; + header.innerHTML = + '' + label + ""; + wrapper.appendChild(header); + + var body = document.createElement("div"); + body.className = "plan-inline-body"; + try { + body.innerHTML = renderMarkdown(content); + } catch (e) { + body.textContent = content; + } + if (content.split("\n").length > 12) { + makeCollapsible(body); + body.setAttribute( + "aria-label", + "Plan content (collapsed). Activate to expand.", + ); + } + wrapper.appendChild(body); + + if (feedback && action === "amending") { + var fb = document.createElement("div"); + fb.className = "plan-inline-feedback"; + fb.textContent = "Feedback: " + feedback; + wrapper.appendChild(fb); + } + + messagesEl.appendChild(wrapper); + scrollToBottom(); } // --- Send message --- @@ -1647,6 +1731,9 @@ function autoResize() { } inputEl.addEventListener("input", autoResize); +document + .getElementById("plan-feedback") + .addEventListener("input", _updatePlanRejectBtn); inputEl.addEventListener("keydown", function (e) { if (e.key === "Enter" && !e.shiftKey) { e.preventDefault(); @@ -1749,7 +1836,9 @@ document.addEventListener("keydown", function (e) { resolvePlan(""); } else if (e.key === "Escape") { e.preventDefault(); - resolvePlan("reject"); + var hasFb = + document.getElementById("plan-feedback").value.trim().length > 0; + resolvePlan(hasFb ? "" : "reject"); } else if (e.key === "Tab") { var focusable = document.querySelectorAll( "#plan-dialog input, #plan-dialog button", diff --git a/turnstone/ui/static/index.html b/turnstone/ui/static/index.html index 3362f597..ce94725e 100644 --- a/turnstone/ui/static/index.html +++ b/turnstone/ui/static/index.html @@ -76,10 +76,10 @@ diff --git a/turnstone/ui/static/style.css b/turnstone/ui/static/style.css index 7398aac2..ce66979c 100644 --- a/turnstone/ui/static/style.css +++ b/turnstone/ui/static/style.css @@ -497,11 +497,73 @@ cursor: pointer; font-weight: 600; letter-spacing: 0.02em; - transition: filter 0.15s; + display: inline-flex; + align-items: center; + gap: 5px; + transition: filter 0.15s, background 0.2s; } #plan-buttons button:hover { filter: brightness(1.1); } -#btn-plan-approve { background: var(--green); color: var(--bg); } -#btn-plan-reject { background: var(--red); color: var(--bg); } +#plan-buttons button:focus-visible { outline: 2px solid var(--fg-bright); outline-offset: 2px; } +#plan-buttons button .key { + display: inline-block; + background: rgba(0,0,0,0.2); + border-radius: var(--radius-sm); + padding: 0 4px; + font-size: 10px; + font-weight: 600; +} +#btn-plan-approve { background: var(--green); color: var(--on-color); } +#btn-plan-reject { background: var(--red); color: var(--on-color); } + +/* Inline plan block (rendered in chat after plan review) */ +.plan-inline { + margin: 8px 0; + border: 1px solid var(--border); + border-radius: var(--radius); + overflow: hidden; +} +.plan-inline-header { + padding: 6px 12px; + background: var(--bg-surface); + border-bottom: 1px solid var(--border); + font-size: 12px; + font-family: var(--font-display); + font-weight: 600; + letter-spacing: 0.02em; +} +.plan-inline-label.plan-approved { color: var(--green); } +.plan-inline-label.plan-rejected { color: var(--red); } +.plan-inline-label.plan-amending { color: var(--accent); } +.plan-inline-body { + padding: 10px 14px; + background: var(--code-bg); + font-size: 12px; + line-height: 1.6; + max-height: 300px; + overflow-y: auto; +} +.plan-inline-body.collapsed { max-height: 150px; position: relative; } +.plan-inline-body.collapsed::after { + content: 'click to expand'; + position: absolute; + bottom: 0; left: 0; right: 0; + text-align: center; + padding: 8px 0 4px; + background: linear-gradient(transparent, var(--code-bg)); + color: var(--fg-dim); + font-size: 11px; + cursor: pointer; +} +.plan-inline-body h2, .plan-inline-body h3 { font-size: 13px; margin: 10px 0 4px; color: var(--accent); } +.plan-inline-body p { margin: 4px 0; } +.plan-inline-body ol, .plan-inline-body ul { margin: 4px 0; padding-left: 20px; } +.plan-inline-feedback { + padding: 6px 12px; + border-top: 1px solid var(--border); + font-size: 12px; + color: var(--accent); + font-style: italic; +} /* ========================================================================== Focus indicators — server-specific overrides