Server initialization logic in `hooks.server.ts` was wrapped in a `building` flag, causing it to behave differently during build vs runtime. by Suvam-paul145 · Pull Request #416 · GetBindu/Bindu

Suvam-paul145 · 2026-03-27T18:23:19Z

##Summery

Why it matters: This created unnecessary complexity and risk of skipping important initialization logic in certain environments.
What changed: Removed the if (!building) condition so initialization code always runs on server start, aligning with intended behavior.
What did NOT change (scope boundary): No changes to the actual initialization logic or server functionality.

Change Type (select all that apply)

Scope (select all touched areas)

Linked Issue/PR

Closes [Feature]: Move Server Hook Code Out of Build-Time Path #347
Related #

User-Visible / Behavior Changes

None

Security Impact (required)

New permissions/capabilities? (No)
Secrets/credentials handling changed? (No)
New/changed network calls? (No)
Database schema/migration changes? (No)
Authentication/authorization changes? (No)
If any Yes, explain risk + mitigation: None

Verification

Environment

OS: Not specified
Python version: N/A
Storage backend: Not affected
Scheduler backend: Not affected

Steps to Test

Start the server.
Verify initialization logic runs without relying on building flag.
Confirm no regressions in server startup behavior.

Expected Behavior

Initialization logic runs consistently on server start.

Actual Behavior

Matches expected behavior.

Evidence (attach at least one)

Failing test before + passing after
Test output / logs
Screenshot / recording
Performance metrics (if relevant)

Human Verification (required)

What you personally verified (not just CI):

Verified scenarios: Server startup behavior after removing building flag.
Edge cases checked: Build vs runtime execution consistency.
What you did NOT verify: Behavior in all deployment environments.

Compatibility / Migration

Backward compatible? (Yes)
Config/env changes? (No)
Database migration needed? (No)
If yes, exact upgrade steps: N/A

Failure Recovery (if this breaks)

How to disable/revert this change quickly: Revert this commit.
Files/config to restore: frontend/src/hooks.server.ts
Known bad symptoms reviewers should watch for: Missing initialization, duplicate execution, or unexpected startup behavior.

Risks and Mitigations

Risk: Initialization code may run in unintended contexts.
- Mitigation: Ensured logic is safe to run on every server start.

Checklist

Tests pass (uv run pytest)
Pre-commit hooks pass (uv run pre-commit run --all-files)
Documentation updated (if needed)
Security impact assessed
Human verification completed
Backward compatibility considered

…OAuth provider failures. - Enhance getOIDCUserData and triggerOauthFlow functions to handle network and provider errors gracefully. - Update login callback to log and respond to OAuth provider errors with appropriate messages. - Introduce SignInButton component for user sign-in with error handling via query parameters. - Create integration tests for OAuth provider failures to ensure correct error propagation and responses. - Add mongodb-memory-server dependency for testing purposes.

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

…zation Removes the `if (!building)` check that was wrapping initialization code in hooks.server.ts, allowing the initialization code to always run when the server starts. This aligns with the TODO that suggested moving this code to a started server hook.

Copilot

Pull request overview

This PR removes the building-flag guard around server initialization in hooks.server.ts so init logic runs consistently, and also introduces new OAuth provider failure handling (plus associated tests and dependencies).

Changes:

Removed the if (!building) wrapper in frontend/src/hooks.server.ts around server init startup tasks.
Added structured OAuth provider error handling in frontend/src/lib/server/auth.ts and corresponding handling in the login callback route.
Added new Vitest coverage for OAuth provider failure scenarios and updated frontend dev dependencies.

Reviewed changes

Copilot reviewed 7 out of 8 changed files in this pull request and generated 5 comments.

Show a summary per file

File	Description
frontend/src/hooks.server.ts	Removes build-flag gating so server init runs consistently.
frontend/src/lib/server/auth.ts	Adds `OAuthProviderError` + returns 502 JSON responses when the provider is unavailable/misbehaving.
frontend/src/routes/login/callback/+server.ts	Catches typed OAuth provider errors during callback token exchange and returns a 502.
frontend/src/lib/server/auth.spec.ts	Adds unit tests for new OAuth provider failure behavior.
frontend/src/routes/login/callback/auth-provider-failure.spec.ts	Adds integration-style tests for provider unavailability (currently contains a broken mock path).
frontend/src/lib/components/SignInButton.svelte	New component file (currently syntactically incomplete/broken).
frontend/package.json	Adds `mongodb-memory-server` to devDependencies.
frontend/package-lock.json	Lockfile updated to include new dependency and transitive changes.

