diff --git a/rtut/__main__.py b/rtut/__main__.py index 4c31c72..24b1c96 100644 --- a/rtut/__main__.py +++ b/rtut/__main__.py @@ -45,6 +45,8 @@ def _run(pk, a) -> int: R.retry(r, a.step); return 0 if a.action == "skip": R.skip(r, a.step, a.note); return 0 + if a.action == "feedback": + R.add_feedback(r, a.rating or "", a.note); print("recorded"); return 0 old, new = R.exercise_update(r) import difflib print("".join(difflib.unified_diff(old.splitlines(True), new.splitlines(True), "pack.yaml", "pack.yaml"))) @@ -63,7 +65,7 @@ def main(argv: list[str] | None = None) -> int: sp = sub.add_parser(name) sp.add_argument("root", nargs="?", default=".") sp = sub.add_parser("run", help="record a run of a native pack") - sp.add_argument("action", choices=["new", "list", "status", "cmd", "start", "verify", "done", "fail", "retry", "skip", "finish"]) + sp.add_argument("action", choices=["new", "list", "status", "cmd", "start", "verify", "done", "fail", "retry", "skip", "finish", "feedback"]) sp.add_argument("manifest") sp.add_argument("run_id", nargs="?") sp.add_argument("step", nargs="?") @@ -71,12 +73,15 @@ def main(argv: list[str] | None = None) -> int: sp.add_argument("--param", "-p", action="append", default=[], help="name=value") sp.add_argument("--evidence", "-e", action="append", default=[], help="name=value (non-secret)") sp.add_argument("--note", default="") + sp.add_argument("--rating", choices=["good", "neutral", "poor"], help="feedback: how the runbook felt") sp.add_argument("--yes", action="store_true", help="confirm running a verify command") sp.add_argument("--pass", dest="passed", action="store_true", help="manual check: operator confirms it passed") sp.add_argument("--apply", action="store_true", help="finish: write the exercise-status change") sp = sub.add_parser("ui", help="serve the local UI on 127.0.0.1") sp.add_argument("roots", nargs="*", default=["."]) sp.add_argument("--port", type=int, default=8877) + sp = sub.add_parser("feedback", help="list the latest feedback of every run of a pack") + sp.add_argument("manifest") sp = sub.add_parser("launch"); sp.add_argument("manifest") sp.add_argument("--timeout", type=float, default=30.0) sp = sub.add_parser("show"); sp.add_argument("manifest") @@ -113,6 +118,11 @@ def main(argv: list[str] | None = None) -> int: if not pk.ok: print("\n".join(pk.errors), file=sys.stderr) return 1 + if a.cmd == "feedback": + from . import run as R + for f in R.all_feedback(pk): + print(f"{f['at']} {R.RATINGS[f['rating']]} {f['operator']} {f['run_id']} {f['comment']}") + return 0 if a.cmd == "run": return _run(pk, a) if a.cmd == "launch": diff --git a/rtut/run.py b/rtut/run.py index 29a822d..b0cb19c 100644 --- a/rtut/run.py +++ b/rtut/run.py @@ -232,3 +232,33 @@ def exercise_update(run: Run, today: date | None = None) -> tuple[str, str]: new = STATUS_LINE.sub(f"exercise_status: exercised {today.isoformat()} by {run.data['operator']}", old, count=1) return old, new + + +RATINGS = {"good": ":)", "neutral": ":|", "poor": ":("} + + +def add_feedback(run: Run, rating: str, comment: str = "") -> dict: + """Record how the runbook felt. Allowed at any time; the latest entry is the current one.""" + if rating not in RATINGS: + raise RunError(f"rating must be one of {', '.join(RATINGS)}") + entry = {"rating": rating, "comment": _clean(comment.strip(), "comment"), + "at": datetime.now(timezone.utc).isoformat(timespec="microseconds")} # sortable even within one second + run.data.setdefault("feedback", []).append(entry) + run.save() + return entry + + +def latest_feedback(run: Run) -> dict | None: + fb = run.data.get("feedback") or [] + return fb[-1] if fb else None + + +def all_feedback(pack: Pack) -> list[dict]: + """Latest feedback of every run of this pack, newest first.""" + out = [] + for rid in list_runs(pack): + r = load(pack, rid) + fb = latest_feedback(r) + if fb: + out.append({"run_id": rid, "operator": r.data["operator"], **fb}) + return sorted(out, key=lambda x: x["at"], reverse=True) diff --git a/rtut/ui.py b/rtut/ui.py index 2685905..408abf2 100644 --- a/rtut/ui.py +++ b/rtut/ui.py @@ -34,7 +34,7 @@ def page(title: str, body: str) -> bytes: "const f=b.closest('form'),n=f.querySelector('[name=note]'),w=f.querySelector('[data-warn]');" "if(n&&!n.value.trim()&&!b.dataset.warned){ev.preventDefault();b.dataset.warned='1';w.hidden=false;n.focus();}}));" # keep the operator on the current step after every action - "const c=document.getElementById('current');if(c)c.scrollIntoView({block:'center'});" + "const c=document.getElementById(location.hash==='#feedback'?'feedback':'current');if(c)c.scrollIntoView({block:'center'});" "").encode() @@ -42,6 +42,9 @@ def e(s) -> str: return html.escape(str(s)) +SMILEYS = (("good", "🙂", "Went well"), ("neutral", "😐", "So-so"), ("poor", "🙁", "Poor")) + + class App: def __init__(self, roots: list[Path]): self.token = secrets.token_urlsafe(16) @@ -83,6 +86,23 @@ class App: "

