fix: split DB errors from ErrNotFound in OIDC link handlers

[Copilot]

Three places in handler/oidc.go silently redirected users with user-facing "not found" messages on transient DB errors, masking failures with no log output.

Changes

• handleLinkCallback – FindByID: branch on errors.Is(err, auth.ErrNotFound) — not-found → redirect "User not found"; other errors → slog.ErrorContext + redirect "Link verification failed"
• handleLinkCallback – LinkOIDCSubject: add slog.ErrorContext before "Failed to link" redirect so store failures are observable
• Link – FindByID: decompose err != nil || u.OIDCSubject != nil into three explicit branches — DB error → log + 500; ErrNotFound → 409; already-linked → 409

// Before
user, err := h.Users.FindByID(r.Context(), linkUserID)
if err != nil {
    http.Redirect(w, r, "/?oidc_link_error="+url.QueryEscape("User not found"), http.StatusFound)
    return
}

// After
user, err := h.Users.FindByID(r.Context(), linkUserID)
if err != nil {
    if errors.Is(err, auth.ErrNotFound) {
        http.Redirect(w, r, "/?oidc_link_error="+url.QueryEscape("User not found"), http.StatusFound)
    } else {
        slog.ErrorContext(r.Context(), "failed to look up user during OIDC link", slog.Any("error", err))
        http.Redirect(w, r, "/?oidc_link_error="+url.QueryEscape("Link verification failed"), http.StatusFound)
    }
    return
}

Greptile Summary

This PR correctly separates transient DB errors from ErrNotFound in three OIDC link handler code paths, adding slog.ErrorContext logging for previously silent failures. The logic and test additions are sound, with one minor semantic concern in the Link handler where a not-found user returns the same 409 "cannot link account" as an already-linked user, making the two conditions indistinguishable to API clients.

Confidence Score: 5/5

Safe to merge; all slog calls include context and error handling logic is correct — one P2 semantic nit on the 409 status for ErrNotFound

Only P2 findings present; the core error-splitting logic is correct, logging is properly instrumented with context, and new tests cover all new branches

handler/oidc.go lines 319–320 (409 vs 404 for ErrNotFound in Link handler)

Important Files Changed

Filename	Overview
handler/oidc.go	Correctly splits DB errors from ErrNotFound in handleLinkCallback and Link; minor semantic issue with 409 used for both "not found" and "already linked" in Link handler
handler/oidc_test.go	Adds test coverage for the new DB-error branches in handleLinkCallback and Link; tests are well-structured and cover the intended code paths

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[handleLinkCallback / Link called] --> B[FindByID]
    B --> C{err?}
    C -- "errors.Is(ErrNotFound)" --> D[Redirect: User not found / 409 Conflict]
    C -- "other DB error" --> E[slog.ErrorContext\nRedirect: Link verification failed / 500]
    C -- nil --> F{OIDCSubject set?}
    F -- yes --> G[Redirect: Already linked / 409 Conflict]
    F -- no --> H[Continue OIDC flow]
    H --> I[LinkOIDCSubject]
    I --> J{err?}
    J -- yes --> K[slog.ErrorContext\nRedirect: Failed to link]
    J -- no --> L[Redirect: oidc_linked=true]

Loading

Prompt To Fix All With AI

Fix the following 1 code review issue. Work through them one at a time, proposing concise fixes.

---

### Issue 1 of 1
handler/oidc.go:319-320
**`ErrNotFound` returns same 409 as "already linked"**

When `FindByID` returns `ErrNotFound` in `Link`, the response (`409 "cannot link account"`) is now identical to the already-linked branch at line 327–329. A client that consumes this API has no way to distinguish "this user account no longer exists" from "this account already has an OIDC identity attached". Returning `http.StatusNotFound` (404) here would preserve the correct semantic and keep the two failure modes distinguishable.

```suggestion
		if errors.Is(err, auth.ErrNotFound) {
			writeError(r.Context(), w, http.StatusNotFound, "user not found")
```

_{Reviews (3): Last reviewed commit: "chore: merge origin/main and resolve con..." | Re-trigger Greptile}

[Copilot]

Pull request overview

This PR improves observability and correctness in the OIDC account-linking flow by distinguishing “not found” from transient/unknown store errors, avoiding user-facing “not found” redirects that mask backend failures.

Changes:

• In handleLinkCallback, branch on errors.Is(err, auth.ErrNotFound) for FindByID, and log + redirect a distinct message on non-ErrNotFound errors.
• In handleLinkCallback, log failures from LinkOIDCSubject before redirecting.
• In Link, split the combined err != nil || u.OIDCSubject != nil check into explicit branches, logging and returning 500 on non-ErrNotFound store errors.

Show a summary per file

File	Description
handler/oidc.go	Separates ErrNotFound from DB/store errors in OIDC linking paths and adds error logging for better operational visibility.

Copilot's findings

Tip

Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

• Files reviewed: 1/1 changed files
• Comments generated: 2