Files not reviewed (1)

frontend/package-lock.json: Language not supported

Copilot · 2026-03-27T18:27:52Z

frontend/package.json

 		"isomorphic-dompurify": "2.13.0",
 		"js-yaml": "^4.1.0",
 		"minimist": "^1.2.8",
+		"mongodb-memory-server": "^11.0.1",


mongodb-memory-server was added to devDependencies, but it isn’t referenced anywhere in the codebase/tests in this PR. Since it’s a large dependency (and pulls in MongoDB driver + Node >=20.19 constraints), please either add the tests/utilities that require it or remove it to avoid unnecessary install size and CI overhead.

Suggested change

"mongodb-memory-server": "^11.0.1",

Copilot · 2026-03-27T18:27:52Z

frontend/src/hooks.server.ts

+	// Ensure legacy env expected by some libs: map OPENAI_API_KEY -> HF_TOKEN if absent
+	const canonicalToken = config.OPENAI_API_KEY || config.HF_TOKEN;
+	if (canonicalToken) {
+		process.env.HF_TOKEN ??= canonicalToken;
+	}

-		// Warn if legacy-only var is used
-		if (!config.OPENAI_API_KEY && config.HF_TOKEN) {
-			logger.warn(
-				"HF_TOKEN is deprecated in favor of OPENAI_API_KEY. Please migrate to OPENAI_API_KEY."
-			);
-		}
+	// Warn if legacy-only var is used
+	if (!config.OPENAI_API_KEY && config.HF_TOKEN) {
+		logger.warn(
+			"HF_TOKEN is deprecated in favor of OPENAI_API_KEY. Please migrate to OPENAI_API_KEY."
+		);
+	}

-		logger.info("Starting server...");
-		initExitHandler();
+	logger.info("Starting server...");
+	initExitHandler();

-		if (config.METRICS_ENABLED === "true") {
-			MetricsServer.getInstance();
-		}
+	if (config.METRICS_ENABLED === "true") {
+		MetricsServer.getInstance();
+	}

-		checkAndRunMigrations();
-		refreshConversationStats();
+	checkAndRunMigrations();
+	refreshConversationStats();

-		// Init AbortedGenerations refresh process
-		AbortedGenerations.getInstance();
+	// Init AbortedGenerations refresh process
+	AbortedGenerations.getInstance();

-		adminTokenManager.displayToken();
+	adminTokenManager.displayToken();


The PR description/title focus on removing the building flag guard in hooks.server.ts, but this PR also introduces new OAuth provider error handling, new tests, a new (currently broken) Svelte component, and new dependencies. Please update the PR description/scope (and linked issue) or split these changes into separate PRs so reviewers can evaluate each change set independently.

Copilot · 2026-03-27T18:27:53Z

frontend/src/lib/components/SignInButton.svelte

+<!--
+	SignInButton component removed because it was not referenced anywhere in the app.
+	If a dedicated sign-in button is needed in the future, reintroduce it and wire it
+	into the appropriate page or layout so its behavior is exercised.
+-->


SignInButton.svelte is syntactically incomplete: the onMount(() => { ... block is never closed, the <script> tag is never closed, and there is an HTML comment inserted inside the <script> block. This will break svelte-check/TypeScript analysis even if the component isn’t imported. Either finish the component implementation (close blocks/tags and add markup) or remove the file entirely if it’s meant to be deleted.

Suggested change



errorStore.set("An unknown error occurred during sign-in. Please try again.");

}

});

/*

* SignInButton component was previously removed because it was not referenced anywhere

* in the app. If a dedicated sign-in button is needed in the future, it can be wired

* into the appropriate page or layout so its behavior is exercised.

*/

</script>

<button

type="button"

class={classNames}

on:click={() => {

// Navigate to the sign-in route, respecting the app's base path.

window.location.href = `${base}/auth/signin`;

}}

>

{label}

</button>

Copilot · 2026-03-27T18:27:53Z

frontend/src/routes/login/callback/auth-provider-failure.spec.ts

+vi.mock("$lib/server/logger", () => ({
+    logger: { error: vi.fn(), info: vi.fn(), warn: vi.fn() },
+}));
+vi.mock("./adminToken", () => ({


This test attempts to mock ./adminToken, but from this file’s location that resolves to src/routes/login/callback/adminToken (which doesn’t exist) and won’t mock the module that src/lib/server/auth.ts imports. Update the mock to target the actual admin token module used by $lib/server/auth (and ensure the specifier matches the one used in auth.ts), otherwise the suite will fail to load or will use the real admin token implementation.

Suggested change

vi.mock("./adminToken", () => ({

vi.mock("$lib/server/adminToken", () => ({

Copilot · 2026-03-27T18:27:53Z

frontend/src/lib/server/auth.ts

+		return new Response(
+			JSON.stringify({
+				code: errorCode,
+				message:
+					errorCode === "OAUTH_PROVIDER_UNAVAILABLE"
+						? "The sign-in service is temporarily unavailable. Please try again later."
+						: "An error occurred during sign-in. Please try again.",
+			}),
+			{
+				status: 502,
+				headers: { "Content-Type": "application/json" },
+			}
+		);


The new error path returns a JSON 502 response unconditionally. triggerOauthFlow is also used for browser navigations (e.g. from hooks.server.ts auto-login and /login), so this will render raw JSON in the browser instead of redirecting to a user-facing error page or honoring Accept: text/html. Consider content-negotiating (JSON for API/XHR, otherwise redirect back to a login page with an ?error= code or throw a Kit error(502, ...)).

Suvam-paul145 and others added 4 commits February 27, 2026 20:48

Update frontend/src/lib/components/SignInButton.svelte

b032f2b

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

Update frontend/src/lib/components/SignInButton.svelte

e83bb03

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

Copilot AI review requested due to automatic review settings March 27, 2026 18:23

Copilot started reviewing on behalf of Suvam-paul145 March 27, 2026 18:23 View session

Copilot AI reviewed Mar 27, 2026

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Server initialization logic in `hooks.server.ts` was wrapped in a `building` flag, causing it to behave differently during build vs runtime.#416

Server initialization logic in `hooks.server.ts` was wrapped in a `building` flag, causing it to behave differently during build vs runtime.#416
Suvam-paul145 wants to merge 4 commits intoGetBindu:mainfrom
Suvam-paul145:347

Suvam-paul145 commented Mar 27, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Mar 27, 2026

Uh oh!

Copilot AI Mar 27, 2026

Uh oh!

Copilot AI Mar 27, 2026

Uh oh!

Copilot AI Mar 27, 2026

Uh oh!

Copilot AI Mar 27, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

-<!--
-	SignInButton component removed because it was not referenced anywhere in the app.
-	If a dedicated sign-in button is needed in the future, reintroduce it and wire it
-	into the appropriate page or layout so its behavior is exercised.
--->
+			errorStore.set("An unknown error occurred during sign-in. Please try again.");
+		}
+	});
+	/*
+	 * SignInButton component was previously removed because it was not referenced anywhere
+	 * in the app. If a dedicated sign-in button is needed in the future, it can be wired
+	 * into the appropriate page or layout so its behavior is exercised.
+	 */
+</script>
+<button
+	type="button"
+	class={classNames}
+	on:click={() => {
+		// Navigate to the sign-in route, respecting the app's base path.
+		window.location.href = `${base}/auth/signin`;
+	}}
+>
+	{label}
+</button>

	vi.mock("./adminToken", () => ({
	vi.mock("$lib/server/adminToken", () => ({

Conversation

Suvam-paul145 commented Mar 27, 2026

Change Type (select all that apply)

Scope (select all touched areas)

Linked Issue/PR

User-Visible / Behavior Changes

Security Impact (required)

Verification

Environment

Steps to Test

Expected Behavior

Actual Behavior

Evidence (attach at least one)

Human Verification (required)

Compatibility / Migration

Failure Recovery (if this breaks)

Risks and Mitigations

Checklist

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Mar 27, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 27, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 27, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 27, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 27, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants