Skip to content

feat(auth): add DC_NO_BROWSER env var and IS_AI_AGENT to browser-auth gate#432

Merged
bensonwong merged 4 commits intomainfrom
feat/browser-auth-control
Apr 16, 2026
Merged

feat(auth): add DC_NO_BROWSER env var and IS_AI_AGENT to browser-auth gate#432
bensonwong merged 4 commits intomainfrom
feat/browser-auth-control

Conversation

@bensonwong
Copy link
Copy Markdown
Collaborator

Summary

  • Add DC_NO_BROWSER env var: when set, openBrowser() returns early without calling execFile, suppressing browser launch on all platforms
  • Extract canStartBrowserAuth(argv) helper that consolidates the duplicated TTY/env detection logic from requireAuth() and login() into one place
  • Wire IS_AI_AGENT into canStartBrowserAuth() so AI agent environments (Claude Code, Cursor, Codex, etc.) reliably skip browser OAuth without needing DC_NON_INTERACTIVE
  • --browser flag remains a hard opt-in override — it bypasses canStartBrowserAuth() but DC_NO_BROWSER still silences the actual execFile call (useful for headless testing)
  • Improve non-interactive error message: "Non-interactive environment detected (no TTY)" → "Browser authentication is disabled or unavailable in this environment"
  • Trim verbose login success message (removed "The DeepCitation CLI will use this key automatically.")
  • Fix tsconfig.jest.json ignoreDeprecations value: "6.0""5.0" (correct value for suppressing moduleResolution: node10 deprecation in TS 5.x)

Test plan

  • openBrowser unit test: DC_NO_BROWSER=1 prevents execFile call (all platforms)
  • Unit test: DC_NO_BROWSER=1 overrides MSYSTEM in canStartBrowserAuth — exits with non-interactive message
  • Scenario test: CLAUDE_CODE=1 (IS_AI_AGENT=true) triggers non-interactive path in login
  • Scenario test: updated error message assertion (cliAuthScenarios.test.ts)
  • All 163 tests pass (npm test)

Benson and others added 2 commits April 15, 2026 19:48
- Extract canStartBrowserAuth() helper consolidating TTY/env checks
- Respect DC_NO_BROWSER in openBrowser() and browser-auth gate
- Wire IS_AI_AGENT into browser-auth decision
- Improve non-interactive error message clarity
- Fix tsconfig.jest.json ignoreDeprecations value (6.0 → 5.0)

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- Unit test: DC_NO_BROWSER=1 overrides MSYSTEM in canStartBrowserAuth
- Scenario test: CLAUDE_CODE=1 (IS_AI_AGENT) triggers non-interactive path
- Add clarifying comment on --browser vs DC_NO_BROWSER layering

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@vercel
Copy link
Copy Markdown

vercel bot commented Apr 16, 2026

The latest updates on your projects. Learn more about Vercel for GitHub.

4 Skipped Deployments
Project Deployment Actions Updated (UTC)
agui-chat-deepcitation Ignored Ignored Preview Apr 16, 2026 1:51pm
deepcitation-langchain-rag-chat Ignored Ignored Preview Apr 16, 2026 1:51pm
mastra-rag-deepcitation Ignored Ignored Preview Apr 16, 2026 1:51pm
nextjs-ai-sdk-deepcitation Ignored Ignored Preview Apr 16, 2026 1:51pm

@claude
Copy link
Copy Markdown

claude bot commented Apr 16, 2026

Code Review

Overall this is a clean, well-scoped PR. The canStartBrowserAuth() extraction is the right call — the two duplicated isInteractive blocks were a latent divergence risk. A few observations below, ranging from a genuine issue to minor nits.


Bug: tsconfig.jest.json ignoreDeprecations fix

"5.0" is correct. The value must match the TypeScript version in which the deprecation was introduced (TS 5.x introduced moduleResolution: node10 deprecation), not some future version. "6.0" would suppress nothing because TS 6 doesn't exist. Good catch.


Concern: DC_NO_BROWSER has two subtly different behaviors depending on --browser

In canStartBrowserAuth():

if (argv.includes("--browser")) return true;   // hard opt-in
if (... || process.env.DC_NO_BROWSER ...) return false;

