diff --git a/resources/cells/oversight.clj b/resources/cells/oversight.clj index 53ea55a8..d152d6f6 100644 --- a/resources/cells/oversight.clj +++ b/resources/cells/oversight.clj @@ -442,7 +442,8 @@ (mapv (fn [r] {:reason (clip (:reason r) (gates/threshold :oversight-note-chars)) - :targets (str/join ", " (map str (keys (:attempt r))))}) + :targets (or (:target r) + (str/join ", " (map str (keys (:attempt r)))))}) rs)})) ;; Gates that fire across runs and are never ;; met. The mirror of :candidates above — diff --git a/resources/prompts/roles/reviewer.md b/resources/prompts/roles/reviewer.md index 78517b7b..fba78dfd 100644 --- a/resources/prompts/roles/reviewer.md +++ b/resources/prompts/roles/reviewer.md @@ -6,18 +6,16 @@ not to write the feature yourself. Read what changed and judge it against the feature's intent: -- Run `git diff` (via `shell`) to see exactly what the implementors changed. +- The diff of what the implementors changed is in front of you; read the files + around it (`read_file`, `grep`) where the diff alone does not show enough. - Check the change actually does what the feature asked — not something adjacent that looks similar. An edit that touches the wrong thing is a defect, even if the code is correct. -- Run the tests (`jolt -M:test`) if the change is code. Unverified work does not - pass review. +- You do not run the tests: the verify step runs them after you. Judge whether + the change is covered — code with no test that would catch it being wrong is + a defect to name. Finish by shipping a verdict with the `done` tool. State **PASS** or **REVISE** on the first line, then your findings. PASS means the round is good enough for the critic to gate. REVISE means the implementors need another round — list the specific defects to fix, concretely, so they can act on them. - -The loop you run in is `reviewer` — it is yours to tune (`manifest show -reviewer` / `manifest save`). Tune your loop, not the implementors' or the -critic's. diff --git a/resources/prompts/roles/supervisor.md b/resources/prompts/roles/supervisor.md index a459892f..dd119b89 100644 --- a/resources/prompts/roles/supervisor.md +++ b/resources/prompts/roles/supervisor.md @@ -364,10 +364,12 @@ You have three. Use the smallest one that fits the evidence: could land under any adjustment you tried, not a task the defaults starved. **Land every harness fix through its tool.** When you author a new version - of a cell, manifest, or prompt, the natural way to write a large body is - `write_file` — and then you MUST finish with the matching save: + of a cell, manifest, or prompt, land it with the matching save: `cell save {name, file}` / `manifest save {name, file}` / - `prompt save {name, file}`, passing the path you just wrote. The save is + `prompt save {name, file}`. For a large body, write it with `shell` to a + scratch file under the run root outside the workflow directories and pass + that path as `file`; for a small change to a manifest, `manifest patch` + edits it in place. The save is what VALIDATES the change and records why you made it. This project's workflow lives in its `.samizdat/` directory (`cells/`, `manifests/`, `prompts/`, the policy tables): an edit written straight into one of those diff --git a/resources/roles.edn b/resources/roles.edn index 649ba010..048f564f 100644 --- a/resources/roles.edn +++ b/resources/roles.edn @@ -79,7 +79,7 @@ :tools #{"intervene" "read_file" "read_digest" "grep" "lsp" "skill" "doc" "complete" "manual" "introspect" "cells" "cell" "manifest" "prompt" "policy" "adopt" "fetch_turn" "fetch_artifact" "recall" "remember" "forget" "retire" "outcome" - "experiment" "message" "eval" "shell" "websearch" "done" "give_up"} + "experiment" "verdict" "message" "eval" "shell" "websearch" "done" "give_up"} :prompt "roles/supervisor" :sees #{:run-health :gate-health :failures}} diff --git a/src/samizdat/agent/tools.clj b/src/samizdat/agent/tools.clj index 8e20d858..ce3d3e20 100644 --- a/src/samizdat/agent/tools.clj +++ b/src/samizdat/agent/tools.clj @@ -45,6 +45,7 @@ [samizdat.agent.toolerr :as toolerr] [samizdat.lexicon :as lexicon] [samizdat.prompt :as prompt] + [samizdat.store.journal :as journal] [samizdat.agent.tools.base :as base] [samizdat.agent.tools.adopt] [samizdat.agent.tools.ask] @@ -179,6 +180,22 @@ (do (Thread/sleep (toolerr/backoff-ms attempt base-backoff-ms)) (recur (inc attempt)))))))) +(defn- journal-refusal! + "Write down a self-edit the harness validated and declined, whatever its + kind, so the supervisor's brief and later runs can read it back as ruled + out. The cell path journals its own, richer note inside the mutation + protocol and says so with :journaled?; every other kind was returned to the + branch and never written down (karamazov-na2k.1)." + [{:keys [conn run-id tool-name] :as ctx} r] + (when (and (:edit-rejected? r) (not (:journaled? r)) conn run-id) + (try + (journal/note! conn run-id :mutation-rolled-back + {:data {:target (str tool-name + (when-let [n (some-> (base/arg ctx :name) str not-empty)] + (str " " n))) + :reason (str (:result r))}}) + (catch Throwable e (log/warn "could not journal a refused edit:" (ex-message e)))))) + (defn run-tool "Dispatch one tool call and return a result the loop can always use. @@ -221,7 +238,8 @@ (str "`" tool-name "` " fault ". This is a harness fault, not yours — the call was fine.")) known)) - (redact-result r known)))))) + (do (journal-refusal! ctx r) + (redact-result r known))))))) ;; Re-exports: loop.clj and the tests reach the tool surface through this ;; namespace and keep working unchanged. diff --git a/src/samizdat/agent/tools/manifest.clj b/src/samizdat/agent/tools/manifest.clj index 04a8caff..4577eda5 100644 --- a/src/samizdat/agent/tools/manifest.clj +++ b/src/samizdat/agent/tools/manifest.clj @@ -195,6 +195,19 @@ (or (us/load-latest conn :manifest name) {:body body :version nil})))) +(defn- save! + "Store `text` as the newest version of manifest `name` and return the + version. Through the userspace seam, which in a project with files WRITES + the role's file — the file is what runs, the store is its history. Saving + to the store alone recorded an edit that never ran, and the next read + recorded the untouched file as a newer version on top of it + (karamazov-8nuu). A connection with no bound project (a test, a REPL) is + written to directly, as before." + [conn name text why] + (if (userspace/bound?) + (userspace/save! :manifest name text why) + (us/save! conn :manifest name text "project" why))) + (defn- read-ops "The `ops` argument as op maps ready for patch/apply-ops: a JSON list of objects (keys keywordized on the way in, values strings) or one EDN string. @@ -344,7 +357,7 @@ (base/rejected branch (str "`manifest save` refused: " complaint "\n\n" usage)) - (let [v (us/save! conn :manifest name edn-text "project" why)] + (let [v (save! conn name edn-text why)] (base/ok branch (str (saved-line name v) (dispatch-report edn-text) (cycle-report edn-text)) @@ -382,7 +395,7 @@ "\n\n" (ops-help))) (let [new (:new outcome) text (patch/render body old new) - v (us/save! conn :manifest name text "project" why)] + v (save! conn name text why)] (base/ok branch (str (saved-line name v) "\n\n" (format-diff (patch/diff-manifests old new) old new) diff --git a/src/samizdat/agent/tools/mutate.clj b/src/samizdat/agent/tools/mutate.clj index 1c64fbcd..0a04ba3d 100644 --- a/src/samizdat/agent/tools/mutate.clj +++ b/src/samizdat/agent/tools/mutate.clj @@ -214,11 +214,14 @@ ;; Rolled back: validate or the soak refused it, the registry ;; is restored, and nothing was stored. A correctable edit, ;; not a branch failure — see base/rejected. - (base/rejected branch - (str "Cell '" name "' was NOT saved; the loop is" - " unchanged and nothing entered this project's" - " history.\n\n" (:reason r) - "\n\nFix it and save again.")))))) + ;; :journaled? — propose-cell! already wrote the refusal, + ;; with the attempt, as :mutation-rolled-back. + (assoc (base/rejected branch + (str "Cell '" name "' was NOT saved; the loop is" + " unchanged and nothing entered this project's" + " history.\n\n" (:reason r) + "\n\nFix it and save again.")) + :journaled? true))))) "revert" (let [v (some-> (base/arg ctx :version) str str/trim not-empty parse-long) diff --git a/test/samizdat/oversight_test.clj b/test/samizdat/oversight_test.clj index 269f1702..09846e7b 100644 --- a/test/samizdat/oversight_test.clj +++ b/test/samizdat/oversight_test.clj @@ -807,6 +807,27 @@ (is (str/includes? (str prob) "soak did not terminate") "with the reason the protocol gave")))) +(deftest a-refused-edit-of-any-kind-reaches-the-supervisor + ;; karamazov-na2k.1. Only the cell path journaled its refusals; a manifest + ;; that did not compile, a policy body that broke its table, a prompt that + ;; would not render went back to the branch as base/rejected and were never + ;; written down, so the brief after a compaction — and every later run — + ;; had no idea they had been tried. + (require 'samizdat.agent.tools) + (let [conn (db/open! ":memory:") + rid (runs/start-run! conn {:problem "p"}) + r ((requiring-resolve 'samizdat.agent.tools/run-tool) + {:branch {:id "SUP"} :conn conn :run-id rid :tool-name "manifest" + :args {:action "save" :name "broken" :edn "{:cells {:x :no-such-cell}}" + :rationale "a bad save on purpose"}})] + (is (:edit-rejected? r) (:result r)) + (let [notes (journal/notes conn rid :mutation-rolled-back) + {:keys [prob]} (reasoning-over conn rid)] + (is (= 1 (count notes)) "the refusal is journaled once") + (is (= "manifest broken" (:target (first notes)))) + (is (str/includes? (str prob) "manifest broken") + "and the brief names what was refused")))) + (deftest a-refused-mutation-is-worth-a-model-call ;; A refusal means the supervisor tried to change the harness and the ;; protocol said no. That is precisely a moment where the next pass will diff --git a/test/samizdat/roles_test.clj b/test/samizdat/roles_test.clj index 6e0d7c48..a97b2200 100644 --- a/test/samizdat/roles_test.clj +++ b/test/samizdat/roles_test.clj @@ -11,7 +11,8 @@ cannot reach it\"). A supervisor once spent 108 of a run's 211 turns hunting a source tree it was never going to be allowed to open. Roles were differentiated by ADDITION; they are constructed now." - (:require [clojure.string :as str] + (:require [clojure.java.io :as io] + [clojure.string :as str] [clojure.test :refer [deftest testing is]] [samizdat.agent.roles :as roles] ;; the registry must be LOADED for the surface check to mean anything @@ -119,3 +120,27 @@ t s] (is (contains? registered t) (str r " may use `" t "`, which is not a registered tool"))))) + +(defn- tools-named-in + "Every registered tool a role prompt names in backticks, the way the prompts + tell a model to call one: `verdict {name, action, why}`, `write_file`." + [text] + (let [registered (set (roles/all-tool-names))] + (->> (re-seq #"`([a-z_]+)[ `({]" text) + (map second) + (filter registered) + set))) + +(deftest every-tool-a-role-prompt-names-is-on-its-surface + ;; karamazov-kcvb / na2k.2. roles/supervisor.md told the supervisor to settle + ;; experiments with `verdict` and author bodies with `write_file`; neither + ;; was on its surface, so it held the one experiment slot forever. Nothing + ;; compared what a role is told to call with what it may call. + (doseq [r (roles/names) + :let [surface (roles/surface r) + p (:prompt (roles/spec r)) + text (some-> p (#(io/resource (str "prompts/" % ".md"))) slurp)] + :when (and text (set? surface)) + t (tools-named-in text)] + (is (contains? surface t) + (str (name r) "'s prompt tells it to use `" t "`, which is not on its surface")))) diff --git a/test/samizdat/userspace_files_test.clj b/test/samizdat/userspace_files_test.clj index d1b16296..91e88e1d 100644 --- a/test/samizdat/userspace_files_test.clj +++ b/test/samizdat/userspace_files_test.clj @@ -303,3 +303,34 @@ (is (= "edited in the file" (:description (:definition ((requiring-resolve 'samizdat.workflow/load-loop!) _c "loop")))))))) + +(defn- manifest-tool [conn args] + (require 'samizdat.agent.tools.manifest) + ((requiring-resolve 'samizdat.agent.tools.base/run-tool) + {:branch {:id "B1"} :conn conn :tool-name "manifest" :args args})) + +(deftest the-manifest-tool-writes-the-projects-file + ;; karamazov-8nuu. `manifest save` and `patch` appended a store row and never + ;; wrote .samizdat/manifests/, so in a project with files the edit was + ;; recorded and never ran — and the next read recorded the untouched file + ;; as a newer "file" version, ending the history on a silent revert. + (with-project [root c] + (us/seed-project!) + (let [f (file root "manifests/loop.edn") + load-loop! (requiring-resolve 'samizdat.workflow/load-loop!)] + (testing "save" + (let [edited (pr-str (assoc (edn/read-string (slurp f)) :description "saved by the tool")) + r (manifest-tool c {:action "save" :name "loop" :edn edited + :rationale "a tool save"})] + (is (= :neutral (:category r)) (:result r)) + (is (= edited (slurp f)) "the file is what runs, so the save writes it") + (is (= "saved by the tool" + (:description (:definition (load-loop! c "loop"))))) + (is (= "project" (last (versions-of :manifest "loop"))) + "the history ends on the tool's version, not a re-read of the old file"))) + (testing "patch" + (let [r (manifest-tool c {:action "patch" :name "loop" :rationale "rename a node" + :ops [{:op "rename-cell" :from "journal" :to "record"}]})] + (is (= :neutral (:category r)) (:result r)) + (is (str/includes? (slurp f) ":record")) + (is (= "project" (last (versions-of :manifest "loop")))))))))