Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -21,8 +21,45 @@ export const supportsRequestInitExt = (): boolean => (
&& typeof process.versions["undici"] === "string"
)

const fallbackRandomIdLength = 9
const fallbackRandomHexBytes = 5
let fallbackRequestCounter = 0

const truncateRandomId = (value: string): string => value.slice(0, fallbackRandomIdLength)

const webCrypto = (): Crypto | undefined => (
"crypto" in globalThis ? globalThis.crypto : undefined
)

const randomIdFromUuid = (): string | undefined => {
const cryptoApi = webCrypto()
return typeof cryptoApi?.randomUUID === "function"
? truncateRandomId(cryptoApi.randomUUID().replaceAll("-", ""))
: undefined
}

const randomIdFromGetRandomValues = (): string | undefined => {
const cryptoApi = webCrypto()
if (typeof cryptoApi?.getRandomValues !== "function") {
return undefined
}

const bytes = new Uint8Array(fallbackRandomHexBytes)
cryptoApi.getRandomValues(bytes)
return truncateRandomId(Array.from(bytes, (value) => value.toString(16).padStart(2, "0")).join(""))
}

const randomIdFromClock = (): string => {
const timestamp = Date.now().toString(16)
const counter = fallbackRequestCounter.toString(16).padStart(2, "0")
fallbackRequestCounter = (fallbackRequestCounter + 1) % 256
return `${timestamp}${counter}`.slice(-fallbackRandomIdLength).padStart(fallbackRandomIdLength, "0")
}

export const randomID = (): string => (
globalThis.crypto.randomUUID().replaceAll("-", "").slice(0, 9)
randomIdFromUuid()
?? randomIdFromGetRandomValues()
?? randomIdFromClock()
)
Comment on lines +24 to 63

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

📐 Maintainability & Code Quality | 🟠 Major | 🏗️ Heavy lift

New helpers don't follow the mandated Effect-TS/CORE-purity house style.

All four functions (truncateRandomId, webCrypto, randomIdFromUuid, randomIdFromGetRandomValues, randomIdFromClock, randomID) perform side effects (reading globalThis.crypto, calling Date.now(), mutating fallbackRequestCounter) directly as plain functions, with no TSDoc and no functional comment tags.

As per coding guidelines, "Функции с побочными эффектами (IO, сеть, БД) должны возвращать Effect<Success, Error, Requirements>... Запрещено выбрасывать исключения; все ошибки должны быть в типе" and "Все функции должны иметь строгую TSDoc документацию с @param, @returns, @pure, @effect, @invariant, @precondition, @postcondition, @complexity, @throws (Never для effect функций)" and "Использовать функциональные комментарии с обязательными полями: CHANGE, WHY, QUOTE(ТЗ), REF, SOURCE, FORMAT THEOREM, PURITY (CORE|SHELL), EFFECT, INVARIANT, COMPLEXITY."

None of this is present here. Given the surrounding contract (randomID() is called synchronously as an object-literal value in create-client-runtime.ts), fully converting to Effect would require touching the call site too — flagging for awareness even though it's a larger, cross-cutting change.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@packages/app/src/shell/api-client/create-client-runtime-helpers.ts` around
lines 24 - 63, The helper functions in create-client-runtime-helpers.ts are
plain side-effecting utilities, but the review requires CORE/SHELL style
Effect-TS purity and documentation. Update the random ID helpers around
truncateRandomId, webCrypto, randomIdFromUuid, randomIdFromGetRandomValues,
randomIdFromClock, and randomID to follow the mandated functional contract: add
the required TSDoc and functional comment tags, and make the side-effecting
parts explicit in the API. If converting to Effect is too large, at minimum
isolate the impure operations behind a single shell boundary and keep the rest
pure, and note that randomID’s synchronous use in create-client-runtime.ts will
need a matching call-site change if the signature changes.

Source: Coding guidelines


const isQuerySerializerOptions = (
Expand Down
59 changes: 58 additions & 1 deletion packages/app/tests/api-client/create-client-effect.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@
// COMPLEXITY: O(1) per test

import { Effect, Either } from "effect"
import { describe, expect, it } from "vitest"
import { afterEach, describe, expect, it, vi } from "vitest"

import type { paths } from "../../src/core/api/openapi.js"
import type { ClientOptions } from "../../src/shell/api-client/create-client-types.js"
Expand All @@ -34,6 +34,11 @@ const createMockFetch = (
const createFailingFetch = (message: string) => (_request: Request) =>
Effect.runPromise(Effect.fail(new Error(message)))

afterEach(() => {
vi.restoreAllMocks()
vi.unstubAllGlobals()
})

describe("createClientEffect", () => {
it("returns success envelope for login 200", () =>
Effect.gen(function*() {
Expand Down Expand Up @@ -147,4 +152,56 @@ describe("createClientEffect", () => {
}
}
}).pipe(Effect.runPromise))

it("uses getRandomValues when randomUUID is unavailable", () =>
Effect.gen(function*() {
const capturedIds: Array<string> = []
vi.stubGlobal("crypto", {
getRandomValues: (values: Uint8Array): Uint8Array => {
values.set([0x10, 0x32, 0x54, 0x76, 0x98])
return values
}
})

const apiClientEffect = createClientEffect<AuthPaths>({
baseUrl: "https://petstore.example.com",
credentials: "include",
fetch: createMockFetch(200, { "content-type": "application/json" }, JSON.stringify({ ok: true }))
})

apiClientEffect.use({
onRequest: ({ id }) => {
capturedIds.push(id)
}
})

const result = yield* apiClientEffect.GET("/api/auth/me")

expect(result.status).toBe(200)
expect(capturedIds).toEqual(["103254769"])
}).pipe(Effect.runPromise))

it("falls back to a clock-based request id when Web Crypto is missing", () =>
Effect.gen(function*() {
const capturedIds: Array<string> = []
vi.stubGlobal("crypto", undefined)
vi.spyOn(Date, "now").mockReturnValue(0x1234567)

const apiClientEffect = createClientEffect<AuthPaths>({
baseUrl: "https://petstore.example.com",
credentials: "include",
fetch: createMockFetch(200, { "content-type": "application/json" }, JSON.stringify({ ok: true }))
})

apiClientEffect.use({
onRequest: ({ id }) => {
capturedIds.push(id)
}
})

const result = yield* apiClientEffect.GET("/api/auth/me")

expect(result.status).toBe(200)
expect(capturedIds).toEqual(["123456700"])
}).pipe(Effect.runPromise))
})
Loading