Website feedback: fixes from review

- Rate limiting fails open on KV errors instead of dropping feedback
- Break owner/repo#1, GH-1 and github.com references in user text
- Time the form with the browser's monotonic clock, not wall-clock
- Serialize lgtm approvals and rebase before pushing

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>

.github/workflows/approve-contributor.yml

@@ -7,6 +7,11 @@ on:
   issue_comment:
     types: [created]
 
+# One approval at a time, so two lgtm replies can't race on the same commit.
+concurrency:
+  group: approve-contributor
+  cancel-in-progress: false
+
 jobs:
   approve:
     runs-on: ubuntu-latest
@@ -183,6 +188,8 @@ jobs:
           git config user.email "github-actions[bot]@users.noreply.github.com"
           git add .github/APPROVED_CONTRIBUTORS
           git diff --staged --quiet || git commit -m "chore: approve contributors from issue #${{ github.event.issue.number }}"
+          # main may have moved since checkout; replay the approval on top of it.
+          git pull --rebase origin "${{ github.event.repository.default_branch }}"
           git push
 
       - name: Comment on issue

site/functions/api/feedback.js

@@ -45,7 +45,9 @@ export async function onRequestPost({ request, env }) {
   if (checked.spam) return json(200, { ok: true });
   if (checked.error) return json(400, { error: checked.error });
 
-  if (env.FEEDBACK_RL) {
+  // Best effort: KV is eventually consistent, so bursts can slip past, and a
+  // storage error lets the feedback through rather than losing it.
+  if (env.FEEDBACK_RL) try {
     const ip = request.headers.get("cf-connecting-ip") || "unknown";
     const hour = Math.floor(Date.now() / 3600e3);
     const day = Math.floor(Date.now() / 86400e3);
@@ -57,6 +59,8 @@ export async function onRequestPost({ request, env }) {
     if (await overLimit(env.FEEDBACK_RL, `day:${day}`, TOTAL_PER_DAY, 90000)) {
       return json(429, { error: "The form has had a busy day. Try again tomorrow, or use GitHub." });
     }
+  } catch (err) {
+    console.log(`Rate limit check failed: ${err}`);
   }
 
   const res = await fetch(`https://api.github.com/repos/${env.GITHUB_REPO}/issues`, {

site/lib/feedback.js

@@ -17,18 +17,25 @@ const GITHUB_LOGIN = /^[A-Za-z0-9](?:[A-Za-z0-9-]{0,37}[A-Za-z0-9])?$/;
 
 const oneLine = (value, max) => String(value ?? "").replace(/\s+/g, " ").trim().slice(0, max);
 
-// Mentions in someone else's text would ping strangers, and issue refs would
-// cross-link, so break both with a zero-width space.
+// Mentions in someone else's text would ping strangers, and issue references
+// (#1, owner/repo#1, GH-1, github.com links) would add backlinks to other
+// people's issues, so break them all with a zero-width space.
+const ZWSP = "\u200b";
 export function defang(text) {
-  return text.replace(/@(?=[A-Za-z0-9])/g, "@​").replace(/(^|\s)#(?=\d)/g, "$1#​");
+  return text
+    .replace(/@(?=[A-Za-z0-9])/g, `@${ZWSP}`)
+    .replace(/#(?=\d)/g, `#${ZWSP}`)
+    .replace(/\b(GH)-(?=\d)/gi, `$1${ZWSP}-`)
+    .replace(/\b(github)\.com/gi, `$1${ZWSP}.com`);
 }
 
 // Returns { error } or { value } with every field trimmed and bounded.
-export function validate(input, now = Date.now()) {
+export function validate(input) {
   if (!input || typeof input !== "object") return { error: "Send the form as JSON." };
   if (oneLine(input.website, 200)) return { spam: true };
-  const started = Number(input.started);
-  if (!Number.isFinite(started) || now - started < MIN_FILL_MS) return { spam: true };
+  // Measured in the browser with a monotonic clock, so clock skew doesn't matter.
+  const elapsed = Number(input.elapsed);
+  if (!Number.isFinite(elapsed) || elapsed < MIN_FILL_MS) return { spam: true };
 
   const kind = Object.hasOwn(KINDS, input.kind) ? input.kind : "other";
   const title = oneLine(input.title, LIMITS.title[1]);

site/public/js/feedback.js

@@ -2,7 +2,7 @@
 const form = document.getElementById("feedback");
 const errorBox = document.getElementById("error");
 const send = document.getElementById("send");
-const started = Date.now();
+const started = performance.now();
 
 const HINTS = {
   bug: "What you tried, what happened, and what you expected.",
@@ -50,7 +50,7 @@ form.addEventListener("submit", async (e) => {
   e.preventDefault();
   errorBox.hidden = true;
   const data = Object.fromEntries(new FormData(form));
-  data.started = started;
+  data.elapsed = Math.round(performance.now() - started);
 
   if (data.title.trim().length < 5) {
     form.elements.title.focus();

site/test/feedback.test.mjs

@@ -3,17 +3,16 @@ import assert from "node:assert/strict";
 import { test } from "node:test";
 import { buildIssue, defang, hashIp, MIN_FILL_MS, validate } from "../lib/feedback.js";
 
-const now = 1_800_000_000_000;
 const form = (over = {}) => ({
   kind: "bug",
   title: "Live view freezes",
   message: "After about a minute the live view stops updating.",
-  started: now - MIN_FILL_MS - 1,
+  elapsed: MIN_FILL_MS + 1,
   ...over,
 });
 
 test("accepts a normal report and trims it", () => {
-  const { value, error } = validate(form({ title: "  Live   view freezes ", os: " macOS 26 " }), now);
+  const { value, error } = validate(form({ title: "  Live   view freezes ", os: " macOS 26 " }));
   assert.equal(error, undefined);
   assert.equal(value.title, "Live view freezes");
   assert.equal(value.os, "macOS 26");
@@ -21,25 +20,25 @@ test("accepts a normal report and trims it", () => {
 });
 
 test("flags the honeypot and too-fast submissions as spam", () => {
-  assert.deepEqual(validate(form({ website: "http://spam" }), now), { spam: true });
-  assert.deepEqual(validate(form({ started: now - 500 }), now), { spam: true });
-  assert.deepEqual(validate(form({ started: undefined }), now), { spam: true });
+  assert.deepEqual(validate(form({ website: "http://spam" })), { spam: true });
+  assert.deepEqual(validate(form({ elapsed: 500 })), { spam: true });
+  assert.deepEqual(validate(form({ elapsed: undefined })), { spam: true });
 });
 
 test("rejects short, long and malformed input", () => {
-  assert.match(validate(form({ title: "hi" }), now).error, /title/);
-  assert.match(validate(form({ message: "short" }), now).error, /more/);
-  assert.match(validate(form({ message: "x".repeat(5001) }), now).error, /under/);
-  assert.match(validate(form({ github: "not a user!" }), now).error, /GitHub/);
-  assert.match(validate(null, now).error, /JSON/);
+  assert.match(validate(form({ title: "hi" })).error, /title/);
+  assert.match(validate(form({ message: "short" })).error, /more/);
+  assert.match(validate(form({ message: "x".repeat(5001) })).error, /under/);
+  assert.match(validate(form({ github: "not a user!" })).error, /GitHub/);
+  assert.match(validate(null).error, /JSON/);
 });
 
 test("unknown kinds become other feedback", () => {
-  assert.equal(validate(form({ kind: "__proto__" }), now).value.kind, "other");
+  assert.equal(validate(form({ kind: "__proto__" })).value.kind, "other");
 });
 
 test("builds a labelled issue that credits a GitHub user", () => {
-  const { value } = validate(form({ github: "@octocat", version: "0.3.1", steamos: "20260922" }), now);
+  const { value } = validate(form({ github: "@octocat", version: "0.3.1", steamos: "20260922" }));
   const issue = buildIssue(value);
   assert.equal(issue.title, "Bug report: Live view freezes");
   assert.deepEqual(issue.labels, ["feedback", "bug"]);
@@ -48,15 +47,17 @@ test("builds a labelled issue that credits a GitHub user", () => {
 });
 
 test("anonymous feedback says replies won't reach the sender", () => {
-  const issue = buildIssue(validate(form({ kind: "other" }), now).value);
+  const issue = buildIssue(validate(form({ kind: "other" })).value);
   assert.deepEqual(issue.labels, ["feedback"]);
   assert.match(issue.body, /won't see replies/);
 });
 
 test("breaks mentions, issue refs and table cells in user text", () => {
-  assert.equal(defang("ping @valve about #12"), "ping @​valve about #​12");
-  assert.equal(defang("email me@example.com"), "email me@​example.com");
-  const issue = buildIssue(validate(form({ os: "a | b" }), now).value);
+  assert.equal(defang("ping @valve about #12"), "ping @\u200bvalve about #\u200b12");
+  assert.equal(defang("email me@example.com"), "email me@\u200bexample.com");
+  assert.equal(defang("see valve/steam#7 and GH-8"), "see valve/steam#\u200b7 and GH\u200b-8");
+  assert.equal(defang("https://github.com/a/b/issues/1"), "https://github\u200b.com/a/b/issues/1");
+  const issue = buildIssue(validate(form({ os: "a | b" })).value);
   assert.match(issue.body, /\| a \\\| b \|/);
 });