Commit2e8a4d5unknown_key

fix(ai-review): fail closed on parse error + fix broken legal links + audit

fix(ai-review): fail closed on parse error + fix broken legal links + audit

Four-agent audit (route inventory / link-and-form validation / bun test +
tsc + smoke crawl / critical-user-journey trace) of the codebase. Full
findings in AUDIT.md. Health snapshot: 1995/1995 tests pass, tsc clean,
zero broken links/forms/fetches across 172 routes.

This commit fixes the two P0 bugs the audit surfaced, plus a stub-DB
crash:

1. AI review silently auto-approved PRs on JSON parse failure
   (src/lib/ai-review.ts). The reviewDiff() catch block returned
   `approved: true` whenever Claude's output drifted off the JSON spec,
   and `approved: parsed.approved !== false` meant a missing field also
   counted as approval. K2 auto-merge reads this signal — so a parse
   blip could silently merge a PR with no real review. Now fails
   closed: requires explicit `parsed.approved === true`, otherwise
   surfaces the parse error in the summary comment and returns
   `approved: false`.

2. Register form linked to /legal/terms and /legal/privacy (404). The
   mounted routes live at /terms and /privacy via src/routes/legal.tsx.
   The src/routes/legal/ subdirectory contains an unmounted set of
   alternate routes — dead code, flagged in AUDIT.md but not deleted in
   this commit. Updated auth.tsx to use the live paths.

3. AUDIT.md — full findings document, severity-ranked. Lists 4 P0 (3
   fixed in earlier commits + 1 fixed here), 7 P1 (dead routes, hangs
   on stub DB, silent failures), 3 P2 (pattern issues, no current
   crash source). Roadmap section for "usable as a normal website".

Tests touched: ai-review.test.ts, auto-merge.test.ts,
pulls-ai-rereview.test.ts, mcp-write.test.ts — 66/66 still pass.

Claude committed on May 16, 2026Parent: 67f64a3

3 files changed+161−82e8a4d528a4bf9bd76336019903de0d2c59c2400

3 changed files+161−8

AddedAUDIT.md+142−0View fileUnified Split

@@ -0,0 +1,142 @@

1# Gluecron Audit — 2026-05-16

3Comprehensive end-to-end audit run on branch `claude/fix-aa-loop-issue-PonMQ`

4at commit `67f64a3`. Four parallel agents covered: route inventory, link/form

5target validation, automated checks (bun test + tsc + smoke crawl), and

6critical-user-journey code-path tracing.

8## Health snapshot

10| Check | Result |

11|---|---|

12| `bun test` | **1995 pass / 0 fail / 2 skip** across 143 files |

13| `bunx tsc --noEmit` | **clean — zero errors** |

14| Registered HTTP routes | **172** |

15| Broken internal `href` | **1** — register form links to non-existent `/legal/*` |

16| Broken form actions | 0 |

17| Broken client `fetch()` calls | 0 |

18| Routes hanging on stub DB | 2 — `/` and `/status` |

19| Routes 500ing on stub DB | 1 — `/explore` |

21## Verified bugs (severity-ranked)

23### P0 — Fixed this session

251. **AA reload loop on admin dashboard.** Layout registered `/sw.js` and

26 `/sw-push.js` at the same scope `/`; the SW spec only allows one

27 active SW per scope, so each registration kept replacing the other and

28 the `updatefound → reload` hook fired on every page load. Cause of the

29 "deploy pill flashing" / "typing wiped" / "buttons don't work"

30 symptoms. **Commit `d7ba05d`.** Includes regression test.

322. **AI code review silently auto-approves PRs.** `src/lib/ai-review.ts`

33 `reviewDiff()` returned `{ approved: true }` whenever Claude's output

34 couldn't be parsed as JSON. Combined with the `approved: parsed.approved

