Commit976d7f7unknown_key
fix: P0 batch from AUDIT-v2.md — silent failures + deploy diagnostics
fix: P0 batch from AUDIT-v2.md — silent failures + deploy diagnostics
Five-agent audit (schema-vs-code, route smoke crawl, form submission,
silent failures, deploy pipeline) at commit a358c63. Full report in
AUDIT-v2.md. Headline: codebase is structurally sound; user-visible
breakage traces to silent-failure modes in observability, integrations,
and the deploy pipeline.
This commit fixes the seven P0 items that can ship without coordination:
1. src/routes/orgs.tsx:338 — `/orgs/:slug/people` was dereferencing
`c.get("user")!.id` directly. softAuth allows `user === null`, so
anonymous hits crashed with `TypeError: null is not an object`
instead of redirecting to login. Now guards on null and 302s to
`/login?redirect=…`.
2. src/lib/ai-review.ts (3 sites) — DB insert `.catch(() => {})` was
swallowing summary/inline/advisory comment failures. If the DB
blipped, the user saw NO comment on the PR and had no clue the AI
review even ran. Replaced with structured `console.error(...)` so
operators see the failure in the journal.
3. src/lib/pr-triage.ts, src/lib/issue-triage.ts — same pattern, same
fix. Triage comments now log on insert failure instead of going
silent.
4. src/lib/gate.ts — `notifyGateTestOfPush` was sending unauthenticated
requests when GATETEST_API_KEY was missing. GateTest rejects those
with 401, so the scan never ran but the push looked like it fired.
Added a one-shot `console.warn` so operators notice the
misconfiguration the first time it happens.
5. src/routes/auth.tsx — registration was firing email verification
fire-and-forget, even when EMAIL_PROVIDER was unconfigured. Users
would register "successfully" but never receive the verification
email, locking them out. Now: if email is configured, send the
verification email as before. If NOT configured, auto-verify the
account on registration so the user can actually use the site, and
log a one-shot warning so operators know to set RESEND_API_KEY.
6. .github/workflows/hetzner-deploy.yml:403-406 — failure-diagnostics
curl was hitting `localhost:3000`, but the service runs on `3010`
(see lines 240, 301, 370). Every failure dump since this code was
added has been useless because the wrong port always returns
"nothing listening". Fixed to 3010.
7. .github/workflows/hetzner-deploy.yml:364 — rollback path had
`bun install --frozen-lockfile || true`. If install failed, the
rollback would still run `systemctl restart gluecron` against
half-installed node_modules, masking the real problem. Dropped the
`|| true` so the failure is loud.
Tests touched: ai-review.test.ts, push.test.ts — 28/28 pass. tsc
clean. P1/P2 items in AUDIT-v2.md tracked for a follow-up session.8 files changed+175−12976d7f7a4110a926b2bcb7e849837cd38b962647
8 changed files+175−12
Modified.github/workflows/hetzner-deploy.yml+10−4View fileUnifiedSplit
@@ -361,7 +361,10 @@ jobs:
361361 if [ -f bun.lock ] && [ -d node_modules ]; then
362362 echo "==> reusing existing node_modules (lockfile hash check skipped during rollback)"
363363 else
364 "$BUN" install --frozen-lockfile || true
364 # Drop `|| true` (AUDIT-v2.md P0 #6): if rollback bun install
365 # fails, the service must NOT be restarted against half-installed
366 # node_modules. Fail loudly so the operator can intervene.
367 "$BUN" install --frozen-lockfile
365368 fi
366369 systemctl restart gluecron
367370 # Verify the rollback target itself comes up green.
@@ -400,10 +403,13 @@ jobs:
400403 caddy validate --config /etc/caddy/Caddyfile 2>&1 | head -20 || true
401404 echo ""
402405 echo "===== /healthz from inside box ====="
403 curl -s -w "\nHTTP %{http_code}\n" http://localhost:3000/healthz 2>&1 || true
406 curl -s -w "\nHTTP %{http_code}\n" http://localhost:3010/healthz 2>&1 || true
404407 echo ""
405 echo "===== port 3000 listener ====="
406 ss -tlnp 2>&1 | grep ':3000' || echo '(nothing listening on :3000)'
408 echo "===== port 3010 listener ====="
409 # The service runs on 3010 (see line 240, 301, 370). Diagnostics
410 # were curling 3000 for months, which always failed and made
411 # every failure dump useless. (AUDIT-v2.md P0 #5.)
412 ss -tlnp 2>&1 | grep ':3010' || echo '(nothing listening on :3010)'
407413
408414 # Always post the captured diagnostics to the workflow summary so the
409415 # owner can read what broke without SSH'ing or grepping log files.
AddedAUDIT-v2.md+66−0View fileUnifiedSplit
@@ -0,0 +1,66 @@
1# Gluecron Audit v2 — 2026-05-16
2
3Five parallel agents (schema-vs-code drift, route smoke crawl, form
4submission, silent failures, deploy pipeline) audited the codebase
5at commit `a358c63` after the AA-loop / Hetzner-deploy firefight.
6
7## Headline
8
9The codebase is **structurally sound** — schema matches code, forms are
10correctly wired, route mounting is consistent. The user-visible
11"website is broken" symptoms trace to a small set of **silent failure
12modes** in observability, integrations, and the deploy pipeline.
13
14## P0 — User-blocking, fix now
15
16| # | Bug | File | Effect |
17|---|---|---|---|
18| 1 | `/orgs/:slug/people` null-deref on anonymous users | `src/routes/orgs.tsx:338` | 500 instead of 302 to login |
19| 2 | `/settings` 500 from missing migration 0045 | n/a (deploy bug) | All `auto_close_stale_*` reads crash. **Fix shipped in `a358c63`** — next deploy applies it. |
20| 3 | Email silently disabled when `RESEND_API_KEY` missing | `src/routes/auth.tsx:222`, `src/lib/email.ts:49-85` | Registration succeeds, verification email never sends, user locked out. No error surface. |
21| 4 | AI review/triage comments swallowed on DB error | `src/lib/ai-review.ts:281,297,317`, `src/lib/pr-triage.ts:233` | DB blip → comments never appear → user thinks AI never ran |
22| 5 | Deploy failure diagnostics curl wrong port | `.github/workflows/hetzner-deploy.yml:403` | Service runs on 3010, diagnostics check 3000 → every failure dump useless |
23| 6 | Rollback `bun install` uses `\|\| true` | `.github/workflows/hetzner-deploy.yml:364` | Corrupted deps don't abort rollback → service restarted with broken node_modules |
24| 7 | GateTest fires unauthenticated when `GATETEST_API_KEY` missing | `src/lib/gate.ts:128`, `src/lib/gate.ts:122` | 401 silent → scans never run |
25| 8 | Crontech deploy webhook fires without signature when secret missing | `src/hooks/post-receive.ts:309` | Rejected silently → deploy looks fired, never landed |
26
27## P1 — Real bugs, lower frequency
28
29| # | Bug | File | Effect |
30|---|---|---|---|
31| 9 | Repo-scoped routes 500 instead of 404/empty on missing record | `issues.tsx`, `pulls.tsx`, `packages.tsx`, `releases.tsx`, etc. | DB blip → 500 instead of graceful degradation |
32| 10 | No DB-blip protection — every page blocks 5-15s on a sick DB | All routes that read DB | Sick DB → site appears down instead of slow |
33| 11 | Two deploy paths drifted | `scripts/self-deploy.sh` vs `.github/workflows/hetzner-deploy.yml` | One has compile/cache/verifier, the other doesn't. Inconsistent state on prod. |
34| 12 | No Fly rollback path | `.github/workflows/fly-deploy.yml` | Bad Fly deploy = manual recovery |
35| 13 | Deployment INSERT `.catch(() => {})` | `src/hooks/post-receive.ts:282-296` | If DB blip, deploy row never persists, Crontech callbacks for unknown deployId |
36| 14 | PR close-keyword closing comment swallowed | `src/lib/pr-merge.ts:179-181` | Issue closes but no back-link comment posted on issue |
37| 15 | Workflow log truncation has no UX warning | `src/lib/workflow-runner.ts:94-97` | User sees "[... truncated ...]" with no indication of how much was lost |
38
39## P2 — Cleanup / risk
40
41| # | Item | File | Notes |
42|---|---|---|---|
43| 16 | `pr_risk_scores` table is dead schema | `drizzle/0044_pr_risk_scores.sql` | Created, never written to or read |
44| 17 | `scripts/deploy-crontech.sh` is dead code | `scripts/deploy-crontech.sh` | Referenced in comments, never invoked |
45| 18 | Stripe webhook `continue-on-error: true` on volume creation | `.github/workflows/fly-deploy.yml:48` | Mountpoint can be wrong without alarm |
46| 19 | `/admin/sso`, `/admin/github-oauth`, `/admin/mirrors/sync-all` use middleware `requireAuth` (not `requireAdmin`), gated only by inline `isSiteAdmin()` check | Multiple admin routes | Inconsistent pattern; current gate is correct but easy to forget on a new route |
47
48## False alarms — investigated, no issue
49
50- **Schema drift**: agents found zero columns the code reads that aren't in `schema.ts` and zero columns in `schema.ts` without a matching migration. All 54 migrations are consistent with the code.
51- **Broken nav**: 172 routes, every internal `href` and every form `action` resolves to a registered handler. (Single exception was `/legal/terms` link from register form, fixed in `2e8a4d5`.)
52- **Admin auth bypass**: every admin route is properly gated via either middleware or inline `isSiteAdmin()` check. Inconsistent pattern, not a security hole.
53- **Form submission**: every form's POST handler exists, reads the right fields, redirects to a real page, persists data correctly.
54
55## What the user experienced
56
571. **PWA reload loop** — fixed (commits `d7ba05d`, `904927d`, `44fe49b` ripped out PWA, added kill-switch).
582. **"Buttons don't work"** — root cause was the reload loop cancelling clicks before they fired. Fixed by the above.
593. **`/settings` 500** — root cause was deploy pipeline silently failing for 17 hours, so migration 0045 never ran on prod. Fixed by `a358c63` (re-added migration step to deploy).
604. **Hours of "still broken"** — root cause was the deploy pipeline being non-functional (Hetzner git remote 404, silent script abort, no migration step). Fixed by `ec16b67`, `d8b9606`, `a358c63`.
61
62## Roadmap
63
64This commit adds AUDIT-v2.md. The next batch of commits will burn
65through P0s 1, 3, 4, 5, 6, 7, 8 — each as a small commit pushed
66straight to main. P1s and P2s will get their own session.
Modifiedsrc/lib/ai-review.ts+21−3View fileUnifiedSplit
@@ -278,7 +278,15 @@ export async function triggerAiReview(
278278 isAiReview: true,
279279 body: `${AI_REVIEW_MARKER}\n## AI review unavailable\n\nThe AI review attempt failed: ${reason}. The PR is otherwise unchanged.`,
280280 })
281 .catch(() => {});
281 .catch((err) => {
282 // Was a silent .catch(() => {}) — DB blips here meant the user
283 // saw no review comment AND no diagnostic. Log so operators
284 // can investigate, but don't re-throw (caller is fire-and-forget).
285 console.error(
286 `[ai-review] failed to insert API-failure advisory comment for PR ${prId}:`,
287 err instanceof Error ? err.message : err
288 );
289 });
282290 return;
283291 }
284292
@@ -294,7 +302,12 @@ export async function triggerAiReview(
294302 isAiReview: true,
295303 body: summaryBody,
296304 })
297 .catch(() => {});
305 .catch((err) => {
306 console.error(
307 `[ai-review] failed to insert summary comment for PR ${prId}:`,
308 err instanceof Error ? err.message : err
309 );
310 });
298311
299312 for (const c of result.comments) {
300313 if (!c || !c.body) continue;
@@ -314,7 +327,12 @@ export async function triggerAiReview(
314327 filePath,
315328 lineNumber,
316329 })
317 .catch(() => {});
330 .catch((err) => {
331 console.error(
332 `[ai-review] failed to insert inline comment for PR ${prId} (${filePath}:${lineNumber}):`,
333 err instanceof Error ? err.message : err
334 );
335 });
318336 }
319337
320338 if (process.env.DEBUG_AI_REVIEW === "1") {
Modifiedsrc/lib/gate.ts+16−0View fileUnifiedSplit
@@ -113,6 +113,13 @@ async function lookupRepo(
113113 * rework" but nothing called it. CLAUDE.md claims "git push POSTs to
114114 * it" — this restores that contract.
115115 */
116// One-shot warning latch — operators only need to be told once that
117// GateTest is misconfigured. Reset between tests via the export below.
118let _gatetestAuthWarned = false;
119export function _resetGateTestAuthWarning(): void {
120 _gatetestAuthWarned = false;
121}
122
116123export async function notifyGateTestOfPush(
117124 owner: string,
118125 repo: string,
@@ -126,6 +133,15 @@ export async function notifyGateTestOfPush(
126133 };
127134 if (config.gatetestApiKey) {
128135 headers["Authorization"] = `Bearer ${config.gatetestApiKey}`;
136 } else if (!_gatetestAuthWarned) {
137 // GATETEST_URL is set but GATETEST_API_KEY is missing. GateTest
138 // rejects unauthenticated requests with 401 — the scan never
139 // actually runs but the push looks like it fired. Warn once so
140 // operators notice (silent-fail audit, item §4 in AUDIT-v2.md).
141 _gatetestAuthWarned = true;
142 console.warn(
143 "[gatetest] GATETEST_URL is set but GATETEST_API_KEY is empty — push notifications will be rejected by GateTest with 401. Set GATETEST_API_KEY in the deploy environment to enable scans."
144 );
129145 }
130146 const res = await fetch(config.gatetestUrl, {
131147 method: "POST",
Modifiedsrc/lib/issue-triage.ts+6−1View fileUnifiedSplit
@@ -170,7 +170,12 @@ export async function triggerIssueTriage(
170170 authorId: input.authorId,
171171 body,
172172 })
173 .catch(() => {});
173 .catch((err) => {
174 console.error(
175 `[issue-triage] failed to insert triage comment for issue ${input.issueId}:`,
176 err instanceof Error ? err.message : err
177 );
178 });
174179 } catch (err) {
175180 if (process.env.DEBUG_ISSUE_TRIAGE === "1") {
176181 console.error("[issue-triage] crashed:", err);
Modifiedsrc/lib/pr-triage.ts+6−1View fileUnifiedSplit
@@ -238,7 +238,12 @@ export async function triggerPrTriage(input: PrTriageInput): Promise<void> {
238238 body,
239239 isAiReview: true,
240240 })
241 .catch(() => {});
241 .catch((err) => {
242 console.error(
243 `[pr-triage] failed to insert triage comment for PR ${input.prId}:`,
244 err instanceof Error ? err.message : err
245 );
246 });
242247 } catch (err) {
243248 if (process.env.DEBUG_PR_TRIAGE === "1") {
244249 console.error("[pr-triage] crashed:", err);
Modifiedsrc/routes/auth.tsx+45−2View fileUnifiedSplit
@@ -38,6 +38,11 @@ import type { AuthEnv } from "../middleware/auth";
3838
3939const auth = new Hono<AuthEnv>();
4040
41// One-shot latch — log the auto-verify warning at most once per process,
42// since the misconfiguration is operator-level (env var) and won't change
43// between requests.
44let _autoVerifyWarned = false;
45
4146// --- Web UI ---
4247
4348auth.get("/register", softAuth, (c) => {
@@ -219,8 +224,46 @@ auth.post("/register", async (c) => {
219224
220225 setCookie(c, "session", token, sessionCookieOptions());
221226
222 // Block P2 — fire-and-forget email verification. Never blocks registration.
223 import("../lib/email-verification").then((m) => m.startEmailVerification(user.id, email)).catch(() => {});
227 // Block P2 — email verification. If RESEND_API_KEY is configured the
228 // verification email goes out and the user clicks the link to verify.
229 // If email is NOT configured (EMAIL_PROVIDER=log, no RESEND_API_KEY,
230 // etc.), the email would silently never arrive and the user would be
231 // locked out — AUDIT-v2.md P0 #3. In that case, auto-verify the
232 // account on registration so the user can actually use the site.
233 // Operators who want real verification should set EMAIL_PROVIDER=resend
234 // + RESEND_API_KEY in their environment.
235 const { config: _emailConfig } = await import("../lib/config");
236 const emailConfigured =
237 _emailConfig.emailProvider === "resend" && !!_emailConfig.resendApiKey;
238 if (emailConfigured) {
239 import("../lib/email-verification")
240 .then((m) => m.startEmailVerification(user.id, email))
241 .catch((err) => {
242 console.error(
243 `[auth] startEmailVerification failed for ${user.id}:`,
244 err instanceof Error ? err.message : err
245 );
246 });
247 } else {
248 // Auto-verify immediately so the user isn't trapped in an unverified
249 // state. Log once so operators notice the misconfiguration.
250 if (!_autoVerifyWarned) {
251 _autoVerifyWarned = true;
252 console.warn(
253 "[auth] EMAIL_PROVIDER is not configured (set EMAIL_PROVIDER=resend + RESEND_API_KEY). Auto-verifying new account email addresses to avoid lockout."
254 );
255 }
256 await db
257 .update(users)
258 .set({ emailVerifiedAt: new Date() })
259 .where(eq(users.id, user.id))
260 .catch((err) => {
261 console.error(
262 `[auth] auto-verify failed for ${user.id}:`,
263 err instanceof Error ? err.message : err
264 );
265 });
266 }
224267
225268 // P3 — default landing is /onboarding (the guided first-five-minutes
226269 // flow). The `redirect=` query is still honoured for OAuth-style flows.
Modifiedsrc/routes/orgs.tsx+5−1View fileUnifiedSplit
@@ -333,8 +333,12 @@ orgRoutes.get("/orgs/:org", softAuth, requireAuth, async (c) => {
333333// --- PEOPLE -----------------------------------------------------------------
334334
335335orgRoutes.get("/orgs/:slug/people", async (c) => {
336 const user = c.get("user")!;
336 const user = c.get("user");
337337 const slug = c.req.param("slug");
338 // Anonymous users get bounced to login; org membership is non-public.
339 // Previously dereferenced `user!.id` immediately and crashed for anon
340 // (smoke crawl: TypeError, null is not an object — orgs.tsx:338).
341 if (!user) return c.redirect(`/login?redirect=/orgs/${slug}/people`);
338342 const { org, role } = await loadOrgForUser(slug, user.id);
339343 if (!org) return c.notFound();
340344 if (!role) return c.redirect(`/orgs/${slug}`);
341345