chore: merge fix/watch-mode-keepalive-tri-13065 (review fixes round 2)

Author: kathiekiwi

.changeset/chat-stream-mid-turn-reconnect.md

@@ -4,4 +4,4 @@
 "trigger.dev": patch
 ---
 
-Chat in the browser now reconnects when the connection drops mid-turn, instead of leaving the reply stuck as if it were still generating. Reports can be fetched as structured data with the `json` format, and the shortest report period is now one minute (`30m`, `1h`, `7d`). The `mint-token` command's help is clearer too: a token minted without `--cap` is read-only, and `--ttl` shows the correct maximum lifetime of 7 days.
+Chat in the browser now reconnects when the connection drops mid-turn, instead of leaving the reply stuck as if it were still generating. Reports can be fetched as structured data with the `json` format, and the shortest report period is now one minute (`1m`, `30m`, `1h`, `7d`). The `mint-token` command's help is clearer too: a token minted without `--cap` is read-only, and `--ttl` shows the correct maximum lifetime of 7 days.

.changeset/chat-stream-supersede-race.md

@@ -0,0 +1,5 @@
+---
+"@trigger.dev/sdk": patch
+---
+
+Fixed a race where quickly restarting a chat stream could break stop and reconnect for the new stream.

.claude/agents/code-reviewer.md

@@ -0,0 +1,13 @@
+---
+name: code-reviewer
+description: Adversarially verifies one landed packet against its requirement; read-only.
+model: opus
+---
+
+You are an adversarial code reviewer for one landed packet. READ-ONLY: never modify code, never commit, never push, never post to GitHub.
+
+- Try to refute that the change answers its stated requirement; look for the failure scenario, not confirmation.
+- Check the diff for unrelated drift, dead code, broken semantics of neighbors, and whether tests prove the actual invariant (would the test fail if the fix were subtly wrong?).
+- Check the change landed in the correct PR/branch of the stack.
+- Distinguish fact from inference; cite exact file:line evidence.
+- Return: verdict (approve / needs-changes) with evidence per concern, and the exact minimal correction when needs-changes.

.claude/agents/code-writer.md

@@ -0,0 +1,16 @@
+---
+name: code-writer
+description: Implements exactly one work packet — minimal diff, targeted checks, own-paths-only commits.
+model: opus
+---
+
+You are a code writer. Implement exactly the one work packet in your prompt.
+
+- Minimal diff; match surrounding style and idiom.
+- Prefer no comment at all; comment only a non-obvious constraint, max 2 short lines. All texts (comments, commit messages) short, clear, simple.
+- Verify the packet's own diagnosis against the code before applying; if it is wrong, STOP without committing and report why.
+- Run only the targeted checks for your packet: the relevant vitest files, `pnpm run typecheck --filter <pkg>` when the change warrants it. Never full suites unless asked.
+- `pnpm run format` on touched files before committing.
+- Stage and commit ONLY your packet's files. Conventional commit message. NO Claude attribution, no Co-Authored-By.
+- Push only if the packet explicitly says to.
+- Return: what changed, evidence (test output), commit SHA, and anything contradicting the diagnosis.

.claude/agents/software-architect.md

@@ -0,0 +1,12 @@
+---
+name: software-architect
+description: Resolves contested design questions against the specs; decision + rationale, never code.
+model: opus
+---
+
+You are a software architect. Resolve exactly the contested design question in your prompt against the given specs/contracts. READ-ONLY.
+
+- Ground the decision in the actual code and the project's design contracts (GUIDEBOOK, Linear specs) — not in generic best practice.
+- Weigh stack boundaries: which PR owns the change, what merges independently.
+- Prefer the smallest decision that unblocks the packet; flag speculative architecture rather than endorsing it.
+- Return: the decision, its rationale, rejected alternatives (one line each), and exactly what the dependent packet should do.

apps/webapp/app/components/code/StreamdownRenderer.test.ts

@@ -20,6 +20,16 @@ describe("restrictModelUrls (image src)", () => {
     expect(restrictModelUrls("//evil.tld/pixel.gif", "src", img)).toBeUndefined();
   });
 
+  it("drops a backslash-authority image, which the browser reads as protocol-relative", () => {
+    expect(restrictModelUrls("\\\\evil.example/pixel.gif", "src", img)).toBeUndefined();
+    expect(restrictModelUrls("/\\evil.example/pixel.gif", "src", img)).toBeUndefined();
+  });
+
+  it("drops an image hidden behind a leading C0 control, which the URL parser discards", () => {
+    expect(restrictModelUrls("\u0001//evil.tld/p.gif", "src", img)).toBeUndefined();
+    expect(restrictModelUrls("\u0000https://evil.tld/p.gif", "src", img)).toBeUndefined();
+  });
+
   it("keeps inline and same-origin images", () => {
     expect(restrictModelUrls("data:image/png;base64,AAAA", "src", img)).toBe(
       "data:image/png;base64,AAAA"

apps/webapp/app/components/code/StreamdownRenderer.tsx

@@ -12,18 +12,25 @@ const SAFE_LINK_SCHEMES = new Set(["http:", "https:", "mailto:"]);
 export const restrictModelUrls: UrlTransform = (url, key, node) => {
   const value = url.trim();
   const isImage = node.tagName === "img" || key === "src" || key === "srcset";
+  // What the browser will actually resolve, which is not what `trim()` leaves: the URL parser
+  // drops C0 controls (`trim()` keeps them) and reads `\` as `/` for special schemes, so
+  // a leading-control `//evil.tld` and `\\evil.tld` both name a remote host. Classify on this; return the
+  // original `url` untouched whenever it is allowed.
+  const normalized = value
+    .replace(/[\u0000-\u001f]/g, "")
+    .replace(/^[/\\]+/, (run) => "/".repeat(run.length));
 
   if (isImage) {
     // Inline images carry their own bytes; a relative path resolves to our own origin.
-    if (/^data:/i.test(value) || /^blob:/i.test(value)) return url;
+    if (/^data:/i.test(normalized) || /^blob:/i.test(normalized)) return url;
     // Absolute or protocol-relative means a remote host — strip it so nothing is fetched.
-    if (/^[a-z][a-z0-9+.-]*:/i.test(value) || value.startsWith("//")) return undefined;
+    if (/^[a-z][a-z0-9+.-]*:/i.test(normalized) || normalized.startsWith("//")) return undefined;
     return url;
   }
 
   // Links: relative and protocol-relative are fine; otherwise require a safe scheme.
-  if (value.startsWith("//")) return url;
-  const schemeMatch = /^([a-z][a-z0-9+.-]*):/i.exec(value);
+  if (normalized.startsWith("//")) return url;
+  const schemeMatch = /^([a-z][a-z0-9+.-]*):/i.exec(normalized);
   if (!schemeMatch) return url;
   return SAFE_LINK_SCHEMES.has(`${schemeMatch[1].toLowerCase()}:`) ? url : undefined;
 };

apps/webapp/app/components/dashboard-agent/DashboardAgentMessages.render.test.ts

@@ -64,8 +64,13 @@ function markup(parts: unknown[]) {
   );
 }
 
+/** Every needle must be present: a missing one is -1, and -1 comparisons pass vacuously. */
 function order(html: string, ...needles: string[]) {
-  return needles.map((needle) => html.indexOf(needle));
+  return needles.map((needle) => {
+    const at = html.indexOf(needle);
+    expect(at, `missing "${needle}"`).toBeGreaterThan(-1);
+    return at;
+  });
 }
 
 describe("action rows render at the end of the turn", () => {

apps/webapp/app/components/dashboard-agent/model-markdown.test.ts

@@ -31,6 +31,29 @@ describe("stripModelImages", () => {
     expect(html).not.toContain("attacker.example");
   });
 
+  it("strips a nested tag, which one pass would splice back into a whole one", () => {
+    expect(stripModelImages(`<im<img src="${BEACON}">g src="${BEACON}">`)).not.toContain(
+      "attacker.example"
+    );
+    expect(stripModelImages("<scr<script>ipt>")).toBe("");
+  });
+
+  it("bails out safely on nesting too deep to unwrap, instead of looping on it", () => {
+    // Each layer needs one more pass, so this is the shape that makes the strip quadratic.
+    let nested = `<script src="${BEACON}">`;
+    for (let i = 0; i < 20_000; i++) nested = `<scr${nested}ipt>`;
+
+    const started = performance.now();
+    const stripped = stripModelImages(nested);
+    const elapsed = performance.now() - started;
+
+    // No bracket survives, so nothing can parse as an element — the URL is left as inert prose.
+    expect(stripped).not.toContain("<script");
+    expect(stripped).not.toContain("<");
+    expect(render(stripped)).not.toMatch(/<script\b/i);
+    expect(elapsed).toBeLessThan(1_000);
+  });
+
   it("strips an image hidden inside a code fence", () => {
     const stripped = stripModelImages(`\`\`\`\n![](${BEACON})\n\`\`\``);
     expect(stripped).not.toContain("attacker.example");

apps/webapp/app/components/dashboard-agent/model-markdown.ts

@@ -13,10 +13,33 @@ function plainAlt(alt: string): string {
   return alt.replace(/[![\]<>`]/g, "").trim();
 }
 
+/**
+ * One pass isn't enough: removing a match can splice the surrounding text into a fresh one
+ * (`<scr<script>ipt>`), so repeat until nothing changes. Nesting deep enough to need more than
+ * `MAX_PASSES` is adversarial, not real model output, and looping it out is quadratic on the
+ * render thread — so past the bound we stop and blunt every character the strips key on. The
+ * result is over-stripped, never half-stripped.
+ */
+const MAX_PASSES = 25;
+const STRIP_CHARS = /[![\]<>]/g;
+
+function replaceUntilStable(
+  text: string,
+  pattern: RegExp,
+  replacer: (whole: string, ...groups: string[]) => string
+): string {
+  let current = text;
+  for (let pass = 0; pass < MAX_PASSES; pass++) {
+    const next = current.replace(pattern, replacer);
+    if (next === current) return current;
+    current = next;
+  }
+  return current.replace(STRIP_CHARS, "");
+}
+
 // Strips inside code fences too: a fence-aware pass is bypassable with a half-fence.
 export function stripModelImages(text: string): string {
-  return text
-    .replace(MARKDOWN_IMAGE, (_whole, alt: string) => plainAlt(alt))
-    .replace(MARKDOWN_SHORTCUT_IMAGE, (_whole, alt: string) => plainAlt(alt))
-    .replace(FETCHING_TAG, "");
+  let out = replaceUntilStable(text, MARKDOWN_IMAGE, (_whole, alt: string) => plainAlt(alt));
+  out = replaceUntilStable(out, MARKDOWN_SHORTCUT_IMAGE, (_whole, alt: string) => plainAlt(alt));
+  return replaceUntilStable(out, FETCHING_TAG, () => "");
 }