35 !== false` defaulting, **a missing field or any parse failure became an

36 approval signal**, which feeds the K2 auto-merge gate. Fail-open on a

37 security-relevant decision. Now fails closed: explicit `approved: true`

38 required, otherwise returns `false` with a parseable failure summary.

39 **(committed in this batch.)**

413. **Broken legal links in register form.** `src/routes/auth.tsx` linked

42 to `/legal/terms` and `/legal/privacy`; mounted routes are `/terms`,

43 `/privacy`, `/acceptable-use` (no `/legal/` prefix). 404 on click.

44 Fixed to use the live paths. **(committed in this batch.)**

464. **`drizzle.config.ts` crashes obscurely without `DATABASE_URL`.**

47 Non-null assertion → undefined → deep parser crash. Now throws a

48 clean, actionable error at the top of the run. **Commit `67f64a3`.**

50### P1 — Real bugs, not yet fixed

525. **`src/routes/legal/` is an orphan subdirectory.** Contains

53 `terms.tsx`, `privacy.tsx`, `acceptable-use.tsx`, `dmca.tsx`. None of

54 them are mounted in `src/app.tsx`. Self-reference `/legal/*` paths

55 internally. Either mount them (and drop `legal.tsx`), or delete them.

56 Currently dead code that wastes reviewer attention.

586. **Duplicate route registration on `/:owner/:repo/dependencies`.**

59 Registered in both `src/routes/deps.tsx:45` and

60 `src/routes/insights.tsx:183`. Hono is first-wins, and `insightRoutes`

61 mounts first (line 372 vs 411 in `app.tsx`), so the `deps.tsx` handler

62 is **unreachable dead code**.

647. **`/explore` 500s with no DB.** No defensive error boundary around

65 the public repo-list query. Means a Neon hiccup takes the public

66 discovery page down hard rather than degrading. Same pattern as

67 `public-stats.ts` already handles; copy that approach.

698. **`/` and `/status` hang indefinitely on DB failure.** The home page

70 handler blocks on `computePublicStats()` (and that file already has a

71 "degraded to zeros" fallback) but the underlying query path can hang

72 when the connection refuses rather than fails. Needs an explicit

73 `Promise.race` against a 3–5s timeout so the page renders cached or

74 zero data instead of timing out at the proxy.

76### P1 — Silent failures across journeys (from critical-path audit)

789. **Post-receive hook silent failures.** `src/hooks/post-receive.ts`

79 fires auto-repair, analysis, health-score updates as fire-and-forget

80 with `.catch(() => {})`. User's push reports green from git but

81 downstream automation may have crashed. No surface on the repo page.

8310. **Import-bulk has no input validation, no size limits.** A 10GB repo

84 can be cloned into RAM with no progress feedback or timeout

85 (`src/routes/import-bulk.tsx`, `src/lib/import-helper.ts`). Git stderr

86 is truncated to 200 bytes, so real errors are illegible.

8811. **`releaseExpiredWaitTimers` `.set is not a function`** at

89 `src/lib/environments.ts:376`. Caught and silently logged in tests;

90 in production this means env-approval wait timers never release.

91 Looks like a stale Drizzle call after a schema change.

93### P2 — Style / pattern issues (lower priority)

9512. **Admin auth is inconsistent.** Some admin routes use `requireAdmin`

96 middleware, others use inline `gate()` helpers that call

97 `isSiteAdmin()`. **All routes audited are properly admin-gated** —

98 the route auditor flagged false positives on `mirrors`,

99 `github-oauth`, `sso`, but all three have explicit `isSiteAdmin()`

100 checks inside the handler. The inconsistency is a maintenance hazard,

101 not a security hole.

102

10313. **Many helpers `return null` instead of returning a `Response` or

104 throwing.** Pattern in `ai-tests.tsx`, `ai-explain.tsx`,

105 `discussions.tsx`, `issues.tsx`, `admin-ops.tsx`. Non-standard

106 semantics that could mask bugs. Not a current crash source.

107

10814. **Silent `.catch(() => {})` patterns.** Several auth flows swallow

109 notification and audit-log failures without logging. Appropriate for

110 best-effort side effects but lack of observability means operational

111 issues are invisible.

112

113## Verdict

114

115The codebase is **substantially more solid than the user's framing

116suggested**. 1995 tests pass, navigation has zero broken links/forms, all

117admin routes are properly authenticated. The user-visible symptoms ("loop",

118"buttons don't work") trace to a single root cause (the SW scope collision)

119which is now fixed.

120

121The genuine P0s are concentrated in two areas:

122- The **AI auto-merge pipeline** (now fixed — review can no longer fake

123 approval through JSON parse failure)

124- **Failure surfaces** — post-receive, imports, and env approvals fail

125 silently rather than reporting to the user

126

127What this is **not**: a beginner codebase. It is an ambitious one with

128~170 routes and significant feature breadth (SSO, OAuth, GraphQL, MCP,

129marketplace, AI review, etc.). The fragility comes from surface area, not

130craftsmanship.

131

132## What's still needed for "usable as a normal website"

133

1341. Deploy `d7ba05d` + the fixes in this batch to prod.

1352. Run a golden-path smoke test manually: register → create repo → `git

136 push` → file an issue → open a PR. Each step that fails gets a fix.

1373. Address P1 items as discovered during the smoke test, in order of

138 user-visible impact.

139

140Visibility sweep (activity drawer, live admin, repo activity rail, run

141pages) is a separate effort and belongs on a fresh branch after the

142golden-path is green.

Modifiedsrc/lib/ai-review.ts+17−6View fileUnified Split

@@ -110,23 +110,34 @@ ${diffText.slice(0, 100000)}

110110 // Extract JSON from response (may be wrapped in markdown code block)

111111 const jsonMatch = text.match(/\{[\s\S]*\}/);

112112 if (!jsonMatch) {

113 // FAIL-CLOSED: parsing failure must NOT count as approval. Previously

114 // this returned approved:true, which silently auto-approved PRs

115 // whenever Claude's output drifted off the JSON spec — a real auto-

116 // merge hazard. Surface the failure in the summary instead.

113117 return {

114 summary: "AI review completed but could not parse structured output.",

118 summary:

119 "AI review failed: model returned output that could not be parsed as JSON. Treating as not-approved; a human reviewer should look at this PR.",

115120 comments: [],

116 approved: true,

121 approved: false,

117122 };

118123 }

119124 const parsed = JSON.parse(jsonMatch[0]);

120125 return {

121126 summary: parsed.summary || "Review complete.",

122127 comments: Array.isArray(parsed.comments) ? parsed.comments : [],

123 approved: parsed.approved !== false,

128 // Require explicit `approved: true` — undefined or any other value

129 // is treated as not-approved. Same fail-closed principle as above.

130 approved: parsed.approved === true,

124131 };

125 } catch {

132 } catch (err) {

133 // FAIL-CLOSED: JSON.parse threw (truncated output, schema-shaped

134 // garbage, etc.). Echo a short error tail so operators have a

135 // breadcrumb in the comment body, but never approve.

136 const tail = err instanceof Error ? err.message.slice(0, 200) : String(err);

126137 return {

127 summary: text.slice(0, 500),

138 summary: `AI review failed to parse model output (${tail}). Treating as not-approved; a human reviewer should look at this PR.`,

128139 comments: [],

129 approved: true,

140 approved: false,

130141 };

131142 }

132143}

Modifiedsrc/routes/auth.tsx+2−2View fileUnified Split

@@ -101,11 +101,11 @@ auth.get("/register", softAuth, (c) => {

101101 />

102102 <span>

103103 I agree to the{" "}

104 <a href="/legal/terms" target="_blank" rel="noopener">

104 <a href="/terms" target="_blank" rel="noopener">

105105 Terms of Service

106106 </a>{" "}

107107 and{" "}

108 <a href="/legal/privacy" target="_blank" rel="noopener">

108 <a href="/privacy" target="_blank" rel="noopener">

109109 Privacy Policy

110110 </a>

111111 .

112112