This creates a dual-mode semantic for DC_NO_BROWSER:

  • Without --browser: short-circuits the entire OAuth flow → non-interactive error message fires.
  • With --browser: the OAuth flow does start (callback server binds), but openBrowser() is silenced by the guard in auth.ts.

The comment documents this clearly, but the behavior is non-obvious enough that it could surprise someone who sets DC_NO_BROWSER in a CI environment that also generates --browser argv somehow. Consider whether DC_NO_BROWSER warrants a name that reflects its broader "suppress auth flow" role when used standalone, or whether the dual-mode should be documented in the CLI help text.


Issue: requireAuth() silently ignores --browser

requireAuth() calls canStartBrowserAuth() with no argv (defaults to []), so the --browser opt-in never applies to it:

// commands.ts:294
if (!canStartBrowserAuth()) {   // argv defaults to [], --browser never seen

For the login command this is fine — it calls canStartBrowserAuth(argv) directly. But requireAuth() is called from verify, search, and other commands. If a user explicitly passes --browser to one of those commands intending to force interactive OAuth, it will be silently ignored. Whether that's intentional should be confirmed — if so, a comment on the requireAuth() call site would prevent future confusion.


Test nit: process.exit mock dependency is implicit

The new test in cliCommands.test.ts:

try {
  await login([], TEST_BASE_URL);
} catch (e) {
  expect(String(e)).toContain("process.exit(1)");
}

This works only because process.exit is mocked to throw elsewhere in the test suite. The mock isn't visible in the diff and isn't set up locally in the new describe block. If the global mock is ever removed or scoped differently, this test will hang or fail opaquely. Prefer:

await expect(login([], TEST_BASE_URL)).rejects.toThrow("process.exit(1)");

or make the process.exit mock explicit in the test setup.


Missing test: --browser bypasses IS_AI_AGENT

The PR wires IS_AI_AGENT into canStartBrowserAuth() and documents --browser as the override, but there's no unit test for the bypass path:

// Not tested:
canStartBrowserAuth(["--browser"]) // should return true even when IS_AI_AGENT=true

A single unit test for canStartBrowserAuth in cliCommands.test.ts covering the three branches (IS_AI_AGENT + no flag, IS_AI_AGENT + --browser, DC_NO_BROWSER + --browser) would lock in the contract.


Minor: IS_AI_AGENT module-level constant is correctly handled

The mock in cliCommands.test.ts (IS_AI_AGENT: false) works because Jest resolves the module mock before the import runs. Tests needing IS_AI_AGENT=true correctly use subprocess-based tests (cliAuthScenarios.test.ts) with CLAUDE_CODE: "1" in the env — this is the right approach and avoids the static constant problem.


Summary: The canStartBrowserAuth() refactor, IS_AI_AGENT wiring, and DC_NO_BROWSER guard are sound. The main actionable items are: (1) clarify/guard the requireAuth() + --browser gap, (2) make the process.exit mock dependency explicit in the new test, and (3) add a unit test for the --browser bypass of IS_AI_AGENT. The tsconfig.jest.json fix is correct and should go in as-is.

@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Apr 16, 2026

✅ Playwright Test Report

Status: Tests passed

📊 Download Report & Snapshots (see Artifacts section)

What's in the Visual Snapshots

The gallery includes visual snapshots for:

  • 🖥️ Desktop showcase (all variants × all states)
  • 📱 Mobile showcase (iPhone SE viewport)
  • 📟 Tablet showcase (iPad viewport)
  • 🔍 Popover states (verified, partial, not found)
  • 🔗 URL citation variants

Run ID: 24514073946

Benson and others added 2 commits April 16, 2026 07:38
TypeScript 6.0 requires ignoreDeprecations "6.0" (not "5.0") to silence
TS5107 for the node10 moduleResolution used by ts-jest/CommonJS.

Also collapses a multi-line captureOutput call to satisfy biome format.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@bensonwong bensonwong merged commit 88523c2 into main Apr 16, 2026
14 checks passed
@bensonwong bensonwong deleted the feat/browser-auth-control branch April 16, 2026 15:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant