[Claude code generated PR] refactor(historian-base): extract createRouteContext helper

[dhr-verma]

Summary

• Extracted the duplicated 4-line router/throttle prologue shared across 8 Historian route modules into a single createRouteContext() helper in routes/utils.ts
• Added IHistorianRouteContext interface for the returned context object
• summaries.ts uses a destructuring alias (tenantThrottleOptions: tenantGeneralThrottleOptions) to preserve its existing local variable name without touching downstream code
• Removed now-unused Router, IThrottleMiddlewareOptions, and Constants imports from the 8 standard route files

Test plan

• pnpm run build:compile passes (root)
• npm run test passes in historian-base — 51 tests passing (6 new unit tests for createRouteContext)
• New unit tests cover: router shape, maxTokenLifetimeSec value, throttle options shape, throttler map wiring, absent-key fallback to 0

Related

Closes #26591

Review notes

• When maxTokenLifetimeSec is absent from config, the helper falls back to 0. In verifyToken this causes all tokens to be rejected (safe-fail). A startup warning could be added in a follow-up if desired.
• The throttleIdPrefix test uses a defensive ternary; a future cleanup could split it into an explicit type assertion + direct call for clearer failure messages.

🤖 Generated with Claude Code

[Copilot]

Pull request overview

This PR extracts a repeated 4-line router/throttle setup block shared across 8 Historian route modules into a single createRouteContext() helper, reducing duplication and centralizing the configuration in routes/utils.ts.

Changes:

• Added createRouteContext() helper and IHistorianRouteContext interface to routes/utils.ts
• Refactored 8 route files (blobs.ts, commits.ts, refs.ts, tags.ts, trees.ts, contents.ts, headers.ts, summaries.ts) to use the new helper
• Added 6 unit tests for createRouteContext in routes.spec.ts

Reviewed changes

Copilot reviewed 12 out of 13 changed files in this pull request and generated 2 comments.

Show a summary per file

File	Description
routes/utils.ts	Adds IHistorianRouteContext interface and createRouteContext() helper; also includes minor formatting fixes
routes/git/blobs.ts	Adopts createRouteContext(); removes duplicate prologue and unused imports
routes/git/commits.ts	Adopts createRouteContext(); removes duplicate prologue and unused imports
routes/git/refs.ts	Adopts createRouteContext(); removes duplicate prologue and unused imports
routes/git/tags.ts	Adopts createRouteContext(); removes duplicate prologue and unused imports
routes/git/trees.ts	Adopts createRouteContext(); removes duplicate prologue and unused imports
routes/repository/contents.ts	Adopts createRouteContext(); removes duplicate prologue and unused imports
routes/repository/headers.ts	Adopts createRouteContext(); removes duplicate prologue and unused imports
routes/summaries.ts	Adopts createRouteContext() with destructuring alias for tenantGeneralThrottleOptions
services/restGitService.ts	Indentation-only formatting fix
runnerFactory.ts	Indentation-only formatting fixes
test/routes.spec.ts	Adds 6 unit tests for createRouteContext()

[Copilot]

The defensive ternary used for throttleIdPrefix means that if throttleIdPrefix is accidentally set to a non-function value, this test will pass with undefined === "myTenant" rather than failing. The test should directly assert that throttleIdPrefix is a function first (e.g., assert.strictEqual(typeof ctx.tenantThrottleOptions.throttleIdPrefix, "function")), and then call it unconditionally. This would produce a clear failure message if the shape changes.

Suggested change

	assert.strictEqual(
	typeof ctx.tenantThrottleOptions.throttleIdPrefix === "function"
	? ctx.tenantThrottleOptions.throttleIdPrefix(mockReq)
	: undefined,
	"myTenant",
	);
	assert.strictEqual(typeof ctx.tenantThrottleOptions.throttleIdPrefix, "function");
	assert.strictEqual(ctx.tenantThrottleOptions.throttleIdPrefix(mockReq), "myTenant");

[Copilot]

The previous code in the individual route files used config.get("maxTokenLifetimeSec") without a fallback, while this helper now silently falls back to 0 when the key is absent. A value of 0 for maxTokenLifetimeSec causes all tokens to be immediately rejected in verifyToken, which is a silent behavioral change for any deployment where this config key is accidentally omitted. The PR description acknowledges this and notes a startup warning could be added as a follow-up, but returning 0 without any warning logged here makes the failure mode hard to diagnose. Consider at least logging a warning (e.g., via Lumberjack.warning) when config.get("maxTokenLifetimeSec") returns null/undefined before falling back to 0.

Suggested change

	const maxTokenLifetimeSec = (config.get("maxTokenLifetimeSec") as number | null) ?? 0;
	const rawMaxTokenLifetimeSec = config.get("maxTokenLifetimeSec") as number | null | undefined;
	if (rawMaxTokenLifetimeSec == null) {
	Lumberjack.warning(
	"Configuration key 'maxTokenLifetimeSec' is missing; defaulting to 0 which will cause all tokens to be immediately rejected.",
	getLumberBaseProperties(),
	);
	}
	const maxTokenLifetimeSec = (rawMaxTokenLifetimeSec as number | null) ?? 0;

[zhangxin511]

Looks good, lease hace @znewton for another eye