You are a constitutional council ranking individual git commits for ownership allocation. Compare these two commits. Decide which contributed more lasting value to the project. Judge substance, not spectacle: - Prefer correct, lasting design and real bugfixes over churn, formatting, renames, or generated noise. - Prefer clarity and necessity over sheer line count. A small precise change can beat a large diffuse one. - Do not favor a side merely because its patch is longer or noisier. - Weight what the change does for the project, not the contributor's name. Return ONLY a JSON object: {"winner": "A" or "B", "ratio": "N:M", "explanation": "..."} The explanation must cite concrete differences in the patches (1-3 sentences). Side A — contributor: tommy-mor Side A — commit message: [a45842df] better dev iteration script Side A — unified diff (full patch): diff --git a/.gitignore b/.gitignore index 9cd2eaece834481bee87639cd7cb6be77f33e302..b70cf5b97cd3556e061233d8b0aefd333568a991 100644 --- a/.gitignore +++ b/.gitignore @@ -3,6 +3,7 @@ target/ repomix-output.xml psalms_kjv.txt dev-data/ +fixture-data/ .worktrees/ .specstory/ .specstory/ diff --git a/bb.edn b/bb.edn index 5d1d642d75d03f70879d7673e7ad95360336a8b1..4680e82d4227057d2eea6041dc5647beb759131d 100644 --- a/bb.edn +++ b/bb.edn @@ -47,7 +47,7 @@ "RUST_LOG" "info"})})))} fixture - {:doc "Run local server + mock OAuth + seeded walkthrough data for manual browser demos" + {:doc "Run local server via cargo-watch + mock OAuth + seeded walkthrough data (persistent ./fixture-data/, prefers PORT 8080). Requires cargo-watch." :requires ([test.walkthrough-fixture :as walkthrough-fixture]) :task (walkthrough-fixture/run-fixture)} diff --git a/server/src/html/forum.rs b/server/src/html/forum.rs index e00e7110e2ad778d63b3d00b0cc4ffe20ba2e0d4..815fc6529062b103553644fc15e516334aec2b88 100644 --- a/server/src/html/forum.rs +++ b/server/src/html/forum.rs @@ -976,12 +976,6 @@ pub async fn room_page( html! { (strip) nav class="breadcrumb" { (bc_room(&nav, slug_display, None)) } - h2 { (slug_display) } - p class="muted" { (room_id) } - p class="muted room-links" { - "room garden · " - a href=(nav.garden_root_url()) { "~" } - } (room_members_section_markup(&reduced, &room_id, false)) h3 { "threads" } (render_thread_feed(Some(&nav), "room-thread-feed", &rows, now)) diff --git a/test/common.clj b/test/common.clj index 76a3e99fdb67c10bcb5c2f53d8bcd0917398af50..c6deba0d90995b0e3f9c4b8ad66a516ed19b870e 100644 --- a/test/common.clj +++ b/test/common.clj @@ -107,6 +107,16 @@ (.close ss) port)) +(defn pick-port-prefer + "Use `preferred` if it can be bound, otherwise an ephemeral port (same as `pick-port`)." + [preferred] + (try + (let [ss (java.net.ServerSocket. preferred)] + (.close ss) + preferred) + (catch java.io.IOException _ + (pick-port)))) + (defn wait-for-server "Poll /healthz until it returns 'ok', up to `timeout-ms`." [base-url timeout-ms] diff --git a/test/walkthrough_fixture.clj b/test/walkthrough_fixture.clj index 4b814822029c913e38151ca53848d1c64fb58373..a487ff1ef8548580577914fb33f321586859c9af 100644 --- a/test/walkthrough_fixture.clj +++ b/test/walkthrough_fixture.clj @@ -1,11 +1,18 @@ (ns test.walkthrough-fixture "Launch a local slug server with mock OAuth and seed browser-friendly demo data." (:require [babashka.fs :as fs] + [babashka.process :as p] [cheshire.core :as json] [clojure.string :as str] [test.common :as common] [test.oauth :as oauth])) +(def ^:private fixture-data-dir "fixture-data") +;; Prefer the same port as `bb dev` / `bb watch`; fall back if something else is listening. +(def ^:private preferred-slug-port 8080) +;; First `cargo watch` compile can exceed a release-binary startup; allow several minutes. +(def ^:private health-wait-ms 300000) + (defn- assert! [pred msg] (when-not pred (throw (ex-info msg {})))) @@ -85,49 +92,87 @@ :thread_url (str base-url "/r/" room-short "/" room-slug "/t/walkthrough-thread") :garden_url (str base-url "/r/" room-short "/" room-slug "/~/secret/item")}})) +(defn- rebase-fixture-summary [saved current-base-url current-google-url current-data-dir] + (let [inner (:summary saved) + room (:room inner) + rs (:short room) + lg (:slug room)] + (assoc saved + :base_url current-base-url + :mock_google_url current-google-url + :data_dir (str current-data-dir) + :summary (assoc inner + :room (assoc room + :url (str current-base-url "/r/" rs "/" lg) + :thread_url (str current-base-url "/r/" rs "/" lg "/t/walkthrough-thread") + :garden_url (str current-base-url "/r/" rs "/" lg "/~/secret/item")))))) + +(defn- fixture-log-present? [data-dir] + (let [p (fs/path data-dir "events.jsonl")] + (and (fs/exists? p) (pos? (fs/size p))))) + +(defn- load-or-seed-summary! + [base-url google-url data-dir summary-path] + (if (and (fixture-log-present? data-dir) (fs/exists? summary-path)) + (let [saved (json/parse-string (slurp summary-path) true) + rebased (rebase-fixture-summary saved base-url google-url data-dir)] + (spit summary-path (json/generate-string rebased {:pretty true})) + (println "") + (println "reusing fixture-data/ (delete the directory for a fresh seed)") + rebased) + (let [seeded (seed-demo! base-url) + s {:base_url base-url + :mock_google_url google-url + :data_dir (str data-dir) + :summary seeded}] + (spit summary-path (json/generate-string s {:pretty true})) + s))) + (defn run-fixture [& _args] - (let [build (common/run-cargo-build-release! ["slugsocial-server"]) - _ (assert! (zero? (:exit build)) "cargo build --release failed") - server-bin "target/release/slugsocial-server" - tmp-dir (str (fs/create-temp-dir {:prefix "slug-walkthrough-"})) - slug-port (common/pick-port) + (let [data-dir (str (fs/absolutize (fs/path (fs/cwd) fixture-data-dir))) + slug-port (common/pick-port-prefer preferred-slug-port) google-port (common/pick-port) base-url (str "http://127.0.0.1:" slug-port) google-url (str "http://127.0.0.1:" google-port) - stable-dir "/tmp/slug-walkthrough-fixture" - summary-path (str stable-dir "/summary.json") + summary-path (str (fs/path data-dir "summary.json")) !server (atom nil) !google (atom nil) - server-env (common/slug-server-env tmp-dir base-url google-url slug-port)] + server-env (merge (common/slug-server-env data-dir base-url google-url slug-port) + {"RUST_LOG" "info"}) + watch-cmd [(common/cargo-bin) "watch" + "-x" "run -p slugsocial-server" + "-w" "server/src" + "-w" "server/static" + "-w" "types/src"]] (try - (fs/create-dirs stable-dir) + (fs/create-dirs data-dir) (reset! !google (oauth/start-mock-google google-port :google-users ["google-user-alice" "google-user-bob"])) - (reset! !server (common/start-server server-bin server-env)) - (assert! (common/wait-for-server base-url 10000) "server did not become healthy") - (let [seeded (seed-demo! base-url) - summary {:base_url base-url - :mock_google_url google-url - :data_dir tmp-dir - :summary seeded}] - (spit summary-path (json/generate-string summary {:pretty true})) + (println "") + (println "starting cargo-watch (first compile may take a while)…") + (flush) + (reset! !server (p/process watch-cmd {:inherit true :env server-env})) + (assert! (common/wait-for-server base-url health-wait-ms) "server did not become healthy") + (let [summary (load-or-seed-summary! base-url google-url data-dir summary-path)] (println "") (println "walkthrough fixture ready") (println (str " base url: " base-url)) (println (str " room page: " (get-in summary [:summary :room :url]))) (println (str " thread page: " (get-in summary [:summary :room :thread_url]))) (println (str " garden page: " (get-in summary [:summary :room :garden_url]))) + (println (str " data dir: " data-dir)) (println (str " summary json: " summary-path)) (println "") (println "seeded users") (println " alice / bob via mock OAuth") (println "") + (println "editing server/src or server/static reloads the server; data persists in fixture-data/") + (println "") (println "press Ctrl-C to stop") (flush) (while true (Thread/sleep 1000))) (finally (when-some [s @!server] (common/kill-server s)) - (when-some [g @!google] ((:stop-fn g))) - (fs/delete-tree tmp-dir))))) + (when-some [g @!google] ((:stop-fn g))))))) Side B — contributor: tommy-mor Side B — commit message: [9ecc4e2e] nice Side B — unified diff (full patch): diff --git a/server/src/api/rpc.rs b/server/src/api/rpc.rs index 5675dcd2e28062acbe3c037b94b77c33adf32474..a18241f3ed7b4a248748fcefc799f9801ca62461 100644 --- a/server/src/api/rpc.rs +++ b/server/src/api/rpc.rs @@ -936,7 +936,15 @@ pub async fn handle_rpc_batch( line_ok(RpcResult::ForumThreads(rpc_list_forum_threads(&reduced, &room))) } RpcCommand::RoomCreate { slug, visibility } => { - match verify_bearer_principal(&headers, &*state.reduced.read().await) { + // Scope the first read so its guard drops before any nested `read().await` / `write().await`. + // A guard from `match verify(..., &*state.reduced.read().await)` would otherwise live for the + // whole `match` and deadlock here (tokio::sync::RwLock is not reentrant). + let principal = { + let reduced = state.reduced.read().await; + verify_bearer_principal(&headers, &*reduced) + }; + match principal { + Err((_, m)) => line_err(m, None), Ok(principal) => { let slug = slug.trim().to_lowercase(); if slug.is_empty() || slug.len() > 64 { @@ -994,7 +1002,6 @@ pub async fn handle_rpc_batch( } } } - Err((_, m)) => line_err(m, None), } } RpcCommand::RoomGrant { @@ -1002,17 +1009,28 @@ pub async fn handle_rpc_batch( username, capability, } => { - match verify_bearer_principal(&headers, &*state.reduced.read().await) { + let principal = { + let reduced = state.reduced.read().await; + verify_bearer_principal(&headers, &*reduced) + }; + match principal { Err((_, m)) => line_err(m, None), Ok(principal) => { - let reduced = state.reduced.read().await; - if !reduced.user_has_cap(&room, &principal, ThreadCapability::Manage) { + let can_manage = { + let reduced = state.reduced.read().await; + reduced.user_has_cap(&room, &principal, ThreadCapability::Manage) + }; + if !can_manage { line_err("requires Manage capability", None) } else { match parse_username(&username) { Err(msg) => line_err("invalid username", Some(msg)), Ok(target) => { - if !reduced.users_by_provider.values().any(|u| u == &target) { + let user_exists = { + let reduced = state.reduced.read().await; + reduced.users_by_provider.values().any(|u| u == &target) + }; + if !user_exists { line_err(format!("user @{target} not found"), None) } else { match parse_capability(&capability) { diff --git a/server/tests/integration.rs b/server/tests/integration.rs index 9162bd5bee84035e3908bfb9c8e201b7878ca339..b930120da09fe7d307f0411b84fb639fb8bd0b15 100644 --- a/server/tests/integration.rs +++ b/server/tests/integration.rs @@ -95,6 +95,23 @@ async fn test_healthz() { assert_eq!(response.text().await.unwrap(), "ok"); } +#[tokio::test] +async fn test_room_create_private_rpc() { + let (addr, _tmp, _log, _handle) = create_test_server().await; + let client = reqwest::Client::new(); + let batch = serde_json::json!([{ + "RoomCreate": { "slug": "secret-project", "visibility": "private" } + }]); + let body = rpc_batch(&client, addr, Some(&test_bearer()), batch).await; + let line = &body["results"][0]; + assert_eq!(line["ok"], true, "room create: {:?}", line); + let room_id = line["result"]["RoomCreated"]["room_id"].as_str().unwrap(); + assert!( + room_id.contains("/secret-project"), + "expected room_id to contain slug, got {room_id}" + ); +} + #[tokio::test] async fn test_index_page() { // HTML routes are offline during the auth-v3 refactor. diff --git a/test/auth.bb b/test/auth.bb index 611e04f1806ef81d678a91f499597fe55f691dd7..a67cc1de763434176d2f68e419dd37a8cd328395 100644 --- a/test/auth.bb +++ b/test/auth.bb @@ -176,7 +176,7 @@ (assert! (= 200 (:status poll)) "pending-session poll returns 200") (let [poll-json (json/parse-string (:body poll) true)] (assert! (:complete poll-json) "pending session complete=true") - (assert! (= "@bbuser" (:user poll-json)) "poll returns @bbuser") + (assert! (= "bbuser" (:user poll-json)) "poll returns stored username bbuser") (assert! (clojure.string/starts-with? (:token poll-json) "slug_") "poll returns bearer token") (println "\nwhoami…") @@ -184,7 +184,7 @@ :headers {"Authorization" (str "Bearer " (:token poll-json))})] (assert! (= 200 (:status who)) "whoami returns 200") (let [who-json (json/parse-string (:body who) true)] - (assert! (= "@bbuser" (:user who-json)) "whoami user is @bbuser")))))) + (assert! (= "bbuser" (:user who-json)) "whoami user is bbuser (stored form)")))))) (println "\nCLI: identity start → OAuth → identity poll → whoami…") (let [cli-home (str tmp-dir "/cli-home") @@ -208,7 +208,7 @@ (str "identity poll exits 0 (stderr: " (:err poll-proc) ")")) (let [poll-cli (json/parse-string (:out poll-proc) true)] (assert! (= "complete" (:phase poll-cli)) "identity poll --json phase") - (assert! (= "@cliuser" (:user poll-cli)) "CLI poll user") + (assert! (= "cliuser" (:user poll-cli)) "CLI poll user (stored form)") (assert! (clojure.string/starts-with? (:token poll-cli) "slug_") "CLI poll token") (let [token-path (str cli-home "/.config/slugsocial/token")] (assert! (fs/exists? token-path) "token written under isolated HOME") @@ -219,7 +219,7 @@ (assert! (zero? (:exit who-proc)) (str "whoami exits 0 (stderr: " (:err who-proc) ")")) (let [who-cli (json/parse-string (:out who-proc) true)] - (assert! (= "@cliuser" (:user who-cli)) "CLI whoami uses saved token")))))))) + (assert! (= "cliuser" (:user who-cli)) "CLI whoami uses saved token")))))))) (finally (when-some [s @!server] (common/kill-server s)) diff --git a/test/common.bb b/test/common.bb index ebb16c615a8b40e8830b5d5765d1e935a45538d0..4ed450377cdbb020f5ff164a812dfbbfc23c0f47 100644 --- a/test/common.bb +++ b/test/common.bb @@ -78,9 +78,20 @@ (defn start-server "Start the slugsocial-server binary with the given env map. - Returns the babashka.process map." - [server-bin env-map] - (p/process [server-bin] {:out :inherit :err :inherit :env env-map})) + Returns the babashka.process map. + + When `log-file` (string path) is provided, stdout and stderr are appended there + instead of inheriting the parent descriptors. Inheriting shared pipes while the + parent blocks on HTTP I/O can fill the pipe buffer and deadlock the server on log writes." + ([server-bin env-map] + (start-server server-bin env-map nil)) + ([server-bin env-map log-file] + (p/process [server-bin] + (if log-file + ;; Two string paths (same file): babashka.process can deref the process cleanly. + ;; :err :out + ProcessBuilder$Redirect breaks stream copying in deref/kill-server. + {:env env-map :out log-file :err log-file} + {:out :inherit :err :inherit :env env-map})))) (defn kill-server "Forcibly kill a server process (babashka.process map) and wait for it to exit." diff --git a/test/grants.bb b/test/grants.bb index 7066c1f2a57f39a362052f8524206dccf37bb7b1..793ba216f2de76a77eb20a76d08403db07ff411b 100644 --- a/test/grants.bb +++ b/test/grants.bb @@ -35,13 +35,14 @@ (defn- http-client [] (-> (java.net.http.HttpClient/newBuilder) (.followRedirects java.net.http.HttpClient$Redirect/ALWAYS) + (.connectTimeout (java.time.Duration/ofSeconds 15)) (.build))) (defn- http-get [url & {:keys [headers]}] (let [b (java.net.http.HttpRequest/newBuilder (java.net.URI/create url))] (doseq [[k v] (or headers {})] (.header b k v)) - (let [req (-> b (.GET) (.build)) + (let [req (-> b (.timeout (java.time.Duration/ofSeconds 60)) (.GET) (.build)) resp (.send (http-client) req (java.net.http.HttpResponse$BodyHandlers/ofString))] {:status (.statusCode resp) :body (.body resp)}))) @@ -52,6 +53,7 @@ (doseq [[k v] (or headers {})] (.header b k v)) (let [req (-> b + (.timeout (java.time.Duration/ofSeconds 60)) (.POST (java.net.http.HttpRequest$BodyPublishers/ofString body)) (.build)) resp (.send (http-client) req (java.net.http.HttpResponse$BodyHandlers/ofString))] @@ -67,6 +69,7 @@ b (java.net.http.HttpRequest/newBuilder (java.net.URI/create url))] (.header b "Content-Type" "application/x-www-form-urlencoded") (let [req (-> b + (.timeout (java.time.Duration/ofSeconds 60)) (.POST (java.net.http.HttpRequest$BodyPublishers/ofString pairs)) (.build)) resp (.send (http-client) req (java.net.http.HttpResponse$BodyHandlers/ofString))]