← Back to Blog

How Do I Audit a Static Analysis Finding Before Opening a Pull Request?

By Caleb Gates

Three checks before any static analysis finding becomes a pull request: quote the cited files in your audit instead of just listing their paths, trace from the user-facing entry point down to the patched file rather than from the patched file outward, and when the throwing function has more than one caller in the codebase, read every caller for pre-check patterns. Skip any one of these and you risk opening a PR that the maintainer closes within 24 hours because the framework or an in-repo wrapper already handles the failure mode you tried to fix.

Quick Answer: Before opening a pull request derived from a static analysis finding, run three checks: (1) quote the actual code from every file your audit cites as evidence, not just the file path, (2) trace from the user-facing entry (form action, HTTP route, event listener) DOWN through every wrapper to the patched file, (3) grep for every call site of the throwing function and read each caller to identify guard patterns. To run the scan that surfaces these findings: npx nark --tsconfig ./tsconfig.json.

The case study: a closed PR to OpenStatus

I shipped a one-file fix to openstatusHQ/openstatus based on a finding from Nark, the TypeScript static analyzer I work on. The PR (#2149) was closed within 24 hours. The maintainer, mxkaske (openstatus co-founder and the file's original author), was right to close it. The fix was redundant and would have actively broken the existing error-toast flow if the patched site had ever been reached.

Here is what Nark found. In apps/status-page/src/lib/auth/providers.ts, NextAuth's Resend provider has a sendVerificationRequest callback that calls queryClient.fetchQuery(trpc.statusPage.validateEmailDomain.queryOptions(...)) without a try-catch. The TanStack Query docs are explicit that fetchQuery throws on rejection. The TRPC procedure validateEmailDomain throws on three conditions: page not found, page not configured for email-domain auth, and invalid email domain. So the unguarded await looked like a real bug.

I drafted a patch that wrapped the call in a try-catch, logged a structured diagnostic on TRPCClientError, and returned undefined to fall through to the existing if (!query) return guard. I added unit tests. The PR body explained the failure mode in detail. The diff was small.

The maintainer's reply:

this is redundant, we already catch the error in the server action. before sending out the email, we call the same function. if it fails, we are returning an error toast to the user. it would break our Error message flow and we wouldn't display the proper message anymore.

He linked to apps/status-page/src/app/(status-page)/[domain]/[locale]/(auth)/login/actions.ts:26. The file was already in my audit's "existing translation sites" list. I had cited it as evidence the codebase has a try-catch convention for TRPCClientError. What I had never done was actually open the file and read what it does.

Why the audit looked thorough but missed the bug

The audit listed seven existing translation sites for TRPCClientError across the openstatus codebase. The list looked like real work. Each entry was file:line with a one-line note about what the file does. The audit's "Decision log" checkbox said "Existing translation sites listed (file:line for each)" and the checkbox was checked.

The checkbox was satisfied. The audit was not.

Citing a file as evidence is not the same as reading it. The cost of opening actions.ts was 30 seconds. The cost of not opening it was a closed PR, an apologetic reply, and a permanent entry in the campaign's "lessons learned" log.

Here is what actions.ts actually does. The server action signInWithResendAction is the function the sign-in form posts to. The relevant region is lines 22 through 50:

const queryClient = getQueryClient();
// NOTE: throws an error if the email domain is not allowed
try {
  await queryClient.fetchQuery(
    trpc.statusPage.validateEmailDomain.queryOptions({
      slug: domain,
      email,
    }),
  );
} catch (error) {
  console.error("[SignIn] Email validation failed", error);
  if (error instanceof TRPCClientError) {
    return { success: false, error: error.message };
  }
  // ...
}

await signIn("resend", { email, redirectTo });

The server action calls validateEmailDomain DIRECTLY, in a try-catch, and bails with the error message on failure. The form renders that as a toast. Only if the pre-check passes does the action invoke signIn("resend", ...), which is what eventually invokes sendVerificationRequest inside providers.ts. By the time my patched code runs, every reachable failure mode has already been caught upstream and surfaced to the user.

The only scenario where my catch could fire is a race condition: the page is deleted, the accessType is changed, or the domain is removed from the allowlist BETWEEN the pre-check at line 22 and the email-send call. That window is rare. And in that rare window, my fix actively breaks the existing toast: it silently aborts and redirects the user to /verify-request ("check your email") when no email was sent.

Three audit checks every static analysis finding needs before it becomes a PR

These are the structural changes that would have caught the miss before submission. They are not "be more careful." They are mechanical rules that make the failure visible.

Check 1: Quote the cited file, do not just cite the path

When your audit lists a file as evidence of a convention or a precedent, paste 5 to 15 lines from that file's relevant region into the audit output. If you cannot quote it, you have not read it.

Bad (what my audit did):

Existing TRPCClientError translation sites:
- src/app/(status-page)/.../login/actions.ts:35
- src/components/forms/form-email.tsx:51
- src/components/forms/form-password.tsx:50
- (and four more)

Good (what the audit should have done):

Existing TRPCClientError translation sites:

- src/app/(status-page)/.../login/actions.ts:35
  Region: lines 22-40

      try {
        await queryClient.fetchQuery(
          trpc.statusPage.validateEmailDomain.queryOptions({ slug: domain, email }),
        );
      } catch (error) {
        console.error("[SignIn] Email validation failed", error);
        if (error instanceof TRPCClientError) {
          return { success: false, error: error.message };
        }
      }

  NOTE: This is a PRE-CHECK that bails with a toast. It runs BEFORE signIn().
        The patched file (providers.ts) is invoked via signIn(), which means
        the pre-check has already filtered the failure cases by the time
        providers.ts runs. The patched site is only reachable for race conditions.

That second version is impossible to write without actually opening the file. The longer audit output is the right trade-off. A short audit that cites everything but reads nothing is a short audit that misses real wrappers.

Check 2: Trace from user-facing entry to the patched file, not from the patched file outward

The natural direction when auditing a static finding is to start at the violation site and ask "what catches this throw?" That direction follows the call stack outward through the framework's outer wrapper and stops there. It misses any in-repo wrapper that sits above the framework.

The correct direction is to start at the USER-FACING ENTRY POINT and walk DOWN. For a Next.js app with NextAuth, the user-facing entry is the form action.

Here is the trace I should have done before opening PR #2149:

1. <form action={signInWithResendAction}>
   Location: apps/status-page/src/app/.../login/page.tsx

2. signInWithResendAction (server action)
   Location: apps/status-page/src/app/.../login/actions.ts:11

3. Pre-check inside server action:
   await queryClient.fetchQuery(trpc.statusPage.validateEmailDomain...)
   Location: actions.ts:22-32 (wrapped in try-catch)
   On throw: catches TRPCClientError, returns { success: false, error: error.message }
   Result: toast with the actual error message

4. If pre-check passes:
   await signIn("resend", { email, redirectTo })
   Location: actions.ts:49

5. NextAuth Resend provider's sendVerificationRequest callback
   Location: apps/status-page/src/lib/auth/providers.ts (PATCHED FILE)
   Contains: another call to validateEmailDomain (the throwing call I patched)

Step 3 is the show-stopper. The audit needed to reach step 3 before deciding the patched file at step 5 was worth touching. Tracing outward from step 5 only reaches NextAuth's Auth() wrap in @auth/core and stops there. The server action wrapping the entire signIn() call is invisible from that direction.

The general rule: when the file you are about to patch is a framework callback (NextAuth provider, NestJS exception filter, Express middleware, Fastify hook, SvelteKit error handler, Apollo formatter, tRPC error handler), trace from the user-facing entry that triggers the callback BEFORE writing the diff. Server actions, route handlers, and request-level wrappers in the application sit ABOVE the framework callback and frequently handle the failure modes the callback's static analysis flagged.

Check 3: When the throwing function has more than one caller, read every caller

Static analyzers flag a specific call site. They cannot easily see whether the same function is called from another site in a guard pattern. The audit has to do that.

Concrete procedure. For any fix that wraps a call to function foo():

# Find every call site of foo() in the application code
grep -rn "\.foo\b\|\bfoo(" <repo>/src --include="*.ts" --include="*.tsx" | grep -v ".test."

For each result, document:

  • File and line
  • The enclosing function and its role (server action, route handler, framework callback, internal helper)
  • Whether the call is wrapped in try-catch and what the catch does (bail with error, log and rethrow, swallow)
  • Whether this caller runs BEFORE the patched call site in any reachable user flow

If any caller (a) wraps the call in try-catch, (b) bails on failure, and (c) runs before the patched call site in a user-facing flow, then the patched call site is reachable only for race-condition states. In that case the patch's catch is redundant in the common path and may actively break the guard's error-surfacing behavior in the rare path.

For PR #2149, the grep would have returned both actions.ts:26 (the pre-check) and providers.ts:21 (the patched site I was about to wrap). Reading the actions.ts context would have revealed the pre-check + bail pattern. The audit could have stopped there and written a PREP-BLOCKED note instead of producing a doomed PR.

What this means for AI-suggested code changes

Tools like Claude, Copilot, Cursor, and Nark surface candidate fixes. The candidate finding can be correct in isolation and wrong in context. The audit step is what connects them.

Three failure modes in the audit are easy to fall into when an AI suggests a fix:

  1. Cite-without-read. The agent (or you) lists files as evidence without opening them. The audit checklist looks satisfied. The first reviewer who actually reads the cited file closes the PR.
  2. File-out tracing. The agent traces from the violation site outward, finds the framework wrapper, calls it a day. The in-repo wrapper above the framework is invisible from that direction.
  3. Single-caller assumption. The agent treats the violation as if it is the only call site of the throwing function. When the function is called from a guard elsewhere, the violation is often redundant.

These are the same three failure modes my audit hit on PR #2149. None of them require domain expertise to catch. They require mechanical follow-through: quote the file, trace from the user, grep for callers.

How Nark fits

Nark is a TypeScript static analyzer that scans your code against Nark Profiles for popular npm packages. A Nark Profile defines the runtime behaviors of a package (which functions throw, which timeouts are recoverable, which methods need cleanup) and Nark checks whether your code handles those behaviors correctly.

The scanner found the unguarded fetchQuery in openstatus correctly. The Profile's claim was true: fetchQuery throws and the call site has no try-catch. What the scanner cannot do is reason across function boundaries about whether an upstream wrapper already handles the failure. That is the audit's job. Nark gives you the candidate. The three checks above turn the candidate into a PR worth opening, or a finding worth ignoring.

npx nark --tsconfig ./tsconfig.json

Nark covers 165+ packages with curated Profiles. If you run it and want to act on the findings, run the three checks above before opening any PR derived from a finding in a framework-integration file.

Frequently asked questions

Does every static analysis finding need a PR?

No. Many findings are false positives in context. The most common reason: an upstream wrapper (server action, route handler, framework callback's outer catch) already handles the failure mode. Run the three checks above before deciding a finding is worth a PR.

What is a "guard pattern" in this context?

A guard pattern is when the same throwing function is called from a wrapper that bails on failure BEFORE the patched call site runs. The classic example is a Next.js server action that pre-validates input and returns an error toast on failure, then invokes a framework callback (like NextAuth's signIn()) that internally calls the same validation. The patched call site inside the framework callback is only reachable for race conditions where state changes between the pre-check and the framework call.

How do I know if the patched file is a framework callback?

Look at how the function is invoked. If it is exported and passed as a callback to a third-party library (Resend({ async sendVerificationRequest(...) {...} }), app.use(errorMiddleware), fastify.setErrorHandler(...), @Catch(SomeError) class decorator), it is a framework callback. Framework callbacks have a high rate of upstream-already-handled findings. Always trace from a user-facing entry before patching one.

What should I do when a maintainer closes my PR because of a miss like this?

Withdraw cleanly. Acknowledge the specific function or line they cited (proves you read their reply and the file they referenced). Briefly state the user-facing harm of the patch you proposed. Mention edge cases at most once and not as a counter-argument. Close the PR yourself rather than making the maintainer do it. Do not over-apologize. The clean exit is itself an asset for any future contribution to that maintainer's repos.

Is this only a Next.js or NextAuth problem?

No. The pattern recurs anywhere a framework callback sits below an application wrapper that handles the same failure modes. Examples: NestJS exception filters below controllers that already wrap service calls in try-catch, Express error middleware below routes that already validate input, Apollo formatError below resolvers that already classify errors, tRPC errorFormatter below procedures that throw typed errors. The three audit checks apply to every one of these stacks.

Try it now

npx nark --tsconfig ./tsconfig.json

Nark scans 165+ npm packages for unhandled errors, missing timeouts, and resource cleanup violations. Each finding is a candidate, not a verdict. Run the three audit checks before any finding becomes a PR:

  1. Quote every cited file. If you cannot quote it, you have not read it.
  2. Trace from user-facing entry DOWN to the patched file. Stop the audit at the first upstream wrapper that handles the failure mode.
  3. Grep for every caller of the throwing function. Read each caller for guard patterns.

The full PR I closed is at github.com/openstatusHQ/openstatus/pull/2149. The maintainer's reply, the diff, and the surrounding discussion are public. The lesson generalizes.