Side B fixes a real production NameError in the actual application code (missing GITHUB_API_BASE_URL definition), directly preventing a crash in the OAuth callback path. Side A's changes, while more extensive, are confined to test infrastructure/mocks—valuable for test reliability but not fixing a production defect; it also introduces a subtle regression risk by using 0 instead of -1 for redirect content-length without clear justification.
constitution · epochs · watch · epoch 3
comparison
c_597d3f736194 (tommy-mor) vs c_4ed2e2cca39b (tommy-mor)
download prompt · raw event · cmp_51ff42b1ebd316
council reasoning
B fixes a production NameError by defining the missing GITHUB_API_BASE_URL used in the real OAuth callback path, which directly restores live GitHub auth. A only repairs test doubles (query split regex, getRequestBody, null-safe token/state handling) and Playwright helpers so E2E mocks work again—valuable, but not user-facing runtime behavior.
Side A repairs multiple concrete failures in the OAuth test infrastructure: it fixes query parsing (`str/split` regex), reads POST bodies from `getRequestBody`, guards against nil tokens and states, corrects redirect response handling, wraps mock handlers to avoid crashes, and updates Playwright helpers to read the right selectors and wait reliably. Side B is a useful but narrowly scoped fix that defines `GITHUB_API_BASE_URL` with a default to eliminate a callback `NameError`; its impact is limited compared with the broader restoration of end-to-end authentication testing in Side A.
sides
A — c_597d3f736194 (tommy-mor)
message
[075d4d37] Fix OAuth test mocks so Clojure E2E auth flows work again. HttpServer handlers were crashing on query parsing and token POSTs, which broke Playwright login; also read alias/history via real CSS selectors. Co-authored-by: Cursor <cursoragent@cursor.com>
diff preview
diff --git a/test/auth_login.clj b/test/auth_login.clj
index 6f2f00d7aa7340e63d1ac465b0a2234cec7a983f..aa63b66ccb2471b710ba3d168d0461252b39fc10 100644
--- a/test/auth_login.clj
+++ b/test/auth_login.clj
@@ -14,27 +14,27 @@
(defn- type-alias! [pg text]
(page/evaluate pg
(.replace
- "(() => { const i = document.getElementById('alias-input'); const f = document.getElementById('alias-check-form'); if (!i || !f) return;
+ "(() => { const i = document.getElementById('alias-input'); const f = document.getElementById('alias-check-form'); if (!i || !f) return Promise.resolve('missing-form');
i.value = __TEXT__;
const cf = document.getElementById('alias-claim-field'); if (cf) cf.value = i.value;
return fetch(f.action, { method: 'POST', credentials: 'same-origin',
headers: { 'Content-Type': 'application/x-www-form-urlencoded' },
body: new URLSearchParams(new FormData(f)).toString() })
.then(function (r) { return r.text(); })
- .then(function (t) { eval(t); }); })()"
+ .then(function (t) { eval(t); return document.getElementById('alias-status')?.textContent || ''; }); })()"
"__TEXT__"
- (pr-str text)))
- (Thread/sleep 400))
+ (pr-str text))))
-(defn- element-text [pg test-id]
+(defn- element-text [pg selector]
(let [raw (page/evaluate pg
- (str "document.querySelector('[data-testid=\"" test-id "\"]')?.textContent || ''"))]
+ (str "document.querySelector(" (pr-str selector) ")?.textContent || ''"))]
(when (string? raw) (str/trim raw))))
(defn- wait-for-text [pg test-id text timeout-ms]
- (let [deadline (+ (System/currentTimeMillis) timeout-ms)]
+ (let [deadline (+ (System/currentTimeMillis) timeout-ms)
+ selector (str "[data-testid=\"" test-id "\"]")]
(loop []
- (let [got (or (element-text pg test-id) "")]
+ (let [got (or (element-text pg selector) "")]
(cond
(= got text) got
(< (System/currentTimeMillis) deadline) (do (Thread/sleep 200) (recur))
diff --git a/test/support/mock_oauth.clj b/test/support/mock_oauth.clj
index 5ba7e9be3cf6d2226648e9609a09ed45f306930f..333fe08d56cb06bac2a338cad1c73894d54c4495 100644
--- a/test/support/mock_oauth.clj
+++ b/test/support/mock_oauth.clj
@@ -7,7 +7,7 @@
(defn- query-param [query key]
(when query
(some (fn [pair]
- (let [[k v] (str/split pair "=" 2)]
+ (let [[k v] (str/split pair #"=" 2)]
(when (= k key)
(URLDecoder/decode (or v "") "UTF-8"))))
(str/split query #"&"))))
@@ -31,11 +31,11 @@
(defn- send-redirect [^HttpExchange ex location]
(.set (.getResponseHeaders ex) "Location" location)
- (.sendResponseHeaders ex 302 -1)
+ (.sendResponseHeaders ex 302 0)
(.close (.getResponseBody ex)))
(defn- read-form [^HttpExchange ex]
- (let [body (slurp (.getInputStream ex))]
+ (let [body (slurp (.getRequestBody ex))]
{:code (query-param body "code")
:grant (query-param body "grant_type")}))
@@ -45,7 +45,7 @@
(str/replace #"^[Bb]earer " "")))
(defn- parse-token-user [token]
- (when (str/starts-with? token "mock:")
+ (when (and token (str/starts-with? token "mock:"))
(parse-mock-user (subs token 5))))
(defn- authorize-redirect [exchange query]
@@ -55,7 +55,7 @@
user (parse-mock-user mock-user)
code (str "mock:" (:id user) ":" (:login user))
loc (str redirect-uri "?code=" (java.net.URLEncoder/encode code "UTF-8")
- "&state=" (java.net.URLEncoder/encode state "UTF-8"))]
+ "&state=" (java.net.URLEncoder/encode (or state "") "UTF-8"))]
(send-redirect exchange loc)))
(defn start-mock-oauth
@@ -65,49 +65,56 @@
handler
(proxy [HttpHandler] []
(handle [^HttpExchange exchange]
- (let [uri (.getRequestURI exchange)
- path (.getPath uri)
- query (.getQuery uri)
- method (.getRequestMethod exchange)]
- (cond
- ;; GitHub authorize
- (str/ends-with? path "/login/oauth/authorize")
- (authorize-redirect exchange query)
+ (try
+ (let [uri (.getRequestURI exchange)
+ path (.getPath uri)
+ query (.getQuery uri)
+ method (.getRequestMethod exchange)]
+ (cond
+ ;; GitHub authorize
+ (str/ends-with? path "/login/oauth/authorize")
+ (authorize-redirect exchange query)
- ;; Reddit authorize
- (str/ends-with? path "/api/v1/authorize")
- (authorize-redirect exchange query)
+ ;; Reddit authorize
+ (str/ends-with? path "/api/v1/authorize")
+ (authorize-redirect exchange query)
- ;; GitHub token
- (and (= method "POST") (str/ends-with? path "/login/oauth/access_token"))
- (let [code (or (:code (read-form exchange)) "mock:1002:newbie")]
- (send-json exchange 200 (str "{\"access_token\":\"" code "\",\"token_type\":\"bearer\"}")))
+ ;; GitHub token
+ (and (= method "POST") (str/ends-with? path "/login/oauth/access_token"))
+ (let [code (or (:code (read-form exchange)) "mock:1002:newbie")]
+ (send-json exchange 200 (str "{\"access_token\":\"" code "\",\"token_type\":\"bearer\"}")))
- ;; Reddit token (client_credentials for import + authorization_code for login)
- (and (= method "POST") (str/ends-with? path "/api/v1/access_token"))
- (let [form (read-form exchange)
- grant (or (:grant form) "")
- code (or (:code form) "mock:t2_test:redditor")]
- (if (= grant "client_credentials")
- (send-json exchange 200 "{\"access_token\":\"app-token\",\"token_type\":\"bearer\",\"expires_in\":3600}")
- (send-json exchange 200 (str "{\"access_token\":\"" code "\",\"token_type\":\"bearer\",\"expires_in\":3600}"))))
+ ;; Reddit token (client_credentials for import + authorization_code for login)
+ (and (= method "POST") (str/ends-with? path "/api/v1/access_token"))
+ (let [form (read-form exchange)
+ grant (or (:grant form) "")
+ code (or (:code form) "mock:t2_test:redditor")]
+ (if (= grant "client_credentials")
+ (send-json exchange 200 "{\"access_token\":\"app-token\",\"token_type\":\"bearer\",\"expires_in\":3600}")
+ (send-json exchange 200 (str "{\"access_token\":\"" code "\",\"token_type\":\"bearer\",\"expires_in\":3600}"))))
- ;; GitHub user
- (= path "/user")
- (let [token (bearer-token exchange)
- user (or (parse-token-user token) {:id "1002" :login "newbie" :numeric? true})]
- (send-json exchange 200
- (str "{\"id\":" (:id user) ",\"login\":\"" (:login user) "\"}")))
+ ;; GitHub user
+ (= path "/user")
+ (let [token (bearer-token exchange)
+ user (or (parse-token-user token) {:id "1002" :login "newbie" :numeric? true})]
+ (send-json exchange 200
+ (str "{\"id\":" (:id user) ",\"login\":\"" (:login user) "\"}")))
- ;; Reddit /api/v1/me
- (str/ends-with? path "/api/v1/me")
- (let [token (bearer-token exchange)
- user (or (parse-token-user token) {:id "t2_test" :login "redditor"})]
- (send-json exchange 200
- (str "{\"id\":\"" (:id user) "\",\"name\":\"" (:login user) "\"}")))
+ ;; Reddit /api/v1/me
+ (str/ends-with? path "/api/v1/me")
+ (let [token (bearer-token exchange)
+ user (or (parse-token-user token) {:id "t2_test" :login "redditor"})]
+ (send-json exchange 200
+ (str "{\"id\":\"" (:id user) "\",\"name\":\"" (:login user) "\"}")))
- :else
- (send-json exchange 404 "{\"error\":\"not found\"}")))))]
+ :else
+ (send-json exchange 404 "{\"error\":\"not found\"}")))
+ (catch Throwable t
+ (binding [*out* *err*]
+ (println "mock-oauth handler error:" t))
+ (try
+ (send-json exchange 500 "{\"error\":\"mock-oauth internal\"}")
+ (catch Throwable _))))))]
(.createContext server "/" handler)
(.setExecutor server nil)
(.start server)
diff --git a/test/support/mock_reddit.clj b/test/support/mock_reddit.clj
index a630cf0938722193e9af88382d60e777ff371be4..faa27945b6394363914c853627a0bad1e819a5f9 100644
--- a/test/support/mock_reddit.clj
+++ b/test/support/mock_reddit.clj
@@ -12,7 +12,7 @@
(defn- query-param [query key]
(when query
(some (fn [pair]
- (let [[k v] (str/split pair "=" 2)]
+ (let [[k v] (str/split pair #"=" 2)]
(when (= k key)
(URLDecoder/decode (or v "") "UTF-8"))))
(str/split query #"&"))))
@@ -34,11 +34,11 @@
(defn- send-redirect [^HttpExchange ex location]
(.set (.getResponseHeaders ex) "Location" location)
- (.sendResponseHeaders ex 302 -1)
+ (.sendResponseHeaders ex 302 0)
(.close (.getResponseBody ex)))
(defn- read-form [^HttpExchange ex]
- (let [body (slurp (.getInputStream ex))]
+ (let [body (slurp (.getRequestBody ex))]
{:code (query-param body "code")
:grant (query-param body "grant_type")}))
@@ -48,7 +48,7 @@
(str/replace #"^[Bb]earer " "")))
(defn- parse-token-user [token]
- (when (str/starts-with? token "mock:")
+ (when (and token (str/starts-with? token "mock:"))
(parse-mock-user (subs token 5))))
(defn start-mock-reddit
@@ -72,7 +72,7 @@
user (parse-mock-user (query-param query "mock_user"))
code (str "mock:" (:id user) ":" (:login user))
loc (str redirect-uri "?code=" (java.net.URLEncoder/encode code "UTF-8")
- "&state=" (java.net.URLEncoder/encode state "UTF-8"))]
+ "&state=" (java.net.URLEncoder/encode (or state "") "UTF-8"))]
(send-redirect exchange loc))
(and (= method "POST") (str/ends-with? path "/api/v1/access_token"))
B — c_4ed2e2cca39b (tommy-mor)
message
[0d3270d1] Fix GitHub OAuth callback NameError on missing API base URL. Co-authored-by: Cursor <cursoragent@cursor.com>
diff preview
diff --git a/constitution.py b/constitution.py
index f819007252f435680b8356fb4da83469b21e33af..4dd5b9dfbba231d46289c490f91b1dd5b1018bcf 100644
--- a/constitution.py
+++ b/constitution.py
@@ -119,6 +119,9 @@ JSONL_PATH = pathlib.Path(os.environ.get("JSONL_PATH", "/data/ledger.jsonl"))
GITHUB_CLIENT_ID = os.environ.get("GITHUB_CLIENT_ID", "")
GITHUB_CLIENT_SECRET = os.environ.get("GITHUB_CLIENT_SECRET", "")
+GITHUB_API_BASE_URL = os.environ.get(
+ "GITHUB_API_BASE_URL", "https://api.github.com"
+).rstrip("/")
OPENROUTER_API_KEY = os.environ.get("OPENROUTER_API_KEY", "")
OPENROUTER_BASE_URL = os.environ.get("OPENROUTER_BASE_URL", "https://openrouter.ai").rstrip("/")
Hardlinks — judgments / attempts / prompt
judgments
attempts
Prompt text is loaded only by the download route.