Conversation
Implement a structured validation and normalization layer for user-provided configurations to ensure runtime stability. This update introduces a new FingerprintError class Co-authored-by: gitbot <gitbot@gossorg.in>
Update the project changelog and roadmap component to reflect the upcoming 0.9.5 release focused on robustness Co-authored-by: gitbot <gitbot@gossorg.in>
|
You have run out of free Bugbot PR reviews for this billing cycle. This will reset on March 7. To receive reviews on all of your PRs, visit the Cursor dashboard to activate Pro and start your 14-day free trial. |
❌ Deploy Preview for clever-starlight-3034ea failed. Why did it fail? →
|
❌ Deploy Preview for fingerprint-oss failed. Why did it fail? →
|
Greptile SummaryThis PR introduces v0.9.5 focused on robustness and developer experience improvements. The changes add comprehensive configuration validation, structured error handling, configurable geolocation timeouts, and a minimal preset mode for reduced client compute. Key Changes:
Implementation Quality: Confidence Score: 4/5
|
| Filename | Overview |
|---|---|
| src/errors.ts | New file adding structured error handling with FingerprintError class and typed error codes |
| src/config.ts | Added config validation with geoTimeout, minimal preset support, and strictConfig mode; includes validation function with warnings/errors |
| src/geo-ip.ts | Added timeout support using AbortController with configurable timeoutMs and warning callbacks; proper error handling for timeout scenarios |
| src/helper.ts | Added config-based skipping for audio, WebGL, canvas fingerprinting and reduced font detection when minimal preset is active |
| src/index.ts | Integrated config validation, structured errors, and warnings surface; updated telemetry to include error codes |
| src/systemInfo.ts | Applied minimal preset logic to skip heavy collectors and use simpler hardwareConcurrency detection |
Flowchart
%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[userInfo called with config] --> B[validateUserInfoConfig]
B --> C{Has validation errors?}
C -->|Yes| D[Throw FingerprintError CONFIG_INVALID]
C -->|No| E[Initialize config with normalized values]
E --> F{preset === 'minimal'?}
F -->|Yes| G[Set skip flags for heavy collectors]
F -->|No| H[Use default collectors]
G --> I[Fetch system info & geo data in parallel]
H --> I
I --> J{Geo timeout exceeded?}
J -->|Yes| K[AbortController aborts fetch]
K --> L[Create FingerprintError GEO_TIMEOUT]
L --> M[Add warning to runtimeWarnings]
M --> N[Return mock geo data]
J -->|No| O[Process geo response]
O --> P{Other errors?}
P -->|Yes| Q[toFingerprintError conversion]
Q --> R[Log to telemetry with error.code]
P -->|No| S[Generate result JSON]
N --> S
S --> T{Has warnings?}
T -->|Yes| U[Return result with warnings array]
T -->|No| V[Return result]
R --> W[Return fallback data]
Last reviewed commit: 5fb305f
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
Disable sourcemap generation across all Rollup output formats including CommonJS, ES Modules, and UMD bundles. This architectural decision Co-authored-by: gitbot <gitbot@gossorg.in>
📝 WalkthroughSummary by CodeRabbit
WalkthroughAdds config validation and a minimal preset, structured FingerprintError types and conversion, geolocation fetch with abort/timeout and warning callbacks, runtime flags to skip/reduce expensive collectors, telemetry error.code enrichment, rollup sourcemap disabled, .npmignore, demo sitemap/robots/llms additions, and e2e tests for v0.9.5 config behavior. Changes
Sequence DiagramsequenceDiagram
participant Client
participant Validator as Config Validator
participant Preset as Preset Engine
participant Geo as Geo Fetcher
participant Errors as Error Handler
participant Telemetry
Client->>Validator: userInfo(config)
Validator->>Validator: validateUserInfoConfig() / normalize / collect warnings
alt Validation fails
Validator-->>Errors: throw FingerprintError(CONFIG_INVALID)
Errors->>Telemetry: record error (include error.code)
else Validation passes
Validator->>Preset: normalizedConfig
Preset->>Preset: apply minimal preset? set skip flags & geoTimeout
Preset->>Geo: fetchGeolocationInfo({timeoutMs, onWarning})
Geo->>Geo: start AbortController with timeoutMs
alt Geo timeout
Geo->>Validator: onWarning("GEO_TIMEOUT"), return mock geo
else Fetch error / non-OK
Geo->>Errors: toFingerprintError(...)
Errors->>Telemetry: record error (include error.code)
Geo->>Validator: onWarning("GEO_FAILED"), return mock geo
end
Validator-->>Client: userInfo result (warnings[], systemInfo, hash)
Client->>Telemetry: record spans/metrics (include error.code if present)
end
Estimated Code Review Effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly Related PRs
Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
fingerprint-oss-demo/components/roadmap.tsx (1)
42-48:⚠️ Potential issue | 🟡 MinorPre-existing date inversion in
RELEASE_HISTORYmakes v0.9.0 appear after v0.9.2–0.9.4.v0.9.0 is dated
"2025-07-01"(July) while v0.9.2, 0.9.3, and 0.9.4 are all dated January 2025. Since the component renders these in declaration order and the file claims factual accuracy per CHANGELOG.md, this inverted date likely misleads readers. Worth correcting the date (or the ordering) in a follow-up.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@fingerprint-oss-demo/components/roadmap.tsx` around lines 42 - 48, The RELEASE_HISTORY entry for version "0.9.0" in the roadmap component is dated "2025-07-01", which places it after 0.9.2–0.9.4; update the RELEASE_HISTORY data so chronology is correct by either fixing the "0.9.0" date to the actual release date (make it earlier than the January 2025 entries) or reorder the array so versions are in true chronological order; locate the RELEASE_HISTORY constant in the roadmap component (the entry with version "0.9.0") and adjust the date or array order accordingly to match CHANGELOG.md.src/telemetry.ts (1)
46-46:⚠️ Potential issue | 🟡 MinorStale
serviceVersionin telemetry defaults.The default
serviceVersionis'0.9.3'but this PR is for v0.9.5. Telemetry spans will report the wrong version.Fix
- serviceVersion: '0.9.3', + serviceVersion: '0.9.5',Consider deriving this from
package.jsonor a shared constant to avoid manual updates each release.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/telemetry.ts` at line 46, The telemetry default currently hardcodes serviceVersion ('serviceVersion: \'0.9.3\') and must be updated; replace the hardcoded string with a dynamic source (e.g., import the package version or a shared VERSION constant) and set serviceVersion to that value so spans reflect v0.9.5 automatically; locate the serviceVersion entry in src/telemetry.ts and either (a) import { version } from your package.json and assign serviceVersion: version, or (b) import a shared constant (e.g., APP_VERSION or VERSION) and assign serviceVersion: APP_VERSION, ensuring the package version is kept in one place to avoid future staleness.
🧹 Nitpick comments (9)
src/config.ts (2)
277-285: TelemetrysampleRateis silently clamped without a warning, unlikegeoTimeout.
geoTimeoutemits a warning when clamped (line 273–275), butsampleRateclamping on line 280 does not. For consistency and discoverability, consider adding a similar warning here.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/config.ts` around lines 277 - 285, The telemetry.sampleRate is being silently clamped; update the branch that handles config.telemetry (the telemetry object created from config.telemetry and assigned to normalized.telemetry) to detect when telemetry.sampleRate is out of bounds and push a warning into warnings similar to geoTimeout’s handling; use the same clampNumber(telemetry.sampleRate, 0, 1, 0.1) call but before/after clamping compare the original value and, if changed, push a descriptive message into warnings indicating sampleRate was clamped and what the allowed range is.
215-292: Validation has a few notable behaviors worth confirming.
Line 259:
geoTimeoutalways gets a value (fallback6000) even if the user didn't provide one. This silently injects a default intonormalizedConfig, which could surprise callers who checknormalizedConfig.geoTimeout != nullto determine if the user explicitly set it.Line 260: Any preset value other than
'minimal'(includingundefined) is normalized to'default'. Same concern — the distinction between "user explicitly chose default" and "user didn't specify" is lost.Lines 264-275: Warning messages are generated after normalization, which is correct. However, line 273 warns when
geoTimeoutwas clamped but not when it was replaced from a non-number type (e.g.,geoTimeout: "fast"silently becomes6000with no warning).Add a warning for non-numeric geoTimeout
+ if (config.geoTimeout != null && typeof config.geoTimeout !== 'number') { + warnings.push('Invalid "geoTimeout" type; expected number'); + } if (typeof config.geoTimeout === 'number' && config.geoTimeout !== normalized.geoTimeout) { warnings.push(`"geoTimeout" clamped to ${normalized.geoTimeout}ms`); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/config.ts` around lines 215 - 292, The validation currently always injects defaults: change normalized.geoTimeout to only set when user provided a number (use clampNumber only when typeof config.geoTimeout === 'number') and add a warning when config.geoTimeout is present but not a number; also stop forcing preset to 'default' by making normalized.preset reflect only explicit values (set to 'minimal' if config.preset === 'minimal', to 'default' only if config.preset === 'default', otherwise leave undefined) and keep applyPresetMinimal only when normalized.preset === 'minimal'; update validateUserInfoConfig to implement these changes and reference normalized.geoTimeout, normalized.preset, clampNumber, applyPresetMinimal, and the geoTimeout warning logic.src/helper.ts (1)
36-38: Skip guards inhelper.tsduplicate the checks insystemInfo.ts.Each helper function (e.g.,
getAudioFingerprint,getWebGLInfo,getCanvasFingerprint,getFontPreferences) now short-circuits based on config flags. However,systemInfo.ts(lines 205–237) also checks these same flags and substitutes placeholder values before calling the helpers.Having guards in both places is safe but redundant. The helper-level guards are the better location (self-contained functions protecting themselves), so consider removing the duplicate checks from
systemInfo.tsto avoid maintaining two sets of skip logic and skip-return-value literals.Also applies to: 145-147, 282-284, 388-390
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/helper.ts` around lines 36 - 38, Helpers like getAudioFingerprint, getWebGLInfo, getCanvasFingerprint, and getFontPreferences already short-circuit based on getConfig().skip* flags, so remove the redundant config checks and placeholder substitutions from systemInfo.ts (the blocks that pre-check skipAudioFingerprint/skipWebGL/skipCanvas/skipFontPreferences and return literals) and let the helpers be the single source of truth; keep the helper-level guards as-is and update systemInfo.ts to call the helpers directly, deleting the duplicate conditional branches (also remove the similar duplicated checks at the other noted locations).src/systemInfo.ts (1)
205-237: Skip-guard return values are consistent with their helper counterparts — but see note on duplicate logic.The placeholder objects here (e.g.,
{ vendor: 'skipped', renderer: 'skipped', imageHash: 'minimal_preset' }) match exactly whatgetWebGLInfo()returns whenskipWebGLFingerprintis true (helper.ts line 146). Same for canvas and fonts. This is fine but fragile — if a placeholder changes in one place, it must change in the other. As noted in thehelper.tsreview, consolidating to one location would eliminate this risk.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/systemInfo.ts` around lines 205 - 237, The current code duplicates placeholder return objects for skipped fingerprints (e.g., the inline object used when runtimeConfig.skipWebGLFingerprint vs the object returned by getWebGLInfo(), and similarly for getCanvasFingerprint()/canvas and getFontPreferences()/fontPreferences), which is fragile; fix this by removing inline placeholder objects and delegating the skip logic to the helper functions so callers always call getWebGLInfo(), getCanvasFingerprint(), and getFontPreferences() (or equivalent helpers) and those helpers return the appropriate placeholder when runtimeConfig indicates skipping, ensuring a single source of truth for placeholder values.src/types.ts (1)
179-184:UserInfoConfigvsUserInfoConfigInputdivergence.
UserInfoConfighere lacks thetelemetryfield thatUserInfoConfigInputinconfig.ts(line 61–68) includes. If these are meant to represent the same user-facing config shape, the divergence will confuse consumers. If they serve different purposes, a clarifying doc comment on each would help.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/types.ts` around lines 179 - 184, UserInfoConfig in src/types.ts is missing the telemetry field present on UserInfoConfigInput; either add a telemetry property to UserInfoConfig with the same shape/type as UserInfoConfigInput.telemetry (so both types are aligned), or if the divergence is intentional, add a clarifying doc comment above UserInfoConfig explaining how it differs from UserInfoConfigInput and why telemetry is omitted; reference the two types by name (UserInfoConfig and UserInfoConfigInput) when making the change so the shapes remain consistent and consumers aren’t confused.src/errors.ts (1)
18-24: Passcauseto theErrorconstructor to enable the native error chain.
Errorsupports nativecausechaining since ES2022 via the constructor's options object. Currentlysuper(message)ignores the cause, so the built-inError.causeproperty is not populated. This breaks compatibility with tools that walk standard error chains (e.g., OpenTelemetry, structured loggers). The explicitthis.causeassignment on line 23 shadows the inherited property but doesn't activate the native mechanism.Proposed fix
- constructor(code: FingerprintErrorCode, message: string, options?: { details?: unknown; cause?: unknown }) { - super(message); + constructor(code: FingerprintErrorCode, message: string, options?: { details?: unknown; cause?: unknown }) { + super(message, options?.cause != null ? { cause: options.cause } : undefined); this.name = 'FingerprintError'; this.code = code; this.details = options?.details; - this.cause = options?.cause; }You can also remove the explicit
causeproperty declaration on line 16 and rely on the inheritedError.causeinstead.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/errors.ts` around lines 18 - 24, The constructor of FingerprintError currently calls super(message) and then assigns this.cause, which prevents using native Error.cause chaining; update the FingerprintError constructor to pass the cause into the Error constructor (i.e., call super(message, { cause: options?.cause })) and remove the explicit this.cause assignment (and the separate cause property declaration if present) so the built-in Error.cause is used; keep the existing assignments for this.name, this.code and this.details (options?.details) unchanged.CHANGELOG.md (1)
8-8: Should the header read[0.9.5]with a date?The PR title and branch are
v0.9.5, but the CHANGELOG uses[Unreleased]. Per Keep a Changelog convention,[Unreleased]is replaced with the version and date at release time. If this PR is the release, consider updating the header to## [0.9.5] - 2026-02-19to keep the log accurate upon merge.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@CHANGELOG.md` at line 8, Update the CHANGELOG header currently reading "## [Unreleased]" to the released version and date by replacing it with "## [0.9.5] - 2026-02-19" so the file reflects this PR's release; modify the header line (the line containing "## [Unreleased]") accordingly.src/geo-ip.ts (1)
230-243: UnnecessarynormalizedErrorallocation in the abort branch.
normalizedError(Line 231) is created for every fetch error, but theAbortErrorbranch (Lines 232–239) never uses it — it constructs its owntimeoutErrorinstead. Moving the allocation below the abort check avoids the throwaway object.♻️ Suggested reorder
} catch (fetchError) { - const normalizedError = toFingerprintError(fetchError, 'GEO_FAILED', 'Geolocation fetch failed'); if ((fetchError as any)?.name === 'AbortError') { const timeoutError = new FingerprintError('GEO_TIMEOUT', `Geolocation request timed out after ${timeoutMs}ms`, { cause: fetchError }); onWarning?.(timeoutError.message); StructuredLogger.warn('fetchGeolocationInfo', `${timeoutError.message}, using mock data`, timeoutError); return getMockGeolocationData(); } + const normalizedError = toFingerprintError(fetchError, 'GEO_FAILED', 'Geolocation fetch failed'); onWarning?.('Geolocation request failed, using mock data'); StructuredLogger.warn('fetchGeolocationInfo', 'Error fetching geolocation information, using mock data', normalizedError); return getMockGeolocationData();🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/geo-ip.ts` around lines 230 - 243, The code allocates normalizedError via toFingerprintError(fetchError, 'GEO_FAILED', ...) before checking for an AbortError, but normalizedError is unused in the AbortError branch; move the toFingerprintError call so it only runs in the non-abort path: first check if (fetchError as any)?.name === 'AbortError' and handle by creating the FingerprintError('GEO_TIMEOUT', ...) and returning getMockGeolocationData(), then after that branch call toFingerprintError(...) to produce normalizedError for the remaining error handling and StructuredLogger.warn('fetchGeolocationInfo', ..., normalizedError); keep use of timeoutMs, onWarning, getMockGeolocationData, FingerprintError and StructuredLogger.warn as in the original logic.src/index.ts (1)
219-221: Consider always returningwarnings(even if empty) for a stable return shape.Currently
warningsis conditionally present, producing an inconsistent return type. Consumers using TypeScript will see a union which makes destructuring awkward. Returningwarnings: []unconditionally simplifies the contract and avoids a shape-check at every call site.♻️ Suggested change (both success and fallback paths)
- if (runtimeWarnings.length > 0) { - return { ...result, warnings: runtimeWarnings }; - } - return result; + return { ...result, warnings: runtimeWarnings };Apply the same pattern to the fallback path (Lines 257–259).
Also applies to: 257-259
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/index.ts` around lines 219 - 221, The return shape is unstable because `warnings` is only added when `runtimeWarnings.length > 0`; update both the success return that spreads `result` and the fallback return to always include a `warnings` array (e.g., `warnings: runtimeWarnings` or `warnings: []` when null/undefined) so callers always receive `warnings` (even if empty); locate the returns that use `result` and the fallback return and ensure they always set `warnings: runtimeWarnings || []`.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/types.ts`:
- Around line 159-165: There are duplicate declarations of the FingerprintConfig
type (one used by the public API and one used by internal config code) which can
drift; remove the duplicate by keeping a single FingerprintConfig definition in
types.ts and have config.ts import and use that exported type instead of
redeclaring it, updating any references (e.g., where FingerprintConfig is used
in config parsing/validation functions) to the imported symbol so the project
has one source of truth.
---
Outside diff comments:
In `@fingerprint-oss-demo/components/roadmap.tsx`:
- Around line 42-48: The RELEASE_HISTORY entry for version "0.9.0" in the
roadmap component is dated "2025-07-01", which places it after 0.9.2–0.9.4;
update the RELEASE_HISTORY data so chronology is correct by either fixing the
"0.9.0" date to the actual release date (make it earlier than the January 2025
entries) or reorder the array so versions are in true chronological order;
locate the RELEASE_HISTORY constant in the roadmap component (the entry with
version "0.9.0") and adjust the date or array order accordingly to match
CHANGELOG.md.
In `@src/telemetry.ts`:
- Line 46: The telemetry default currently hardcodes serviceVersion
('serviceVersion: \'0.9.3\') and must be updated; replace the hardcoded string
with a dynamic source (e.g., import the package version or a shared VERSION
constant) and set serviceVersion to that value so spans reflect v0.9.5
automatically; locate the serviceVersion entry in src/telemetry.ts and either
(a) import { version } from your package.json and assign serviceVersion:
version, or (b) import a shared constant (e.g., APP_VERSION or VERSION) and
assign serviceVersion: APP_VERSION, ensuring the package version is kept in one
place to avoid future staleness.
---
Nitpick comments:
In `@CHANGELOG.md`:
- Line 8: Update the CHANGELOG header currently reading "## [Unreleased]" to the
released version and date by replacing it with "## [0.9.5] - 2026-02-19" so the
file reflects this PR's release; modify the header line (the line containing "##
[Unreleased]") accordingly.
In `@src/config.ts`:
- Around line 277-285: The telemetry.sampleRate is being silently clamped;
update the branch that handles config.telemetry (the telemetry object created
from config.telemetry and assigned to normalized.telemetry) to detect when
telemetry.sampleRate is out of bounds and push a warning into warnings similar
to geoTimeout’s handling; use the same clampNumber(telemetry.sampleRate, 0, 1,
0.1) call but before/after clamping compare the original value and, if changed,
push a descriptive message into warnings indicating sampleRate was clamped and
what the allowed range is.
- Around line 215-292: The validation currently always injects defaults: change
normalized.geoTimeout to only set when user provided a number (use clampNumber
only when typeof config.geoTimeout === 'number') and add a warning when
config.geoTimeout is present but not a number; also stop forcing preset to
'default' by making normalized.preset reflect only explicit values (set to
'minimal' if config.preset === 'minimal', to 'default' only if config.preset ===
'default', otherwise leave undefined) and keep applyPresetMinimal only when
normalized.preset === 'minimal'; update validateUserInfoConfig to implement
these changes and reference normalized.geoTimeout, normalized.preset,
clampNumber, applyPresetMinimal, and the geoTimeout warning logic.
In `@src/errors.ts`:
- Around line 18-24: The constructor of FingerprintError currently calls
super(message) and then assigns this.cause, which prevents using native
Error.cause chaining; update the FingerprintError constructor to pass the cause
into the Error constructor (i.e., call super(message, { cause: options?.cause
})) and remove the explicit this.cause assignment (and the separate cause
property declaration if present) so the built-in Error.cause is used; keep the
existing assignments for this.name, this.code and this.details
(options?.details) unchanged.
In `@src/geo-ip.ts`:
- Around line 230-243: The code allocates normalizedError via
toFingerprintError(fetchError, 'GEO_FAILED', ...) before checking for an
AbortError, but normalizedError is unused in the AbortError branch; move the
toFingerprintError call so it only runs in the non-abort path: first check if
(fetchError as any)?.name === 'AbortError' and handle by creating the
FingerprintError('GEO_TIMEOUT', ...) and returning getMockGeolocationData(),
then after that branch call toFingerprintError(...) to produce normalizedError
for the remaining error handling and
StructuredLogger.warn('fetchGeolocationInfo', ..., normalizedError); keep use of
timeoutMs, onWarning, getMockGeolocationData, FingerprintError and
StructuredLogger.warn as in the original logic.
In `@src/helper.ts`:
- Around line 36-38: Helpers like getAudioFingerprint, getWebGLInfo,
getCanvasFingerprint, and getFontPreferences already short-circuit based on
getConfig().skip* flags, so remove the redundant config checks and placeholder
substitutions from systemInfo.ts (the blocks that pre-check
skipAudioFingerprint/skipWebGL/skipCanvas/skipFontPreferences and return
literals) and let the helpers be the single source of truth; keep the
helper-level guards as-is and update systemInfo.ts to call the helpers directly,
deleting the duplicate conditional branches (also remove the similar duplicated
checks at the other noted locations).
In `@src/index.ts`:
- Around line 219-221: The return shape is unstable because `warnings` is only
added when `runtimeWarnings.length > 0`; update both the success return that
spreads `result` and the fallback return to always include a `warnings` array
(e.g., `warnings: runtimeWarnings` or `warnings: []` when null/undefined) so
callers always receive `warnings` (even if empty); locate the returns that use
`result` and the fallback return and ensure they always set `warnings:
runtimeWarnings || []`.
In `@src/systemInfo.ts`:
- Around line 205-237: The current code duplicates placeholder return objects
for skipped fingerprints (e.g., the inline object used when
runtimeConfig.skipWebGLFingerprint vs the object returned by getWebGLInfo(), and
similarly for getCanvasFingerprint()/canvas and
getFontPreferences()/fontPreferences), which is fragile; fix this by removing
inline placeholder objects and delegating the skip logic to the helper functions
so callers always call getWebGLInfo(), getCanvasFingerprint(), and
getFontPreferences() (or equivalent helpers) and those helpers return the
appropriate placeholder when runtimeConfig indicates skipping, ensuring a single
source of truth for placeholder values.
In `@src/types.ts`:
- Around line 179-184: UserInfoConfig in src/types.ts is missing the telemetry
field present on UserInfoConfigInput; either add a telemetry property to
UserInfoConfig with the same shape/type as UserInfoConfigInput.telemetry (so
both types are aligned), or if the divergence is intentional, add a clarifying
doc comment above UserInfoConfig explaining how it differs from
UserInfoConfigInput and why telemetry is omitted; reference the two types by
name (UserInfoConfig and UserInfoConfigInput) when making the change so the
shapes remain consistent and consumers aren’t confused.
| geoTimeout?: number; | ||
| preset?: 'default' | 'minimal'; | ||
| skipCanvasFingerprint?: boolean; | ||
| skipAudioFingerprint?: boolean; | ||
| skipWebGLFingerprint?: boolean; | ||
| reduceFontDetection?: boolean; | ||
| skipGeolocation?: boolean; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# Find FingerprintConfig definitions and their context
rg -n 'interface FingerprintConfig|type FingerprintConfig|export.*FingerprintConfig' --type=ts -A5 -B2 src/types.ts src/config.tsRepository: IntegerAlex/fingerprint-oss
Length of output: 2057
🏁 Script executed:
# Check imports in config.ts to see if it imports from types.ts
head -30 src/config.ts | grep -E "import|from"Repository: IntegerAlex/fingerprint-oss
Length of output: 53
🏁 Script executed:
# Count occurrences and context of FingerprintConfig across codebase
rg -n 'FingerprintConfig' --type=ts | head -40Repository: IntegerAlex/fingerprint-oss
Length of output: 654
🏁 Script executed:
# Read the full FingerprintConfig definition in types.ts
sed -n '151,175p' src/types.tsRepository: IntegerAlex/fingerprint-oss
Length of output: 804
🏁 Script executed:
# Read the full FingerprintConfig definition in config.ts along with imports
sed -n '1,80p' src/config.tsRepository: IntegerAlex/fingerprint-oss
Length of output: 2311
🏁 Script executed:
# Check what is imported in index.ts
rg -n 'import.*FingerprintConfig' src/index.ts -B5 -A2Repository: IntegerAlex/fingerprint-oss
Length of output: 53
🏁 Script executed:
# Check full imports section of config.ts (first 100 lines)
head -100 src/config.ts | head -20Repository: IntegerAlex/fingerprint-oss
Length of output: 573
🏁 Script executed:
# Check if config.ts imports anything from types.ts
rg 'import.*from.*types' src/config.tsRepository: IntegerAlex/fingerprint-oss
Length of output: 53
🏁 Script executed:
# Check what index.ts imports from config.ts and types.ts
rg 'import.*from.*[./]*(types|config)' src/index.ts | head -20Repository: IntegerAlex/fingerprint-oss
Length of output: 117
🏁 Script executed:
# Check all imports of FingerprintConfig across the codebase
rg 'import.*FingerprintConfig.*from' --type=tsRepository: IntegerAlex/fingerprint-oss
Length of output: 53
🏁 Script executed:
# Check the exports from index.ts to see what public API is exposed
rg 'export.*FingerprintConfig|export.*from.*config|export.*from.*types' src/index.ts | head -20Repository: IntegerAlex/fingerprint-oss
Length of output: 53
🏁 Script executed:
# Search for all FingerprintConfig references to understand usage pattern
rg 'FingerprintConfig' --type=ts src/Repository: IntegerAlex/fingerprint-oss
Length of output: 624
🏁 Script executed:
# Check if index.ts re-exports from config.ts or types.ts
cat src/index.ts | grep -A2 'export'Repository: IntegerAlex/fingerprint-oss
Length of output: 1455
🏁 Script executed:
# Find the specific import statement for FingerprintConfig in index.ts
grep -n 'import.*FingerprintConfig' src/index.tsRepository: IntegerAlex/fingerprint-oss
Length of output: 53
🏁 Script executed:
# Get the complete export statement with context to see which module provides FingerprintConfig
sed -n '1,50p' src/index.ts | grep -A10 -B10 FingerprintConfigRepository: IntegerAlex/fingerprint-oss
Length of output: 53
🏁 Script executed:
# List all imports at the top of index.ts
head -50 src/index.tsRepository: IntegerAlex/fingerprint-oss
Length of output: 1883
🏁 Script executed:
# Search for the export type statement that includes FingerprintConfig
rg -n 'export type' src/index.ts -A5 -B5Repository: IntegerAlex/fingerprint-oss
Length of output: 1207
Consolidate duplicate FingerprintConfig definition.
FingerprintConfig is independently declared in both types.ts (lines 151–165) and config.ts (lines 20–45) with identical fields. Since config.ts does not import from types.ts, the two definitions can diverge if either is modified independently. The public API re-exports from types.ts, but internal config functions use the config.ts version, creating a potential mismatch.
Establish a single source of truth by defining FingerprintConfig in one module and importing it in the other.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/types.ts` around lines 159 - 165, There are duplicate declarations of the
FingerprintConfig type (one used by the public API and one used by internal
config code) which can drift; remove the duplicate by keeping a single
FingerprintConfig definition in types.ts and have config.ts import and use that
exported type instead of redeclaring it, updating any references (e.g., where
FingerprintConfig is used in config parsing/validation functions) to the
imported symbol so the project has one source of truth.
Introduce a comprehensive Cypress E2E test suite targeting the configuration logic introduced in version 0.9. Co-authored-by: gitbot <gitbot@gossorg.in>
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (2)
test/e2e/cypress/e2e/unit/config-v095.cy.js (2)
1-179: DRY opportunity: extract repeatedcy.loadFingerprintOSS()wrapper intobeforeEachAll seven tests repeat the identical
cy.loadFingerprintOSS().then((fp) => { ... })wrapping pattern. Hoisting the load into abeforeEachhook reduces boilerplate, makes individual tests easier to read, and centralises any future changes to the load mechanism.♻️ Proposed refactor skeleton
describe('v0.9.5 configuration features', () => { + let fp; + + beforeEach(() => { + cy.loadFingerprintOSS().then((loaded) => { + fp = loaded; + }); + }); + it('minimal preset skips heavy collectors and generates warnings', () => { - cy.loadFingerprintOSS().then((fp) => { - return fp({ preset: 'minimal' }).then((result) => { - // ... - }); + cy.wrap(fp({ preset: 'minimal' })).then((result) => { + // ... }); }); // repeat pattern for remaining tests });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@test/e2e/cypress/e2e/unit/config-v095.cy.js` around lines 1 - 179, Tests repeatedly call cy.loadFingerprintOSS() inside each it block; extract that call into a beforeEach to DRY up the suite. Add a beforeEach that calls cy.loadFingerprintOSS().then(fp => { this.fp = fp }) or uses a locally-scoped variable via let fp; assign the returned factory there, and update each it to use that fp instead of calling cy.loadFingerprintOSS() again; ensure references to the factory in tests still use the same symbol (fp) and retain existing promise handling/returns.
173-175: Misleading comment — transparency message is never assertedThe comment
// Should still have transparency messageimplies an assertion about the transparency config, but the following lines only checkresult.systemInfoandresult.hash. Either add an assertion that the transparency-related content is present in the result, or remove/update the comment to reflect what is actually being validated.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@test/e2e/cypress/e2e/unit/config-v095.cy.js` around lines 173 - 175, The comment "Should still have transparency message" is misleading because the test only asserts result.systemInfo and result.hash; update the test to either (A) add an explicit assertion for the transparency content (e.g., check result.systemInfo.transparency or that result.systemInfo contains the expected transparency string) using the same expect(...) style as result.systemInfo and result.hash, or (B) change the comment to accurately describe the existing assertions (e.g., "Should have system info and hash"); locate the assertion block around result.systemInfo and result.hash and implement one of these two fixes.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@test/e2e/cypress/e2e/unit/config-v095.cy.js`:
- Line 149: The test currently hardcodes the pipe-separated enum string in the
assertion (expect(envWarnings.some(w => w.includes('TEST|DEV|STAGING|PROD')))),
which couples the test to formatting; update the assertion to instead verify the
presence of each allowed environment token individually (e.g., check that at
least one warning contains 'TEST', one contains 'DEV', etc.) or assert for a
stable substring like 'environment' plus individual value checks using the
existing envWarnings array so reordering or separator changes in src/config.ts
won't break the test.
- Around line 113-132: The geoTimeout test uses geoTimeout: 50000 which is
clamped to 20000ms and can stall CI; update the tests that only assert the
clamping warning (the "geoTimeout clamping generates warnings" test and the
"strictConfig accepts valid configurations" case using geoTimeout: 5000) to use
a very small timeout (e.g., 100) or mock the geo-IP network call so fp()
resolves immediately; locate the fp() invocation in the tests
(cy.loadFingerprintOSS().then(fp => fp(configWithClampedTimeout) / strictConfig
test) and either change the geoTimeout values to a short number or add a
cy.intercept() for the geo-IP endpoint before calling fp() so the assertion on
result.warnings still runs without waiting for the clamped abort delay.
- Around line 66-88: The test "strictConfig accepts valid configurations"
currently guards the config-errors assertion with if (result.warnings &&
result.warnings.length > 0), which can silently skip the check; change it to
unconditionally assert that result.warnings is an array
(expect(result.warnings).to.be.an('array')) and then compute configErrors from
result.warnings and assert expect(configErrors).to.have.length(0) so the
validation runs every time; also reduce or remove the long geoTimeout in the
validConfig object (the geoTimeout property on validConfig) to a much smaller
value or drop it to avoid CI hangs when geo-IP is unreachable.
- Around line 22-34: The current assertion in the test uses
result.warnings.some(...) with an OR that accepts any warning containing 'geo',
which lets the clamping warning pass; update the check on the hasTimeoutWarning
variable to only detect actual timeout messages (e.g., look for 'timed out' or
'timeout' alone) and exclude warnings that contain 'geo' (or match the clamping
message). Locate the test using fp({ geoTimeout: 1 }) and the hasTimeoutWarning
computation and replace the loose condition with a stricter includes check (or a
regex) that only matches real timeout wording before asserting
expect(hasTimeoutWarning).to.be.true.
---
Nitpick comments:
In `@test/e2e/cypress/e2e/unit/config-v095.cy.js`:
- Around line 1-179: Tests repeatedly call cy.loadFingerprintOSS() inside each
it block; extract that call into a beforeEach to DRY up the suite. Add a
beforeEach that calls cy.loadFingerprintOSS().then(fp => { this.fp = fp }) or
uses a locally-scoped variable via let fp; assign the returned factory there,
and update each it to use that fp instead of calling cy.loadFingerprintOSS()
again; ensure references to the factory in tests still use the same symbol (fp)
and retain existing promise handling/returns.
- Around line 173-175: The comment "Should still have transparency message" is
misleading because the test only asserts result.systemInfo and result.hash;
update the test to either (A) add an explicit assertion for the transparency
content (e.g., check result.systemInfo.transparency or that result.systemInfo
contains the expected transparency string) using the same expect(...) style as
result.systemInfo and result.hash, or (B) change the comment to accurately
describe the existing assertions (e.g., "Should have system info and hash");
locate the assertion block around result.systemInfo and result.hash and
implement one of these two fixes.
| it('geoTimeout generates warnings when fetch times out', () => { | ||
| cy.loadFingerprintOSS().then((fp) => { | ||
| // Very short timeout to force timeout behavior | ||
| return fp({ geoTimeout: 1 }).then((result) => { | ||
| expect(result).to.be.ok; | ||
| expect(result).to.have.property('warnings'); | ||
| expect(result.warnings).to.be.an('array'); | ||
|
|
||
| // Should include a timeout-related warning | ||
| const hasTimeoutWarning = result.warnings.some(warning => | ||
| warning.includes('timeout') || warning.includes('geo') | ||
| ); | ||
| expect(hasTimeoutWarning).to.be.true; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Search for geoTimeout configuration and clamping logic
rg -i "geoTimeout" --type ts --type js -B 3 -A 5 | head -200Repository: IntegerAlex/fingerprint-oss
Length of output: 10016
Test assertion is loose enough to pass on a clamping warning rather than a timeout warning
The assertion on lines 31–33 accepts any warning containing 'timeout' or 'geo'. When geoTimeout: 1 is passed, the implementation clamps it to the minimum of 1000ms and emits the warning: "geoTimeout" clamped to 1000ms". This warning contains the substring 'geo' and will satisfy the condition without a real timeout ever occurring. The test will pass based on the clamping warning alone, making the test name ("generates warnings when fetch times out") misleading.
Replace the loose || logic with a check for an actual timeout indicator (e.g., 'timed out', 'timeout' alone, or check result.warnings.some(w => w.includes('timed out'))). Note that the clamping behavior is already tested explicitly in the "geoTimeout clamping generates warnings" test (lines 113–132).
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@test/e2e/cypress/e2e/unit/config-v095.cy.js` around lines 22 - 34, The
current assertion in the test uses result.warnings.some(...) with an OR that
accepts any warning containing 'geo', which lets the clamping warning pass;
update the check on the hasTimeoutWarning variable to only detect actual timeout
messages (e.g., look for 'timed out' or 'timeout' alone) and exclude warnings
that contain 'geo' (or match the clamping message). Locate the test using fp({
geoTimeout: 1 }) and the hasTimeoutWarning computation and replace the loose
condition with a stricter includes check (or a regex) that only matches real
timeout wording before asserting expect(hasTimeoutWarning).to.be.true.
| it('strictConfig accepts valid configurations', () => { | ||
| cy.loadFingerprintOSS().then((fp) => { | ||
| const validConfig = { | ||
| strictConfig: true, | ||
| transparency: true, | ||
| message: 'test message', | ||
| preset: 'minimal', | ||
| geoTimeout: 5000 | ||
| }; | ||
|
|
||
| return fp(validConfig).then((result) => { | ||
| expect(result).to.be.ok; | ||
| expect(result).to.have.property('hash'); | ||
| // Should not have config validation errors | ||
| if (result.warnings && result.warnings.length > 0) { | ||
| const configErrors = result.warnings.filter(w => | ||
| w.includes('Unknown config key') || w.includes('Invalid configuration') | ||
| ); | ||
| expect(configErrors).to.have.length(0); | ||
| } | ||
| }); | ||
| }); | ||
| }); |
There was a problem hiding this comment.
Conditional warning assertion silently skips the config-error check when warnings is absent or empty
Line 80 wraps the configErrors assertion in if (result.warnings && result.warnings.length > 0). If result.warnings is undefined or an empty array, the assertion is never executed and the test passes trivially. Every other test in this file unconditionally asserts expect(result.warnings).to.be.an('array'). The guard here is inconsistent and could mask a regression where warnings is unexpectedly absent.
Additionally, geoTimeout: 5000 means this test can hang for up to 5 seconds in CI environments where the geo-IP service is unreachable (see also the more severe instance below at lines 113–132).
🛠️ Proposed fix
return fp(validConfig).then((result) => {
expect(result).to.be.ok;
expect(result).to.have.property('hash');
- // Should not have config validation errors
- if (result.warnings && result.warnings.length > 0) {
- const configErrors = result.warnings.filter(w =>
- w.includes('Unknown config key') || w.includes('Invalid configuration')
- );
- expect(configErrors).to.have.length(0);
- }
+ expect(result).to.have.property('warnings');
+ expect(result.warnings).to.be.an('array');
+ const configErrors = result.warnings.filter(w =>
+ w.includes('Unknown config key') || w.includes('Invalid configuration')
+ );
+ expect(configErrors).to.have.length(0);
});🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@test/e2e/cypress/e2e/unit/config-v095.cy.js` around lines 66 - 88, The test
"strictConfig accepts valid configurations" currently guards the config-errors
assertion with if (result.warnings && result.warnings.length > 0), which can
silently skip the check; change it to unconditionally assert that
result.warnings is an array (expect(result.warnings).to.be.an('array')) and then
compute configErrors from result.warnings and assert
expect(configErrors).to.have.length(0) so the validation runs every time; also
reduce or remove the long geoTimeout in the validConfig object (the geoTimeout
property on validConfig) to a much smaller value or drop it to avoid CI hangs
when geo-IP is unreachable.
| it('geoTimeout clamping generates warnings', () => { | ||
| cy.loadFingerprintOSS().then((fp) => { | ||
| // Test with out-of-range geoTimeout values | ||
| const configWithClampedTimeout = { | ||
| geoTimeout: 50000 // Above max (20000) | ||
| }; | ||
|
|
||
| return fp(configWithClampedTimeout).then((result) => { | ||
| expect(result).to.be.ok; | ||
| expect(result.warnings).to.be.an('array'); | ||
|
|
||
| // Should include clamping warning | ||
| const clampingWarnings = result.warnings.filter(w => | ||
| w.includes('geoTimeout') && w.includes('clamped') | ||
| ); | ||
| expect(clampingWarnings).to.have.length(1); | ||
| expect(clampingWarnings[0]).to.include('20000ms'); // Max value | ||
| }); | ||
| }); | ||
| }); |
There was a problem hiding this comment.
geoTimeout: 50000 clamped to 20 000 ms can stall the entire test suite for up to 20 seconds in CI
Line 117 passes geoTimeout: 50000, which the implementation clamps to 20000 ms. The geo-IP fetch then runs with the clamped 20-second abort timer. Since fp() resolves gracefully on geo timeout (as implied by the AI summary and the optional geoInfo pattern elsewhere), the promise will not resolve until the abort fires — up to 20 seconds later when geo services are unreachable in a typical CI environment.
The same issue applies at a smaller scale to the geoTimeout: 5000 value in the strictConfig accepts valid configurations test (line 73).
Recommended fix: use a short geoTimeout (e.g., 100 ms) in tests that only need to verify the clamping warning message, since the clamping itself is a synchronous/pre-fetch operation:
⚡ Proposed fix
// Test with out-of-range geoTimeout values
const configWithClampedTimeout = {
- geoTimeout: 50000 // Above max (20000)
+ geoTimeout: 50000, // Above max (20000) – verify clamping warning only
+ // Pair with a short effective timeout so the actual fetch aborts quickly.
+ // The clamp warning is emitted before the fetch, so 1 ms is sufficient here.
};Or alternatively, mock the geo-IP fetch in a cy.intercept() so it returns immediately and the geoTimeout value has no real latency cost.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| it('geoTimeout clamping generates warnings', () => { | |
| cy.loadFingerprintOSS().then((fp) => { | |
| // Test with out-of-range geoTimeout values | |
| const configWithClampedTimeout = { | |
| geoTimeout: 50000 // Above max (20000) | |
| }; | |
| return fp(configWithClampedTimeout).then((result) => { | |
| expect(result).to.be.ok; | |
| expect(result.warnings).to.be.an('array'); | |
| // Should include clamping warning | |
| const clampingWarnings = result.warnings.filter(w => | |
| w.includes('geoTimeout') && w.includes('clamped') | |
| ); | |
| expect(clampingWarnings).to.have.length(1); | |
| expect(clampingWarnings[0]).to.include('20000ms'); // Max value | |
| }); | |
| }); | |
| }); | |
| it('geoTimeout clamping generates warnings', () => { | |
| cy.loadFingerprintOSS().then((fp) => { | |
| // Test with out-of-range geoTimeout values | |
| const configWithClampedTimeout = { | |
| geoTimeout: 100 // Short timeout to verify clamping warning without stalling | |
| }; | |
| return fp(configWithClampedTimeout).then((result) => { | |
| expect(result).to.be.ok; | |
| expect(result.warnings).to.be.an('array'); | |
| // Should include clamping warning | |
| const clampingWarnings = result.warnings.filter(w => | |
| w.includes('geoTimeout') && w.includes('clamped') | |
| ); | |
| expect(clampingWarnings).to.have.length(1); | |
| expect(clampingWarnings[0]).to.include('20000ms'); // Max value | |
| }); | |
| }); | |
| }); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@test/e2e/cypress/e2e/unit/config-v095.cy.js` around lines 113 - 132, The
geoTimeout test uses geoTimeout: 50000 which is clamped to 20000ms and can stall
CI; update the tests that only assert the clamping warning (the "geoTimeout
clamping generates warnings" test and the "strictConfig accepts valid
configurations" case using geoTimeout: 5000) to use a very small timeout (e.g.,
100) or mock the geo-IP network call so fp() resolves immediately; locate the
fp() invocation in the tests (cy.loadFingerprintOSS().then(fp =>
fp(configWithClampedTimeout) / strictConfig test) and either change the
geoTimeout values to a short number or add a cy.intercept() for the geo-IP
endpoint before calling fp() so the assertion on result.warnings still runs
without waiting for the clamped abort delay.
| w.includes('environment') | ||
| ); | ||
| expect(envWarnings).to.have.length.greaterThan(0); | ||
| expect(envWarnings.some(w => w.includes('TEST|DEV|STAGING|PROD'))).to.be.true; |
There was a problem hiding this comment.
Hardcoded pipe-separated enum string couples the test to the exact implementation format
w.includes('TEST|DEV|STAGING|PROD') encodes both the allowed values and their separator verbatim. Reordering the values, switching to a comma/space separator, or adding a new environment (e.g., PREPROD) in src/config.ts will silently break this assertion with no obvious connection to the change.
Prefer a looser check that asserts each allowed value is present individually, or assert on a stable part of the message (e.g., w.includes('environment') plus w.includes('TEST')).
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@test/e2e/cypress/e2e/unit/config-v095.cy.js` at line 149, The test currently
hardcodes the pipe-separated enum string in the assertion
(expect(envWarnings.some(w => w.includes('TEST|DEV|STAGING|PROD')))), which
couples the test to formatting; update the assertion to instead verify the
presence of each allowed environment token individually (e.g., check that at
least one warning contains 'TEST', one contains 'DEV', etc.) or assert for a
stable substring like 'environment' plus individual value checks using the
existing envWarnings array so reordering or separator changes in src/config.ts
won't break the test.
Introduce a centralized site configuration in lib/site.ts to manage base URLs, route definitions, and project metadata. This architecture enables dynamic generation of sitemap.xml and robots.txt while ensuring consistency across the application. The update includes a new /llms.txt endpoint, implemented via a Next.js API route with force-static for build-time optimization and edge caching. This endpoint follows the AI discovery specification to provide LLMs with structured context about the fingerprinting library and its documentation. Rewrites in next.config. Co-authored-by: gitbot <gitbot@gossorg.in>
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
fingerprint-oss-demo/components/footer.tsx (1)
45-51: Addrel="noopener noreferrer"to external<Link target="_blank">elements.When using
target="_blank", you should always includerel="noopener noreferrer"for security reasons. While in 2024rel="noopener"is implicit fortarget="_blank"in modern browsers,noreferreris not, and the<a>tag at line 28 already sets it correctly. The<Link>components wrapping external URLs (lines 19, 22, 25, 45, 49) should be consistent.🔒 Proposed fix
- <Link href="https://github.com/IntegerAlex" target="_blank" className="text-sm text-muted-foreground hover:text-foreground transition-colors"> + <Link href="https://github.com/IntegerAlex" target="_blank" rel="noopener noreferrer" className="text-sm text-muted-foreground hover:text-foreground transition-colors"> GitHub Profile </Link> <span className="mx-2 text-muted-foreground">|</span> - <Link href="https://gossorg.in" target="_blank" className="text-sm text-muted-foreground hover:text-foreground transition-colors"> + <Link href="https://gossorg.in" target="_blank" rel="noopener noreferrer" className="text-sm text-muted-foreground hover:text-foreground transition-colors"> Enterprise Solution </Link>The same pattern applies to the other external
<Link target="_blank">elements at lines 19, 22, and 25.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@fingerprint-oss-demo/components/footer.tsx` around lines 45 - 51, The external Link components in footer.tsx (the Link elements wrapping "GitHub Profile" and "Enterprise Solution" and the other external Links at the top of the file) use target="_blank" but are missing rel="noopener noreferrer"; update each external Link component (the Link JSX elements pointing to external URLs such as "https://github.com/IntegerAlex" and "https://gossorg.in") to include rel="noopener noreferrer" so all target="_blank" links follow the same secure pattern.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@fingerprint-oss-demo/app/api/llms/route.ts`:
- Around line 42-48: Add an exception in next.config.mjs before the general
`/api/(.*)` no-store rule that specifically matches `/api/llms` and sets the
intended Cache-Control (e.g., "public, max-age=3600, s-maxage=3600"), then
remove the explicit Cache-Control header from the NextResponse created in
app/api/llms/route.ts (the new NextResponse(...) return) so the response only
carries the single, non-conflicting Cache-Control coming from the config.
In `@fingerprint-oss-demo/lib/site.ts`:
- Around line 3-4: The exported constant siteBaseUrl may include a trailing
slash from NEXT_PUBLIC_SITE_URL which causes double-slash URLs; update the
siteBaseUrl definition to normalize input by trimming any trailing slash when
creating the value (keep the fallback 'https://fingerprint-oss.gossorg.in'
as-is), e.g., compute siteBaseUrl from process.env.NEXT_PUBLIC_SITE_URL (if set)
with a trailing-slash removal (use string.replace(/\/+$/, '') or equivalent) so
all downstream interpolations like `${siteBaseUrl}/sitemap.xml` never produce
double slashes.
---
Nitpick comments:
In `@fingerprint-oss-demo/components/footer.tsx`:
- Around line 45-51: The external Link components in footer.tsx (the Link
elements wrapping "GitHub Profile" and "Enterprise Solution" and the other
external Links at the top of the file) use target="_blank" but are missing
rel="noopener noreferrer"; update each external Link component (the Link JSX
elements pointing to external URLs such as "https://github.com/IntegerAlex" and
"https://gossorg.in") to include rel="noopener noreferrer" so all
target="_blank" links follow the same secure pattern.
| return new NextResponse(body, { | ||
| status: 200, | ||
| headers: { | ||
| 'Content-Type': 'text/plain; charset=utf-8', | ||
| 'Cache-Control': 'public, max-age=3600, s-maxage=3600', | ||
| }, | ||
| }) |
There was a problem hiding this comment.
Conflicting Cache-Control headers for direct /api/llms access.
The global /api/(.*) headers rule in next.config.mjs (lines 68–75) sets Cache-Control: no-store, must-revalidate for all API routes. The route handler here also sets Cache-Control: public, max-age=3600, s-maxage=3600. When /api/llms is accessed directly (bypassing the /llms.txt rewrite), both headers land on the same response — which is RFC-noncompliant and produces unpredictable CDN/browser behavior. The /llms.txt → /api/llms rewrite path is unaffected since /llms.txt never matches the /api/(.*) pattern.
To fix, add an exception rule before the general no-store API rule in next.config.mjs:
🛠️ Proposed fix in next.config.mjs
+ // LLMs.txt static caching
+ {
+ source: '/api/llms',
+ headers: [
+ {
+ key: 'Cache-Control',
+ value: 'public, max-age=3600, s-maxage=3600',
+ },
+ ],
+ },
// API routes caching
{
source: '/api/(.*)',
headers: [Then remove the explicit Cache-Control from the handler to avoid the double-set entirely:
return new NextResponse(body, {
status: 200,
headers: {
'Content-Type': 'text/plain; charset=utf-8',
- 'Cache-Control': 'public, max-age=3600, s-maxage=3600',
},
})📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| return new NextResponse(body, { | |
| status: 200, | |
| headers: { | |
| 'Content-Type': 'text/plain; charset=utf-8', | |
| 'Cache-Control': 'public, max-age=3600, s-maxage=3600', | |
| }, | |
| }) | |
| return new NextResponse(body, { | |
| status: 200, | |
| headers: { | |
| 'Content-Type': 'text/plain; charset=utf-8', | |
| }, | |
| }) |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@fingerprint-oss-demo/app/api/llms/route.ts` around lines 42 - 48, Add an
exception in next.config.mjs before the general `/api/(.*)` no-store rule that
specifically matches `/api/llms` and sets the intended Cache-Control (e.g.,
"public, max-age=3600, s-maxage=3600"), then remove the explicit Cache-Control
header from the NextResponse created in app/api/llms/route.ts (the new
NextResponse(...) return) so the response only carries the single,
non-conflicting Cache-Control coming from the config.
| /** Base URL for the site (no trailing slash). */ | ||
| export const siteBaseUrl = process.env.NEXT_PUBLIC_SITE_URL ?? 'https://fingerprint-oss.gossorg.in' |
There was a problem hiding this comment.
Strip trailing slash from siteBaseUrl at definition to prevent double-slash URLs.
The comment documents the "no trailing slash" contract, but nothing enforces it. If NEXT_PUBLIC_SITE_URL is configured with a trailing slash (e.g., https://example.com/), all downstream interpolations (${siteBaseUrl}/sitemap.xml, ${siteBaseUrl}/demo, etc.) silently produce double-slash URLs, breaking sitemap, robots, and llms.txt entries.
🛡️ Proposed fix
-export const siteBaseUrl = process.env.NEXT_PUBLIC_SITE_URL ?? 'https://fingerprint-oss.gossorg.in'
+export const siteBaseUrl = (
+ process.env.NEXT_PUBLIC_SITE_URL ?? 'https://fingerprint-oss.gossorg.in'
+).replace(/\/$/, '')📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| /** Base URL for the site (no trailing slash). */ | |
| export const siteBaseUrl = process.env.NEXT_PUBLIC_SITE_URL ?? 'https://fingerprint-oss.gossorg.in' | |
| /** Base URL for the site (no trailing slash). */ | |
| export const siteBaseUrl = ( | |
| process.env.NEXT_PUBLIC_SITE_URL ?? 'https://fingerprint-oss.gossorg.in' | |
| ).replace(/\/$/, '') |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@fingerprint-oss-demo/lib/site.ts` around lines 3 - 4, The exported constant
siteBaseUrl may include a trailing slash from NEXT_PUBLIC_SITE_URL which causes
double-slash URLs; update the siteBaseUrl definition to normalize input by
trimming any trailing slash when creating the value (keep the fallback
'https://fingerprint-oss.gossorg.in' as-is), e.g., compute siteBaseUrl from
process.env.NEXT_PUBLIC_SITE_URL (if set) with a trailing-slash removal (use
string.replace(/\/+$/, '') or equivalent) so all downstream interpolations like
`${siteBaseUrl}/sitemap.xml` never produce double slashes.
Add a structured llms.txt endpoint at /api/llms to provide Large Language Models with concise technical Co-authored-by: gitbot <gitbot@gossorg.in>
There was a problem hiding this comment.
🧹 Nitpick comments (1)
fingerprint-oss-demo/components/footer.tsx (1)
52-55: Consider openingllms.txtin a new tab.
/llms.txtis served astext/plain. Withouttarget="_blank", clicking it navigates the user away from the current page entirely, which breaks the browsing context. Addingtarget="_blank"(withrel="noopener noreferrer") is consistent with the other external links in this footer.♻️ Proposed fix
- <Link href="/llms.txt" className="text-sm text-muted-foreground hover:text-foreground transition-colors"> + <Link href="/llms.txt" target="_blank" rel="noopener noreferrer" className="text-sm text-muted-foreground hover:text-foreground transition-colors"> llms.txt </Link>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@fingerprint-oss-demo/components/footer.tsx` around lines 52 - 55, The Link element rendering llms.txt currently navigates the user away; update the Link for href="/llms.txt" in the Footer component to open in a new tab by adding target="_blank" and rel="noopener noreferrer" (matching the other external links) so the file opens in a separate browsing context without breaking the current page.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@fingerprint-oss-demo/lib/site.ts`:
- Around line 3-4: The exported constant siteBaseUrl currently trusts
NEXT_PUBLIC_SITE_URL and can include a trailing slash contrary to its JSDoc;
update the initialization of siteBaseUrl (the exported symbol siteBaseUrl) to
normalize the value by removing any trailing slash(es) from
process.env.NEXT_PUBLIC_SITE_URL (e.g., trim with a regex like replace(/\/+$/,
'') or similar) and fall back to the existing default, so all downstream
interpolations like `${siteBaseUrl}/sitemap.xml` never produce double slashes.
---
Nitpick comments:
In `@fingerprint-oss-demo/components/footer.tsx`:
- Around line 52-55: The Link element rendering llms.txt currently navigates the
user away; update the Link for href="/llms.txt" in the Footer component to open
in a new tab by adding target="_blank" and rel="noopener noreferrer" (matching
the other external links) so the file opens in a separate browsing context
without breaking the current page.
1 participant
No description provided.