Merge branch 'goal/advisor' of clanker/clanker-saddle into web-stack

src/Services/AdvisorService.vala

@@ -512,8 +512,14 @@ namespace ClankerSaddle.Services {
                     @"LLM request failed: $(e.message)");
             }
             if (message.status_code >= 400) {
+                // The provider's own reason (an invalid-schema or
+                // quota refusal names the exact field) must reach the
+                // chat's error row — a bare status code leaves the
+                // human debugging blind. The body is already in the
+                // read buffer; clamp it so a wall of HTML stays short.
                 throw new Error(Quark.from_string("clanker-saddle"), 81,
-                    @"LLM endpoint answered HTTP $(message.status_code)");
+                    @"LLM endpoint answered HTTP $(message.status_code)"
+                    + @"$(error_body_suffix(raw))");
             }
             // get_data() hands back an unterminated array — casting it
             // to string would read past the buffer until an unrelated
@@ -530,6 +536,52 @@ namespace ClankerSaddle.Services {
             return parse_reply((string) terminated);
         }
 
+        /**
+         * The ": <body>" tail for an HTTP >= 400 error message: the
+         * response body decoded (NUL-terminated first —
+         * {@link chat_once}'s note) and clamped to ~500 characters,
+         * collapsing whitespace so a JSON reason reads as one line.
+         * An unreadable body contributes nothing but a note.
+         */
+        private static string error_body_suffix(Bytes? raw) {
+            var data = raw != null ? ((!)raw).get_data() : new uint8[0];
+            if (data.length == 0) {
+                return " (no response body)";
+            }
+            var terminated = new uint8[data.length + 1];
+            for (var index = 0; index < data.length; index++) {
+                terminated[index] = data[index];
+            }
+            terminated[data.length] = '\0';
+            var text = ((string) terminated);
+            // Collapse ALL whitespace runs (providers send JSON on one
+            // line, proxies send walls of indented HTML) to single
+            // spaces so the error row stays a readable one-liner.
+            var collapsed = new StringBuilder();
+            bool in_space = true;
+            for (var index = 0; index < text.length; index++) {
+                char symbol = text[index];
+                if (symbol == ' ' || symbol == '\n' || symbol == '\r'
+                        || symbol == '\t') {
+                    if (!in_space) {
+                        collapsed.append_c(' ');
+                        in_space = true;
+                    }
+                    continue;
+                }
+                collapsed.append_c(symbol);
+                in_space = false;
+            }
+            var flat = collapsed.str.strip();
+            if (flat.length == 0) {
+                return " (empty response body)";
+            }
+            if (flat.length > 500) {
+                flat = flat.substring(0, 500) + "…";
+            }
+            return @": $(flat)";
+        }
+
         /**
          * Parses a chat-completions response body: the first choice's
          * content, tool calls (id, name, raw arguments) and
@@ -559,10 +611,21 @@ namespace ClankerSaddle.Services {
                 throw new Error(Quark.from_string("clanker-saddle"), 82,
                     "LLM response choices array is empty");
             }
+            if (choices.get_element(0).get_node_type() != Json.NodeType.OBJECT) {
+                throw new Error(Quark.from_string("clanker-saddle"), 82,
+                    "LLM response's first choice is not an object");
+            }
             var first = choices.get_object_element(0);
-            if (!first.has_member("message")) {
+            // The message member may be a JSON null (or a non-object)
+            // — has_member is true but get_object_member hands back
+            // null, and every later has_member on it CRITICALs. Treat
+            // a missing/Null/non-object message as the parse failure
+            // it is.
+            if (!first.has_member("message")
+                    || first.get_member("message").get_node_type()
+                        != Json.NodeType.OBJECT) {
                 throw new Error(Quark.from_string("clanker-saddle"), 82,
-                    "LLM choice has no message");
+                    "LLM choice has no message object");
             }
             var reply = new AdvisorReply();
             var choice_message = first.get_object_member("message");
@@ -1180,97 +1243,129 @@ namespace ClankerSaddle.Services {
         /**
          * The tools array sent with every request: the read tools,
          * ask_question and the two suggest tools, described for
-         * function calling.
+         * function calling. Every parameter is a structured JSON
+         * Schema property (the daemon ToolLoop shape) — strict
+         * providers validate this array, so no prose ever lands in a
+         * `type` field — and every schema carries its `required`
+         * array (empty for the parameter-less read tools).
          */
         public static Json.Array tools_array() {
             var tools = new Json.Array();
+
+            var projects_properties = new Json.Object();
             tools.add_object_element(tool_definition("list_projects",
                 "List every project registered in this clanker-saddle fleet.",
-                new string[0], new string[0], new string[0]));
+                new string[0], projects_properties));
+
+            var project_properties = new Json.Object();
+            project_properties.set_object_member("project_id", integer_property(
+                "The project's id"));
             tools.add_object_element(tool_definition("get_project",
                 "One project's full record by id.",
-                new string[] { "project_id" }, new string[] { "integer" },
-                new string[] { "The project's id" }));
+                new string[] { "project_id" }, project_properties));
+
+            var tasks_properties = new Json.Object();
+            tasks_properties.set_object_member("status", string_property(
+                "Optional status filter (draft/queued/working/parked/suspended/done/failed)"));
+            tasks_properties.set_object_member("project_id", integer_property(
+                "Optional project id filter (0 = standalone tasks excluded)"));
             tools.add_object_element(tool_definition("list_tasks",
                 "List tasks, optionally filtered by status and project.",
-                new string[] { "status", "project_id" },
-                new string[] { "string", "integer" },
-                new string[] {
-                    "Optional status filter (draft/queued/working/parked/suspended/done/failed)",
-                    "Optional project id filter (0 = standalone tasks excluded)"
-                }));
+                new string[0], tasks_properties));
+
+            var task_properties = new Json.Object();
+            task_properties.set_object_member("task_id", integer_property(
+                "The task's id"));
             tools.add_object_element(tool_definition("get_task",
                 "One task's full record by id (description, status, mode, branch).",
-                new string[] { "task_id" }, new string[] { "integer" },
-                new string[] { "The task's id" }));
+                new string[] { "task_id" }, task_properties));
+
+            var goals_properties = new Json.Object();
+            goals_properties.set_object_member("project_id", integer_property(
+                "Optional project id filter"));
             tools.add_object_element(tool_definition("list_goals",
                 "List goals, optionally filtered by project.",
-                new string[] { "project_id" }, new string[] { "integer" },
-                new string[] { "Optional project id filter" }));
+                new string[0], goals_properties));
+
+            var goal_properties = new Json.Object();
+            goal_properties.set_object_member("goal_id", integer_property(
+                "The goal's id"));
             tools.add_object_element(tool_definition("get_goal",
                 "One goal's full record by id (brief, plan, branch).",
-                new string[] { "goal_id" }, new string[] { "integer" },
-                new string[] { "The goal's id" }));
+                new string[] { "goal_id" }, goal_properties));
+
+            var rules_properties = new Json.Object();
             tools.add_object_element(tool_definition("list_rules",
                 "List every rule (reusable instructions attached to projects/tasks).",
-                new string[0], new string[0], new string[0]));
+                new string[0], rules_properties));
+
+            var discoveries_properties = new Json.Object();
+            discoveries_properties.set_object_member("query", string_property(
+                "Case-insensitive substring; empty lists the newest rows"));
             tools.add_object_element(tool_definition("search_discoveries",
                 "Search the fleet's discoveries ledger (non-project facts: compiler quirks, library behaviours, tool gotchas) by substring.",
-                new string[] { "query" }, new string[] { "string" },
-                new string[] { "Case-insensitive substring; empty lists the newest rows" }));
+                new string[] { "query" }, discoveries_properties));
+
+            var capabilities_properties = new Json.Object();
             tools.add_object_element(tool_definition("list_capabilities",
                 "List the fleet's capability tags.",
-                new string[0], new string[0], new string[0]));
+                new string[0], capabilities_properties));
+
+            var question_properties = new Json.Object();
+            question_properties.set_object_member("question", string_property(
+                "The question to ask"));
+            question_properties.set_object_member("options", string_array_property(
+                "The concrete options (may be empty for free text)"));
             tools.add_object_element(tool_definition("ask_question",
                 "Ask the user a question with multiple-choice options. The turn parks until they answer. An empty options array asks a free-text question.",
-                new string[] { "question", "options" },
-                new string[] { "string", "array of string" },
-                new string[] { "The question to ask", "The concrete options (may be empty for free text)" }));
+                new string[] { "question" }, question_properties));
+
+            var suggest_task_properties = new Json.Object();
+            suggest_task_properties.set_object_member("title", string_property(
+                "Short task title"));
+            suggest_task_properties.set_object_member("mandate", string_property(
+                "The task's mandate: what to do, on which project, with what verification"));
+            suggest_task_properties.set_object_member("mode", enum_property(
+                "Work mode: implement / investigate / review / test",
+                new string[] { "implement", "investigate", "review", "test" }));
+            suggest_task_properties.set_object_member("project_id", integer_property(
+                "The project id (optional)"));
+            suggest_task_properties.set_object_member("priority", integer_property(
+                "Priority 1-9, 5 default (optional)"));
             tools.add_object_element(tool_definition("suggest_task",
                 "Suggest a task for the user to approve. INERT until they approve it in the chat UI — nothing is created by this call.",
-                new string[] { "title", "mandate", "mode", "project_id", "priority" },
-                new string[] { "string", "string", "string", "integer", "integer" },
-                new string[] {
-                    "Short task title",
-                    "The task's mandate: what to do, on which project, with what verification",
-                    "Work mode: implement / investigate / review / test",
-                    "The project id (optional)",
-                    "Priority 1-9, 5 default (optional)"
-                }));
+                new string[] { "title", "mandate" }, suggest_task_properties));
+
+            var suggest_goal_properties = new Json.Object();
+            suggest_goal_properties.set_object_member("title", string_property(
+                "Short goal title"));
+            suggest_goal_properties.set_object_member("brief", string_property(
+                "The goal brief: what this goal should achieve"));
+            suggest_goal_properties.set_object_member("project_id", integer_property(
+                "The primary project id"));
+            suggest_goal_properties.set_object_member("branch_name", string_property(
+                "Branch name for the goal (optional)"));
             tools.add_object_element(tool_definition("suggest_goal",
                 "Suggest a goal for the user to approve. INERT until they approve it in the chat UI; an approved goal is created as an unqueued draft.",
-                new string[] { "title", "brief", "project_id", "branch_name" },
-                new string[] { "string", "string", "integer", "string" },
-                new string[] {
-                    "Short goal title",
-                    "The goal brief: what this goal should achieve",
-                    "The primary project id",
-                    "Branch name for the goal (optional)"
-                }));
+                new string[] { "title", "brief", "project_id" },
+                suggest_goal_properties));
             return tools;
         }
 
         /**
          * One `{type: function, function: {name, description,
-         * parameters}}` definition. `required` is the first
-         * `names.len`-many parameters when `required_count` > 0 —
-         * here every declared parameter of the ask/suggest tools that
-         * matter; simple read tools pass none.
+         * parameters}}` definition. `parameters` is the caller's
+         * properties object wrapped in the `object` schema — the
+         * structured builders ({@link string_property},
+         * {@link integer_property}, {@link string_array_property},
+         * {@link enum_property}) keep every `type` field a real
+         * JSON-Schema type: strict providers validate the tools array
+         * and reject anything else with an HTTP 400 before a single
+         * message is ever answered.
          */
         private static Json.Object tool_definition(string name, string description,
-                string[] names, string[] types, string[] descriptions) {
-            var properties = new Json.Object();
-            for (var index = 0; index < names.length; index++) {
-                var property = new Json.Object();
-                property.set_string_member("type",
-                    index < types.length ? types[index] : "string");
-                property.set_string_member("description",
-                    index < descriptions.length ? descriptions[index] : "");
-                properties.set_object_member(names[index], property);
-            }
-            var parameters = new Json.Object();
-            parameters.set_string_member("type", "object");
-            parameters.set_object_member("properties", properties);
+                string[] required, Json.Object properties) {
+            var parameters = object_schema(required, properties);
             var wrapper = new Json.Object();
             wrapper.set_string_member("type", "function");
             var function = new Json.Object();
@@ -1280,5 +1375,72 @@ namespace ClankerSaddle.Services {
             wrapper.set_object_member("function", function);
             return wrapper;
         }
+
+        // --- schema property builders (the daemon ToolLoop shape) ---
+
+        /** A `{"type":"string","description":…}` property. */
+        private static Json.Object string_property(string description) {
+            var property = new Json.Object();
+            property.set_string_member("type", "string");
+            property.set_string_member("description", description);
+            return property;
+        }
+
+        /** A `{"type":"integer","description":…}` property. */
+        private static Json.Object integer_property(string description) {
+            var property = new Json.Object();
+            property.set_string_member("type", "integer");
+            property.set_string_member("description", description);
+            return property;
+        }
+
+        /**
+         * A `{"type":"array","items":{"type":"string"},…}` property —
+         * a REAL array schema, never a prose "type" string.
+         */
+        private static Json.Object string_array_property(string description) {
+            var items = new Json.Object();
+            items.set_string_member("type", "string");
+            var property = new Json.Object();
+            property.set_string_member("type", "array");
+            property.set_object_member("items", items);
+            property.set_string_member("description", description);
+            return property;
+        }
+
+        /** A `{"type":"string","enum":[…],…}` property. */
+        private static Json.Object enum_property(string description,
+                string[] values) {
+            var property = string_property(description);
+            var enumeration = new Json.Array();
+            foreach (var value in values) {
+                enumeration.add_string_element(value);
+            }
+            var node = new Json.Node(Json.NodeType.ARRAY);
+            node.init_array(enumeration);
+            property.set_member("enum", node);
+            return property;
+        }
+
+        /**
+         * The `{type: object, properties, required}` wrapper.
+         * `required` is always present (an empty array for the
+         * parameter-less read tools) — some providers reject its
+         * absence outright.
+         */
+        private static Json.Object object_schema(string[] required,
+                Json.Object properties) {
+            var schema = new Json.Object();
+            schema.set_string_member("type", "object");
+            schema.set_object_member("properties", properties);
+            var required_array = new Json.Array();
+            foreach (var name in required) {
+                required_array.add_string_element(name);
+            }
+            var node = new Json.Node(Json.NodeType.ARRAY);
+            node.init_array(required_array);
+            schema.set_member("required", node);
+            return schema;
+        }
     }
 }