" "") + def feedback_form(self, i: int, rid: str, r) -> str: + fb = R.latest_feedback(r) + thanks = "" + if fb: + face = next(f for k, f, _ in SMILEYS if k == fb["rating"]) + thanks = (f"

Thank you. Recorded: {face} {e(fb['comment'] or '(no comment)')} " + "You can change it below.

") + btns = " ".join(f"" + for k, face, t in SMILEYS) + return (f"

How was this runbook?

{thanks}" + f"
{self.hidden()}" + "

Practicable? Anything that felt strange or unclear, or an idea to make it better. " + "Never include secrets.

" + f"

{btns}

") + def run_page(self, i: int, rid: str, msg: str = "") -> bytes: p = self.packs[i]; r = R.load(p, rid); d = p.data out = [f"

{e(d['title'])}

{e(d['exercise_status'])}

" @@ -157,6 +177,8 @@ class App: if r.complete: out.append("

All steps done. To record this run as exercising the pack, review and apply: " f"python -m rtut run finish {e(p.path)} {e(rid)} --apply

") + if r.complete: + out.append(self.feedback_form(i, rid, r)) out.append("

Threat checks

") return page(d["title"], "".join(out)) @@ -205,6 +227,10 @@ def make_handler(app: App, port_holder: list): params = {k[2:]: v for k, v in form.items() if k.startswith("p_")} r = R.create(p, form.get("operator", ""), params) return self._redirect(f"/run/{i}/{r.data['run_id']}#current") + if len(parts) == 4 and parts[0] == "run" and parts[3] == "feedback": + i, rid = int(parts[1]), parts[2] + R.add_feedback(R.load(app.packs[i], rid), form.get("rating", ""), form.get("comment", "")) + return self._redirect(f"/run/{i}/{rid}#feedback") i, rid, sid, action = int(parts[1]), parts[2], parts[3], parts[4] p = app.packs[i]; r = R.load(p, rid) if action == "start": R.start(r, sid) diff --git a/tests/test_feedback.py b/tests/test_feedback.py new file mode 100644 index 0000000..9c288ec --- /dev/null +++ b/tests/test_feedback.py @@ -0,0 +1,41 @@ +import shutil +from pathlib import Path + +import pytest + +from rtut import pack as P, run as R + +ROOT = Path(__file__).resolve().parent.parent + + +@pytest.fixture +def pk(tmp_path): + d = tmp_path / "runbooks" / "hello"; d.mkdir(parents=True) + shutil.copy(ROOT / "runbooks/example-hello/pack.yaml", d / "pack.yaml") + return P.load(d / "pack.yaml") + + +def test_feedback_latest_wins_and_history_kept(pk): + r = R.create(pk, "bernd") + R.add_feedback(r, "poor", "confusing start") + R.add_feedback(r, "good", "fine after a second look") + r2 = R.load(pk, r.data["run_id"]) + assert len(r2.data["feedback"]) == 2 + assert R.latest_feedback(r2)["rating"] == "good" + + +def test_feedback_rejects_bad_rating_and_secrets(pk): + r = R.create(pk, "bernd") + with pytest.raises(R.RunError): + R.add_feedback(r, "great") + with pytest.raises(R.RunError): + R.add_feedback(r, "good", "-----BEGIN RSA PRIVATE KEY-----") + assert "feedback" not in R.load(pk, r.data["run_id"]).data + + +def test_all_feedback_lists_newest_first(pk): + a = R.create(pk, "ann"); b = R.create(pk, "bob"); R.create(pk, "cy") # cy gives none + R.add_feedback(a, "good", "one") + R.add_feedback(b, "poor", "two") + got = R.all_feedback(pk) + assert [g["operator"] for g in got] == ["bob", "ann"] diff --git a/tests/test_ui.py b/tests/test_ui.py index d405e15..2a32975 100644 --- a/tests/test_ui.py +++ b/tests/test_ui.py @@ -178,3 +178,48 @@ def test_done_steps_fold_to_the_title_line_and_others_stay_open(srv): assert "
" in body # done: no 'open' attribute, so folded assert "1. Read the poem" in body.split("
")[1].split("")[0] # title line stays in the summary assert "Comment: fine" in body.split("
")[1].split("
")[0] # the rest is inside the fold + + +def _finish_showcase(srv, t, base): + import subprocess + from rtut import run as R + rid = base.rsplit("/", 1)[1] + r = R.load(srv.app.packs[int(base.split("/")[2])], rid) + for s in r.pack.data["steps"]: + sid = s["id"] + req(srv, "POST", f"{base}/{sid}/start", {"_t": t}) + cmd = R.command_for(r, sid) + if cmd: + subprocess.run(cmd, shell=True, check=True, capture_output=True) + if s["verify"]["expect"] == "manual": + req(srv, "POST", f"{base}/{sid}/verify", {"_t": t, "ok": "1"}) + else: + req(srv, "POST", f"{base}/{sid}/verify", {"_t": t, "yes": "1"}) + ev = {f"e_{n}": "1" for n in s.get("evidence", [])} + req(srv, "POST", f"{base}/{sid}/done", {"_t": t, **ev}) + + +def test_feedback_form_after_all_steps_done_and_recording(srv): + t, loc, _ = _showcase_run(srv) + base = loc.split("#")[0] + _, _, body = req(srv, "GET", base) + assert "How was this runbook?" not in body # not before the run is complete + _finish_showcase(srv, t, base) + _, _, body = req(srv, "GET", base) + assert "All steps done." in body and "How was this runbook?" in body + assert body.index("All steps done.") < body.index("How was this runbook?") + assert "🙂" in body and "😐" in body and "🙁" in body and "name=comment" in body + st, loc2, _ = req(srv, "POST", base + "/feedback", {"_t": t, "rating": "neutral", "comment": "step 3 was unclear"}) + assert st == 303 and loc2.endswith("#feedback") + _, _, body = req(srv, "GET", base) + assert "Thank you." in body and "step 3 was unclear" in body + + +def test_feedback_validation(srv): + t, loc, _ = _showcase_run(srv) + base = loc.split("#")[0] + _finish_showcase(srv, t, base) + assert req(srv, "POST", base + "/feedback", {"_t": t, "rating": "amazing"})[0] == 400 + st, _, body = req(srv, "POST", base + "/feedback", {"_t": t, "rating": "good", "comment": "hvs.ABCDEFGHIJKLMN"}) + assert st == 400 and "secret-looking" in body + assert req(srv, "POST", base + "/feedback", {"rating": "good"})[0] == 403 # CSRF token required diff --git a/workplans/RBT-WP-0004-local-runbook-ui.md b/workplans/RBT-WP-0004-local-runbook-ui.md index 9ad6699..9fefefc 100644 --- a/workplans/RBT-WP-0004-local-runbook-ui.md +++ b/workplans/RBT-WP-0004-local-runbook-ui.md @@ -163,6 +163,19 @@ Depends on RBT-WP-0007 T06-T07. Choose manual or autopilot per run (with timeout push-through controls). In manual mode, an override dialog lists unmet preconditions and the downstream risk before continuing; deviations show in the overview. +## Feedback collector + +```task +id: RBT-WP-0004-T12 +status: done +priority: medium +``` + +Done 2026-09-29: after "All steps done." the run view offers three smileys and a comment +(what felt strange or unclear, ideas to optimize). Stored in the run record (history kept, +latest wins, secrets refused); `rtut feedback ` lists the latest per run. +Follow-up: fold feedback into the overview and use it to prioritize pack refinements. + ## Acceptance Criteria - Bernd can complete the SSH tutorial pack in the UI without pasting outputs to an agent.