fix(auth): guard isOAuthAuth against undefined/null

[jroth1111]

Summary

Fixes a crash in OAuth auth guard when auth state is missing.

Changes

• Make isOAuthAuth null-safe:
  • from: auth.type === "oauth"
  • to: !!auth && auth.type === "oauth"
• Add regression tests to ensure undefined and null inputs return false and do not throw.

Why

isOAuthAuth was dereferencing auth.type directly. When auth is missing/undefined, this can throw TypeError: undefined is not an object (evaluating 'auth.type') before normal fallback handling.

Verification

• npx vitest run src/plugin/auth.test.ts
• npm run typecheck --silent

Closes #474

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

📜 Recent review details

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

Cache: Disabled due to data retention organization setting

Knowledge base: Disabled due to data retention organization setting

📥 Commits

Reviewing files that changed from the base of the PR and between 36a9ab8 and 43a8f0e.

📒 Files selected for processing (1)

• src/plugin/auth.test.ts

🚧 Files skipped from review as they are similar to previous changes (1)

• src/plugin/auth.test.ts

---

Walkthrough

This change makes the isOAuthAuth() type guard null-safe. The function signature is updated to accept AuthDetails | null | undefined and its implementation now checks that auth is present before accessing auth.type. Two regression tests were added to confirm the function does not throw and returns false when called with undefined or null.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~8 minutes

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title accurately summarizes the main change: making isOAuthAuth guard against undefined/null values.
Description check	✅ Passed	The description clearly relates to the changeset, explaining the crash fix, the specific changes made, and reasoning.
Linked Issues check	✅ Passed	The PR fully addresses issue #474: makes isOAuthAuth null-safe by updating the function signature and implementation, and adds regression tests for undefined and null inputs.
Out of Scope Changes check	✅ Passed	All changes directly address issue #474 with no out-of-scope modifications detected.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[greptile-apps]

Greptile Summary

Fixed crash in isOAuthAuth type guard by adding null-safety check (!!auth &&), preventing TypeError when auth state is undefined/null.

Key changes:

• Updated isOAuthAuth to accept AuthDetails | null | undefined and guard against falsy values before property access
• Added regression tests ensuring undefined/null inputs return false without throwing

The fix is minimal, correct, and properly tested. The change aligns with existing usage patterns where isOAuthAuth is already called with potentially null auth values (src/plugin.ts:1347).

Confidence Score: 5/5

• This PR is safe to merge with minimal risk
• The fix is a simple defensive guard that prevents runtime crashes, adds proper test coverage, and doesn't introduce any breaking changes or side effects
• No files require special attention

Important Files Changed

Filename	Overview
src/plugin/auth.ts	Added null/undefined guard to isOAuthAuth type guard, preventing crashes when auth state is missing
src/plugin/auth.test.ts	Added regression tests for null and undefined inputs to ensure type guard handles edge cases safely

_{Last reviewed commit: 43a8f0e}

[coderabbitai]

🧹 Nitpick comments (1)

src/plugin/auth.test.ts (1)

25-33: LGTM — regression tests correctly cover both null-safety requirements from the issue.

Minor nit: each test calls isOAuthAuth twice. Since the function is pure, the not.toThrow() assertion is implicitly covered by the toBe(false) assertion that follows (a thrown error would itself fail the test). You could consolidate to a single assertion per test, but the explicit form is acceptable given the issue explicitly requested verifying "does not throw."

♻️ Optional consolidation (two assertions → one per test)

-  it("returns false for undefined auth without throwing", () => {
-    expect(() => isOAuthAuth(undefined)).not.toThrow();
-    expect(isOAuthAuth(undefined)).toBe(false);
-  });
-
-  it("returns false for null auth without throwing", () => {
-    expect(() => isOAuthAuth(null)).not.toThrow();
-    expect(isOAuthAuth(null)).toBe(false);
-  });
+  it("returns false for undefined auth without throwing", () => {
+    expect(isOAuthAuth(undefined)).toBe(false);
+  });
+
+  it("returns false for null auth without throwing", () => {
+    expect(isOAuthAuth(null)).toBe(false);
+  });

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/plugin/auth.test.ts` around lines 25 - 33, The two tests in auth.test.ts
redundantly call isOAuthAuth twice (once inside expect(...).not.toThrow() and
once for expect(...).toBe(false)); since isOAuthAuth is pure a thrown error
would already fail the test, consolidate each test to a single assertion by
removing the not.toThrow() check and only assert
expect(isOAuthAuth(undefined)).toBe(false) and
expect(isOAuthAuth(null)).toBe(false) respectively, keeping the same test names
and the isOAuthAuth references so the intent remains clear.

📜 Review details

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

Cache: Disabled due to data retention organization setting

Knowledge base: Disabled due to data retention organization setting

📥 Commits

Reviewing files that changed from the base of the PR and between efafbb9 and 36a9ab8.

📒 Files selected for processing (2)

• src/plugin/auth.test.ts
• src/plugin/auth.ts

🧰 Additional context used

🧬 Code graph analysis (2)

src/plugin/auth.test.ts (1)

src/plugin/auth.ts (1)

• isOAuthAuth (5-7)

src/plugin/auth.ts (1)

src/plugin/types.ts (2)

• AuthDetails (21-21)
• OAuthAuthDetails (4-9)

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)

• GitHub Check: Greptile Review

🔇 Additional comments (1)

src/plugin/auth.ts (1)

5-6: LGTM — fix is correct and the type predicate remains sound.

The !!auth && guard properly short-circuits null/undefined before .type access, and auth is OAuthAuthDetails continues to narrow correctly on the true branch. The signature widening is fully backward-compatible with all existing callers.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@src/plugin/auth.test.ts`:
- Around line 25-33: The two tests in auth.test.ts redundantly call isOAuthAuth
twice (once inside expect(...).not.toThrow() and once for
expect(...).toBe(false)); since isOAuthAuth is pure a thrown error would already
fail the test, consolidate each test to a single assertion by removing the
not.toThrow() check and only assert expect(isOAuthAuth(undefined)).toBe(false)
and expect(isOAuthAuth(null)).toBe(false) respectively, keeping the same test
names and the isOAuthAuth references so the intent remains clear.

[jroth1111]

Correction to prior comment (formatting issue):

Addressed valid review feedback in commit 43a8f0e.

• Simplified the null/undefined tests in src/plugin/auth.test.ts to single assertions.
• Preserved behavior and intent.

Verified with:
vitest run src/plugin/auth.test.ts

[NoeFabris]

Please stop opening AI slop issues and PRs @jroth1111