src/actions/AdvisorActions.vala

@@ -286,31 +286,41 @@ namespace ClankerSaddle {
      * The chat page's pulse-driven refresh (goal 27): rebuilds the
      * `chat` slot from current rows so a running turn's appends —
      * tool calls, the assistant reply, a parked question, a
-     * suggestion — stream in without a reload. Navigates nowhere.
-     * Like the dashboard's refresh it is authored unsealed (GET): the
-     * page it serves is already the viewer's own.
+     * suggestion — stream in without a reload. The chat id is SEALED
+     * (author_private, exactly like send/delete): statum.act carries
+     * GET parameters as a QUERY STRING, which request.form never
+     * sees, so an id read from a form field would parse 0 on every
+     * pulse bump and bounce the page to /chats — the redirect loop.
+     * Navigates only when the chat genuinely no longer exists (or
+     * never was the viewer's).
      */
-    public class RefreshAdvisorChatAction : StatumAction {
+    public class RefreshAdvisorChatAction : TypedStatumAction<AdvisorChatActionPrivate> {
 
         private ChatService chat_service = inject<ChatService>();
         private AdvisorService advisor_service = inject<AdvisorService>();
         private UserService users = inject<UserService>();
 
+        protected override Type private_type {
+            get { return typeof(AdvisorChatActionPrivate); }
+        }
+
         public override async DirectiveBuilder handle() throws GLib.Error {
             var guard = yield SpryAuth.require_login(directives(), request, users);
             if (guard != null) {
                 return (!)guard;
             }
 
-            var me = SpryAuth.current(request);
-            if (me == null) {
+            var ctx = request_private;
+            if (ctx == null) {
                 return directives().navigate("/chats");
             }
-            int64 chat_id = Forms.integer(request, "chat_id", 0);
-            if (chat_id <= 0) {
+            var me = SpryAuth.current(request);
+            if (me == null) {
                 return directives().navigate("/chats");
             }
-            var chat = yield chat_service.get_chat(chat_id, ((!)me).user_id);
+
+            var chat = yield chat_service.get_chat(((!)ctx).chat_id,
+                ((!)me).user_id);
             if (chat == null) {
                 // Deleted mid-turn (or never the viewer's): nothing to
                 // refresh — the page's own navigation takes over.

src/entrypoints/ChatDetailEntrypoint.vala

@@ -132,7 +132,12 @@ namespace ClankerSaddle {
                         AdvisorChatActionPrivate>(sealed);
                     pub.delete = ((!)actions).author_private<DeleteAdvisorChatAction,
                         AdvisorChatActionPrivate>(sealed);
-                    pub.refresh = ((!)actions).author<RefreshAdvisorChatAction>("GET");
+                    // GET-sealed (author_private): statum.act urlencodes
+                    // GET parameters as a QUERY STRING no form reader
+                    // ever sees — an unsealed id parses 0 and the
+                    // refresh would navigate away on every bump.
+                    pub.refresh = ((!)actions).author_private<RefreshAdvisorChatAction,
+                        AdvisorChatActionPrivate>(sealed, "GET");
                 }
 
                 var messages = new Vector<ChatMessageView>();

src/pages/app.css

@@ -1339,14 +1339,35 @@ dialog#quote-comment-dialog blockquote {
     border-color: var(--accent);
     margin-bottom: var(--grid-3);
 }
-.advisor-hero form { margin: 0; }
-.advisor-hero textarea {
-    font-size: 1.05rem;
-    min-height: 3.4rem;
-}
-.advisor-hero .form-row { align-items: flex-end; }
 .advisor-hero .notice { margin-bottom: 0; }
 
+/* The hero's own block form — deliberately NOT form.inline (inline-
+   flex squeezes the textarea) nor .form-row (its 34rem column cap is
+   for edit forms, not a hero): a full-width label/textarea/actions
+   stack with a roomy, larger-type composer. It reads as the page's
+   primary control while keeping the app's card language. */
+.advisor-hero-form {
+    display: flex;
+    flex-direction: column;
+    gap: var(--grid-1);
+    margin: 0 0 var(--grid-1);
+}
+.advisor-hero-form label {
+    font-weight: 600;
+    font-size: 0.95rem;
+}
+.advisor-hero-form textarea {
+    width: 100%;
+    min-height: 4.5rem;
+    resize: vertical;
+    font-size: 1.1rem;
+    line-height: 1.45;
+}
+.advisor-hero-form .advisor-hero-actions {
+    display: flex;
+    justify-content: flex-end;
+}
+
 /* The chat page: a column of transcript lanes under the head. */
 .chat-page { display: flex; flex-direction: column; gap: var(--grid-2); }
 .chat-head h2 { margin-bottom: 0.25rem; }

src/pages/chat_detail.html

@@ -23,7 +23,10 @@
             statum.addStateListener('pulse', function () {
                 var chat = chatState();
                 if (chat.refresh) {
-                    statum.act(chat.refresh, { chat_id: chat.id }).catch(function () { });
+                    // The chat id is sealed in the action (GET): no
+                    // parameters travel the query string, so nothing
+                    // can be lost in transport and misparsed to 0.
+                    statum.act(chat.refresh, {}).catch(function () { });
                 }
             });
 

src/pages/home.html

@@ -251,13 +251,19 @@
              the advisor reads fleet state, asks questions and
              suggests tasks/goals, but never runs commands. -->
         <section stm-if="dashboard != null" class="advisor-hero card">
-            <form class="inline advisor-hero-form"
+            <!-- Its own block layout (.advisor-hero-form): NOT
+                 form.inline + .form-row, whose inline-flex and 34rem
+                 column cap squeeze a multi-line textarea into a
+                 cramped sliver. The composer spans the card's full
+                 width with a roomy textarea; the button sits below it,
+                 right-aligned. -->
+            <form class="advisor-hero-form"
                   onsubmit="window.startAdvisorChat(); return false;">
-                <div class="form-row">
-                    <label for="advisor-composer">Ask the advisor</label>
-                    <textarea id="advisor-composer" rows="2"
-                              stm-attribute.disabled="!dashboard.advisor_configured"
-                              placeholder="What should the fleet work on next?"></textarea>
+                <label for="advisor-composer">Ask the advisor</label>
+                <textarea id="advisor-composer" rows="3"
+                          stm-attribute.disabled="!dashboard.advisor_configured"
+                          placeholder="What should the fleet work on next?"></textarea>
+                <div class="advisor-hero-actions">
                     <button type="submit" class="primary"
                             stm-attribute.disabled="!dashboard.advisor_configured">Start chat</button>
                 </div>

test/bundles/advisor_fix.py

@@ -0,0 +1,430 @@
+"""The advisor review-fix bundle (goal 27, rejected-review follow-up):
+one regression guard per defect the completion review found, each
+driven end-to-end through the app and the scripted LLM endpoint.
+
+- the tool schema (bug 1): the REQUEST's tools array carries no prose
+  in a `type` field ("array of string" 400s strict providers),
+  ask_question's options property is the items-object array shape,
+  suggest_task's mode is an enum, and every schema has its `required`
+  array
+- the 400 body (bug 2): a provider 400 with a JSON body lands in the
+  chat's error row verbatim enough to read the provider's reason
+- the redirect loop (bug 3): the chat page's refresh action is SEALED
+  (no form/chat_id round-trip) — a bare GET with no parameters answers
+  a slot SET for the owner and navigates ONLY for a foreign or deleted
+  chat
+- the reply variants (bug 4): provider bodies shaped like real ones
+  (tool_calls absent, an empty message object, an error object over
+  HTTP 200, content null) parse without a json-glib CRITICAL and
+  without wedging the turn
+- the hero markup (bug 5): the dashboard's composer is its own block
+  form (not form.inline/.form-row) with a full-width textarea
+"""
+from harness import *  # noqa: F401,F403 — the shared wire helpers
+import harness  # the module itself (global re-pointing on restarts)
+from bundles import advisor as advisor_module
+from bundles.advisor import ScriptedLlm, advisor_api, chat_rows
+
+
+def db_rows(sql, params=()):
+    conn = db()
+    try:
+        return conn.execute(sql, params).fetchall()
+    finally:
+        conn.close()
+
+
+def wait_status(chat_id, status, timeout=30):
+    def read():
+        rows = db_rows("select status from advisor_chats where id = ?",
+                       (chat_id,))
+        return len(rows) == 1 and rows[0][0] == status and status or False
+    return wait_for(read, timeout=timeout,
+                    what="chat %d -> %s" % (chat_id, status))
+
+
+def restart_with_llm(ctx, llm):
+    """Points the app at `llm` and restarts it (env re-read)."""
+    os.environ["ADVISOR_LLM_BASE_URL"] = "http://127.0.0.1:%d" % llm.port
+    os.environ["ADVISOR_LLM_MODEL"] = "scripted"
+    ctx.app.terminate()
+    try:
+        ctx.app.wait(timeout=10)
+    except subprocess.TimeoutExpired:
+        ctx.app.kill()
+    ctx.app = start_app()
+    ctx.auth = login("admin", "adminpass123")
+    return ctx.auth
+
+
+def schema_types(value, seen):
+    """Collects every JSON-Schema `type` leaf in a parsed structure."""
+    if isinstance(value, dict):
+        for key, item in value.items():
+            if key == "type" and isinstance(item, str):
+                seen.add(item)
+            schema_types(item, seen)
+    elif isinstance(value, list):
+        for item in value:
+            schema_types(item, seen)
+
+
+def run(ctx):
+    auth = ctx.auth
+    # The shared helpers imported from bundles.advisor (advisor_api
+    # reads WEB) captured THIS module's star-import at load; in the
+    # sequential full run those globals are the harness's re-pointed
+    # ones while bundles.advisor's own copy stayed at ITS shard's
+    # values — re-point them exactly as run_bundle does for us.
+    for runtime_name in ("WORK", "DB_PATH", "APP_PORT", "DAEMON_PORT",
+                         "WEB"):
+        setattr(advisor_module, runtime_name,
+                getattr(harness, runtime_name))
+
+    print("== advisor_fix: the tool schema on the wire ==")
+    llm = ScriptedLlm([
+        {"tools": [{"id": "fx-1", "name": "ask_question",
+                    "arguments": json.dumps({
+                        "question": "Schema ok?",
+                        "options": ["yes", "no"]})}]},
+        {"content": "Schema verified."},
+    ])
+    llm.serve()
+    auth = restart_with_llm(ctx, llm)
+    status, body = advisor_api(auth, "POST", "/api/advisor/chat",
+                               {"title": "schema"})
+    schema_chat = int(body["chat_id"])
+    advisor_api(auth, "POST", "/api/advisor/turn",
+                {"chat_id": schema_chat, "message": "check the schema"})
+    check(wait_for(lambda: llm.requests or False, timeout=15,
+                   what="first LLM call") is not None,
+          "the turn reached the LLM")
+
+    tools = llm.requests[0].get("tools") or []
+    names = {t["function"]["name"]: t["function"] for t in tools}
+    check(set(names) >= {"list_projects", "ask_question", "suggest_task",
+                         "suggest_goal"},
+          "the request declares the advisor tool set (%s)" % sorted(names))
+
+    # Bug 1's regression: NO prose may ride a `type` field anywhere in
+    # the tools array — strict providers validate it and 400 the whole
+    # request before answering anything.
+    types_seen = set()
+    schema_types(tools, types_seen)
+    check("array of string" not in types_seen
+          and all(t in ("string", "integer", "array", "object", "boolean",
+                        "number", "function")
+                  for t in types_seen),
+          "every schema type leaf is a real JSON-Schema type (%s)"
+          % sorted(types_seen))
+
+    question_params = names["ask_question"]["parameters"]
+    options_property = question_params["properties"]["options"]
+    check(options_property.get("type") == "array"
+          and options_property.get("items", {}).get("type") == "string",
+          "ask_question's options property is the items-object array shape"
+          " (%s)" % options_property)
+    check(question_params.get("required") == ["question"],
+          "ask_question declares its required array (%s)"
+          % question_params.get("required"))
+
+    task_params = names["suggest_task"]["parameters"]
+    mode_property = task_params["properties"]["mode"]
+    check(mode_property.get("enum") == ["implement", "investigate",
+                                        "review", "test"],
+          "suggest_task's mode is the work-mode enum (%s)"
+          % mode_property.get("enum"))
+    check(task_params.get("required") == ["title", "mandate"],
+          "suggest_task declares title+mandate required (%s)"
+          % task_params.get("required"))
+
+    goal_params = names["suggest_goal"]["parameters"]
+    check(goal_params.get("required") == ["title", "brief", "project_id"],
+          "suggest_goal declares title+brief+project_id required (%s)"
+          % goal_params.get("required"))
+    check("required" in names["list_projects"]["parameters"],
+          "even the parameter-less read tools carry a required array")
+    check(names["get_project"]["parameters"]["properties"]["project_id"]
+          .get("type") == "integer",
+          "integer params keep their integer type")
+
+    # The question parks; answer it so the chat settles.
+    check(wait_status(schema_chat, "awaiting_answer"),
+          "the scripted question parks the chat")
+    advisor_api(auth, "POST", "/api/advisor/answer",
+                {"chat_id": schema_chat, "answer": "yes"})
+    check(wait_status(schema_chat, "idle"), "the schema chat settles idle")
+
+    print("== advisor_fix: the provider 400 body reaches the error row ==")
+    # fail_next answers 500; the 400 scene needs a body the harness can
+    # read back, so serve a raw 400 with a JSON error envelope.
+    llm400 = ScriptedLlm([{"content": "never"}])
+    llm400.serve()
+
+    class Handler400(BaseHTTPRequestHandler):
+        protocol_version = "HTTP/1.1"
+
+        def log_message(self, fmt, *args):
+            pass
+
+        def do_POST(self):
+            length = int(self.headers.get("Content-Length") or 0)
+            self.rfile.read(length)
+            body = json.dumps({
+                "error": {"message": "Invalid schema for tool"
+                          " 'ask_question': options.type 'array of string'"
+                          " is not a valid type",
+                          "type": "invalid_request_error"}}).encode()
+            self.send_response(400)
+            self.send_header("Content-Type", "application/json")
+            self.send_header("Content-Length", str(len(body)))
+            self.end_headers()
+            self.wfile.write(body)
+
+    server400 = ThreadingHTTPServer(("127.0.0.1", 0), Handler400)
+    threading.Thread(target=server400.serve_forever, daemon=True).start()
+    auth = restart_with_llm(ctx, llm400)
+    # Point the env at the raw 400 server AFTER the restart read the
+    # (valid) llm400 for the configured check; the restart below binds
+    # the app to the 400 endpoint.
+    os.environ["ADVISOR_LLM_BASE_URL"] = \
+        "http://127.0.0.1:%d" % server400.server_address[1]
+    ctx.app.terminate()
+    try:
+        ctx.app.wait(timeout=10)
+    except subprocess.TimeoutExpired:
+        ctx.app.kill()
+    ctx.app = start_app()
+    ctx.auth = auth = login("admin", "adminpass123")
+
+    status, body = advisor_api(auth, "POST", "/api/advisor/chat",
+                               {"title": "errors"})
+    err_chat = int(body["chat_id"])
+    advisor_api(auth, "POST", "/api/advisor/turn",
+                {"chat_id": err_chat, "message": "provoke the 400"})
+    check(wait_status(err_chat, "idle"),
+          "the 400 turn returns the chat to idle")
+    error_rows = wait_for(lambda: [r for r in chat_rows(err_chat,
+                                                        "kind, content")
+                                   if r[0] == "error"] or False,
+                          timeout=15, what="400 error row")
+    check(error_rows is not None and len(error_rows) == 1,
+          "the 400 lands as exactly one error row")
+    content = error_rows[0][1] if error_rows else ""
+    check("HTTP 400" in content,
+          "the error row names the status (%s)" % content[:120])
+    check("is not a valid type" in content and "Invalid schema" in content,
+          "the error row CONTAINS the provider's own message text (%s)"
+          % content[:200])
+    check(len(content) <= 600,
+          "the error row stays clamped (%d chars)" % len(content))
+    server400.shutdown()
+    server400.server_close()
+    llm400.kill()
+
+    print("== advisor_fix: the refresh action is sealed (no redirect) ==")
+    # A quiet scripted LLM: the scene only needs a chat that exists.
+    llmr = ScriptedLlm([{"content": "quiet"}])
+    llmr.serve()
+    auth = restart_with_llm(ctx, llmr)
+    status, body = advisor_api(auth, "POST", "/api/advisor/chat",
+                               {"title": "refresh"})
+    refresh_chat = int(body["chat_id"])
+    advisor_api(auth, "POST", "/api/advisor/turn",
+                {"chat_id": refresh_chat, "message": "say something"})
+    check(wait_status(refresh_chat, "idle"), "the refresh chat settles")
+
+    page = slot_public(auth, "/chats/%d" % refresh_chat, "chat")
+    check("refresh" in page, "the chat slot carries the refresh action")
+    refresh = page["refresh"]
+    check(refresh.get("method") == "GET" and refresh.get("private"),
+          "the refresh action is GET and SEALED (private blob present)")
+    # The pulse listener calls statum.act(chat.refresh, {}) — a bare
+    # GET with NO parameters. Before the fix, Forms.integer read 0 and
+    # every bump navigated to /chats (the redirect loop).
+    result = act(auth, refresh, {})
+    sets = [d for d in result if d.get("type") == "set"]
+    navigations = [d for d in result if d.get("type") == "navigate"]
+    check(len(sets) == 1 and not navigations,
+          "the bare GET refresh answers a slot SET, no navigate (%s)"
+          % [d.get("type") for d in result])
+    set_chat = json.loads(sets[0]["snapshot_frame"]["content"])
+    check(set_chat["slot"]["type"] == "chat"
+          and set_chat["public"]["id"] == refresh_chat,
+          "the refreshed slot is the owner's own chat")
+
+    # The chat page's script must no longer pass a chat_id (the param
+    # that was silently lost in the query string).
+    detail_html = http_get_page(auth, "/chats/%d" % refresh_chat)
+    check("statum.act(chat.refresh, {})" in detail_html,
+          "the pulse listener invokes the refresh with no parameters")
+
+    # A foreign viewer's sealed refresh: they cannot author one (the
+    # not-found slot carries no actions), and a deleted chat navigates.
+    second = "fix-" + str(int(time.time()))
+    users_state = slot_public(auth, "/users", "users")
+    act(auth, users_state["create"], {
+        "username": second, "email": second + "@example.com",
+        "forename": "Fix", "surname": "Bundle",
+        "date_of_birth": "1990-01-01",
+        "password": "fixpass123", "confirm_password": "fixpass123"})
+    other = login(second, "fixpass123")
+    other_page = slot_public(other, "/chats/%d" % refresh_chat, "chat")
+    check(other_page["id"] == 0 and "refresh" not in other_page,
+          "a foreign viewer gets the not-found slot with no refresh")
+    status, _ = advisor_api(auth, "POST", "/api/advisor/chat/delete",
+                            {"chat_id": refresh_chat})
+    doomed = slot_public(auth, "/chats", "chats")
+    # Author a refresh from a LIVE chat, delete the chat, then fire the
+    # action: only the genuine not-found path may navigate.
+    status, body = advisor_api(auth, "POST", "/api/advisor/chat",
+                               {"title": "doomed"})
+    doomed_chat = int(body["chat_id"])
+    advisor_api(auth, "POST", "/api/advisor/turn",
+                {"chat_id": doomed_chat, "message": "populate"})
+    check(wait_status(doomed_chat, "idle"), "the doomed chat settles")
+    doomed_page = slot_public(auth, "/chats/%d" % doomed_chat, "chat")
+    advisor_api(auth, "POST", "/api/advisor/chat/delete",
+                {"chat_id": doomed_chat})
+    result = act(auth, doomed_page["refresh"], {})
+    check(any(d.get("type") == "navigate" and d.get("uri") == "/chats"
+              for d in result),
+          "a refresh for a genuinely deleted chat navigates to /chats")
+
+    print("== advisor_fix: reply variants parse without a CRITICAL ==")
+    # Variants real providers send: no tool_calls member at all, an
+    # empty message object, an error object over HTTP 200, and content
+    # null. Each must parse (or throw a catchable Error) — never a
+    # json-glib CRITICAL and never a wedged turn.
+    variants = [
+        ("no tool_calls member",
+         {"choices": [{"index": 0, "finish_reason": "stop",
+                       "message": {"role": "assistant",
+                                   "content": "plain reply"}}],
+          "model": "scripted"}),
+        ("empty message object",
+         {"choices": [{"index": 0, "finish_reason": "stop",
+                       "message": {}}], "model": "scripted"}),
+        ("null content",
+         {"choices": [{"index": 0, "finish_reason": "stop",
+                       "message": {"role": "assistant", "content": None}}],
+          "model": "scripted"}),
+        ("null message object",
+         {"choices": [{"index": 0, "finish_reason": "stop",
+                       "message": None}], "model": "scripted"}),
+        ("first choice not an object",
+         {"choices": [None], "model": "scripted"}),
+        ("error object over HTTP 200",
+         {"error": {"message": "quota exceeded", "type": "insufficient_quota"},
+          "model": "scripted"}),
+    ]
+    variant_logs = []
+    for label, envelope in variants:
+        llmv = ScriptedLlm([{"content": "unused"}])
+        llmv.serve()
+        # Swap the served envelope by pointing the script at a raw
+        # handler: simplest is a dedicated one-shot server.
+        class HandlerVariant(BaseHTTPRequestHandler):
+            protocol_version = "HTTP/1.1"
+
+            def log_message(self, fmt, *args):
+                pass
+
+            def do_POST(self):
+                length = int(self.headers.get("Content-Length") or 0)
+                self.rfile.read(length)
+                served = json.dumps(envelope).encode()
+                self.send_response(200)
+                self.send_header("Content-Type", "application/json")
+                self.send_header("Content-Length", str(len(served)))
+                self.end_headers()
+                self.wfile.write(served)
+
+        serverv = ThreadingHTTPServer(("127.0.0.1", 0), HandlerVariant)
+        threading.Thread(target=serverv.serve_forever, daemon=True).start()
+        os.environ["ADVISOR_LLM_BASE_URL"] = \
+            "http://127.0.0.1:%d" % serverv.server_address[1]
+        ctx.app.terminate()
+        try:
+            ctx.app.wait(timeout=10)
+        except subprocess.TimeoutExpired:
+            ctx.app.kill()
+        ctx.app = start_app()
+        ctx.auth = auth = login("admin", "adminpass123")
+        status, body = advisor_api(auth, "POST", "/api/advisor/chat",
+                                   {"title": label})
+        variant_chat = int(body["chat_id"])
+        advisor_api(auth, "POST", "/api/advisor/turn",
+                    {"chat_id": variant_chat, "message": "parse this"})
+        # The turn's TERMINAL ROW is the deterministic signal (a fresh
+        # chat already reads idle) — an assistant reply, an error row,
+        # or the no-choices throw landing as one.
+        terminal = wait_for(lambda: [r for r in chat_rows(variant_chat,
+                                                          "kind")
+                                     if r[0] in ("assistant", "error")]
+                            or False, timeout=20,
+                            what="%s terminal row" % label)
+        check(terminal is not None,
+              "the %s variant lands a terminal row (%s)"
+              % (label, [r[0] for r in chat_rows(variant_chat, "kind")]))
+        check(wait_status(variant_chat, "idle", timeout=20),
+              "the %s variant leaves the chat idle (no wedge)" % label)
+        try:
+            with open(os.path.join(WORK, "app.log"), "r",
+                      errors="replace") as handle:
+                variant_logs.append(handle.read())
+        except OSError:
+            pass
+        serverv.shutdown()
+        serverv.server_close()
+        llmv.kill()
+
+    # A CRITICAL in ANY variant's app log would name the json-glib
+    # accessor (json_object_get_array_member, json_object_has_member,
+    # …) — none may appear for any of the driven shapes.
+    log_text = "\n".join(variant_logs)
+    critical_lines = [line for line in log_text.splitlines()
+                      if "Json-CRITICAL" in line]
+    check(not critical_lines,
+          "no json-glib CRITICAL lines across the variants (%d seen)%s"
+          % (len(critical_lines),
+             " e.g. %s" % critical_lines[0][:120] if critical_lines else ""))
+
+    print("== advisor_fix: the hero composer markup ==")
+    home_html = http_get_page(auth, "/")
+    check('class="advisor-hero card"' in home_html,
+          "the hero keeps its card")
+    check('class="advisor-hero-form"' in home_html,
+          "the hero form carries its own layout class")
+    check('class="inline advisor-hero-form"' not in home_html,
+          "the hero form is NOT form.inline")
+    # The FORM tag onward (the explanatory comment above it names the
+    # old classes in prose — it is not markup).
+    hero_block = '<form class="advisor-hero-form"' + home_html.split(
+        '<form class="advisor-hero-form"')[1].split("</form>")[0]
+    check("form-row" not in hero_block,
+          "the hero form no longer uses the 34rem .form-row column")
+    check('id="advisor-composer"' in hero_block
+          and "<textarea" in hero_block,
+          "the hero textarea rides the form")
+    css = ""
+    for candidate in (os.path.join(os.path.dirname(os.path.abspath(
+            harness.__file__)), "..", "src", "pages", "app.css"),):
+        try:
+            with open(candidate, "r", errors="replace") as handle:
+                css += handle.read()
+        except OSError:
+            pass
+    check(".advisor-hero-form textarea" in css
+          and "min-height: 4.5rem" in css and "width: 100%" in css,
+          "the CSS gives the hero textarea its full-width roomy layout")
+    check(".advisor-hero .form-row" not in css,
+          "the old squeezed .form-row hero rule is gone")
+
+    llm.kill()
+    llmr.kill()
+
+    # Restore the daemon-facing LLM env the other checks may rely on
+    # (the advisor env is test-local).
+    os.environ.pop("ADVISOR_LLM_BASE_URL", None)
+    os.environ.pop("ADVISOR_LLM_MODEL", None)

test/harness.py

@@ -1325,7 +1325,7 @@ BUNDLES_ORDER = ("pages", "dispatch", "subtasks", "lineage",
                  "replay", "goalui", "forcewake", "users", "stacks", "notes",
                  "resurrect", "origins", "parity", "understanding",
                  "discoveries", "knowledge_cycle", "advisor", "chatui",
-                 "advisor_arc")
+                 "advisor_arc", "advisor_fix")
 
 
 def run_bundle(name):