From 4a475e22bd24a3fb2e9bac90b5db1086832bca6e Mon Sep 17 00:00:00 2001 From: skulidropek <66840575+skulidropek@users.noreply.github.com> Date: Wed, 17 Jun 2026 18:52:30 +0000 Subject: [PATCH 1/3] fix(effect): align API and session-sync compliance boundaries - add API and session-sync lint:effect gates and wire them into the root script - decode session-sync boundary payloads with schemas and run the CLI entrypoint through Effect NodeRuntime - move API agent snapshot persistence and terminal persistence queue toward Effect/platform primitives - keep command builder core pure by passing clonedOnHostname explicitly Proof of fix: - rtk bun run lint - rtk bun run typecheck - rtk bun run --cwd packages/api typecheck - rtk bun run --cwd packages/api lint - rtk bun run test - rtk bun run --cwd packages/api test - rtk bun run lint:effect --- bun.lock | 20 ++- package.json | 2 +- .../api/eslint.effect-ts-check.config.mjs | 78 +++++++++ packages/api/package.json | 3 + packages/api/src/api/contracts.ts | 1 + packages/api/src/api/schema.ts | 3 +- packages/api/src/services/agents.ts | 147 +++++++++++------ packages/api/src/services/federation.ts | 10 +- packages/api/src/services/projects.ts | 3 +- .../api/src/services/terminal-sessions.ts | 87 +++++++--- packages/api/tests/agents.test.ts | 93 ++++++++++- .../core/command-builders-shared.ts | 23 +++ .../frontend-lib/core/command-builders.ts | 5 +- .../frontend-lib/core/command-options.ts | 1 + .../frontend-lib/usecases/scrap-path.ts | 13 +- .../eslint.effect-ts-check.config.mjs | 78 +++++++++ packages/docker-git-session-sync/package.json | 11 ++ .../docker-git-session-sync/src/backup.ts | 98 ++--------- packages/docker-git-session-sync/src/main.ts | 47 ++++-- .../docker-git-session-sync/src/schemas.ts | 155 ++++++++++++++++++ packages/docker-git-session-sync/src/shell.ts | 51 +++--- .../tests/session-files.test.ts | 25 +++ .../lib/src/core/command-builders-shared.ts | 23 +++ packages/lib/src/core/command-builders.ts | 5 +- packages/lib/src/core/command-options.ts | 1 + packages/lib/src/usecases/scrap-path.ts | 13 +- 26 files changed, 754 insertions(+), 242 deletions(-) create mode 100644 packages/api/eslint.effect-ts-check.config.mjs create mode 100644 packages/docker-git-session-sync/eslint.effect-ts-check.config.mjs create mode 100644 packages/docker-git-session-sync/src/schemas.ts diff --git a/bun.lock b/bun.lock index 5fc002a6..7d3e9eb2 100644 --- a/bun.lock +++ b/bun.lock @@ -12,7 +12,7 @@ }, "packages/api": { "name": "@effect-template/api", - "version": "0.2.1", + "version": "0.2.2", "dependencies": { "@effect-template/lib": "workspace:*", "@effect/platform": "^0.96.1", @@ -27,6 +27,7 @@ }, "devDependencies": { "@effect/vitest": "^0.29.0", + "@eslint-community/eslint-plugin-eslint-comments": "^4.7.2", "@eslint/js": "10.0.1", "@types/node": "^25.9.3", "@types/ws": "^8.18.1", @@ -36,12 +37,13 @@ "fast-check": "4.8.0", "globals": "^17.6.0", "typescript": "^6.0.3", + "typescript-eslint": "^8.61.1", "vitest": "^4.1.9", }, }, "packages/app": { "name": "@prover-coder-ai/docker-git", - "version": "1.3.4", + "version": "1.3.8", "bin": { "docker-git": "dist/src/docker-git/main.js", }, @@ -151,22 +153,32 @@ }, "packages/docker-git-session-sync": { "name": "@prover-coder-ai/docker-git-session-sync", - "version": "1.0.62", + "version": "1.0.64", "bin": { "docker-git-session-sync": "dist/docker-git-session-sync.js", }, + "dependencies": { + "@effect/platform": "^0.96.1", + "@effect/platform-node": "^0.107.0", + "@effect/schema": "^0.75.5", + "effect": "^3.21.3", + }, "devDependencies": { "@effect/vitest": "^0.29.0", + "@eslint-community/eslint-plugin-eslint-comments": "^4.7.2", "@types/node": "^25.9.3", "@vitejs/plugin-react": "^6.0.2", + "eslint": "^10.5.0", + "globals": "^17.6.0", "typescript": "^6.0.3", + "typescript-eslint": "^8.61.1", "vite": "^8.0.16", "vitest": "^4.1.9", }, }, "packages/lib": { "name": "@effect-template/lib", - "version": "1.2.0", + "version": "1.2.1", "dependencies": { "@effect/cli": "^0.75.2", "@effect/cluster": "^0.59.0", diff --git a/package.json b/package.json index cc1246c6..b1a942d8 100644 --- a/package.json +++ b/package.json @@ -46,7 +46,7 @@ "web:serve": "bun run --cwd packages/app serve:web", "lint": "bun run --filter @prover-coder-ai/docker-git-terminal lint && bun run --filter @prover-coder-ai/docker-git lint && bun run --filter @effect-template/lib lint", "lint:tests": "bun run --filter @prover-coder-ai/docker-git lint:tests", - "lint:effect": "bun run --filter @prover-coder-ai/docker-git-terminal lint:effect && bun run --filter @prover-coder-ai/docker-git lint:effect && bun run --filter @effect-template/lib lint:effect", + "lint:effect": "bun run --filter @prover-coder-ai/docker-git-session-sync lint:effect && bun run --filter @prover-coder-ai/docker-git-terminal lint:effect && bun run --filter @prover-coder-ai/docker-git lint:effect && bun run --filter @prover-coder-ai/docker-git-container lint:effect && bun run --filter @effect-template/lib lint:effect && bun run --filter @effect-template/api lint:effect", "test": "bun run --filter @prover-coder-ai/docker-git-session-sync test && bun run --filter @prover-coder-ai/docker-git-terminal test && bun run --filter @prover-coder-ai/docker-git test && bun run --filter @effect-template/lib test", "typecheck": "bun run --filter @prover-coder-ai/docker-git-session-sync typecheck && bun run --filter @prover-coder-ai/docker-git-terminal typecheck && bun run --filter @prover-coder-ai/docker-git typecheck && bun run --filter @effect-template/lib typecheck", "start": "bun run --cwd packages/app build:docker-git && bun ./packages/app/dist/src/docker-git/main.js" diff --git a/packages/api/eslint.effect-ts-check.config.mjs b/packages/api/eslint.effect-ts-check.config.mjs new file mode 100644 index 00000000..3bc0d6ed --- /dev/null +++ b/packages/api/eslint.effect-ts-check.config.mjs @@ -0,0 +1,78 @@ +// CHANGE: add API-local Effect-TS compliance lint profile. +// WHY: expose Effect migration blockers in the API package, which previously only ran baseline ESLint. +// QUOTE(TZ): "packages/api не защищён lint:effect" +// REF: user-request-2026-06-17-effect-compliance-api +// SOURCE: n/a +// FORMAT THEOREM: lint(api) = hard unsafe bypass errors + visible Effect migration blockers +// PURITY: SHELL +// EFFECT: eslint config +// INVARIANT: config does not alter runtime behavior. +// COMPLEXITY: O(1)/O(1) +import eslintComments from "@eslint-community/eslint-plugin-eslint-comments" +import globals from "globals" +import tseslint from "typescript-eslint" + +const migrationBlockers = [ + { + selector: "SwitchStatement", + message: "Effect migration blocker: use Match.exhaustive instead of switch." + }, + { + selector: "TryStatement", + message: "Effect migration blocker: use Effect.try / Effect.catch* instead of try/catch." + }, + { + selector: "AwaitExpression", + message: "Effect migration blocker: use Effect.gen / Effect.flatMap instead of await." + }, + { + selector: "FunctionDeclaration[async=true], FunctionExpression[async=true], ArrowFunctionExpression[async=true]", + message: "Effect migration blocker: use Effect.gen / Effect.tryPromise instead of async functions." + }, + { + selector: "NewExpression[callee.name='Promise']", + message: "Effect migration blocker: use Effect.async / Effect.tryPromise instead of new Promise." + }, + { + selector: "CallExpression[callee.object.name='Promise']", + message: "Effect migration blocker: use Effect combinators instead of Promise.*." + } +] + +export default tseslint.config({ + name: "api-effect-ts-compliance", + files: ["src/**/*.ts", "tests/**/*.ts"], + languageOptions: { + parser: tseslint.parser, + globals: { ...globals.node } + }, + plugins: { + "@typescript-eslint": tseslint.plugin, + "eslint-comments": eslintComments + }, + rules: { + "@typescript-eslint/ban-ts-comment": ["error", { + "ts-check": false, + "ts-expect-error": true, + "ts-ignore": true, + "ts-nocheck": true + }], + "@typescript-eslint/no-explicit-any": "error", + "@typescript-eslint/no-restricted-types": ["warn", { + types: { + Promise: { + message: "Avoid Promise in public types. Use Effect.Effect." + }, + "Promise<*>": { + message: "Avoid Promise. Use Effect.Effect." + } + } + }], + "eslint-comments/disable-enable-pair": "error", + "eslint-comments/no-unlimited-disable": "error", + "eslint-comments/no-unused-disable": "error", + "eslint-comments/no-use": "error", + "no-console": "error", + "no-restricted-syntax": ["warn", ...migrationBlockers] + } +}) diff --git a/packages/api/package.json b/packages/api/package.json index 465809ec..b61ad57f 100644 --- a/packages/api/package.json +++ b/packages/api/package.json @@ -15,6 +15,7 @@ "pretypecheck": "bun run --cwd ../terminal build && bun run --cwd ../container build && bun run --cwd ../lib build", "typecheck": "tsc --noEmit -p tsconfig.json", "lint": "eslint .", + "lint:effect": "eslint --config eslint.effect-ts-check.config.mjs .", "pretest": "bun run --cwd ../terminal build && bun run --cwd ../container build && bun run --cwd ../lib build", "test": "vitest run" }, @@ -41,6 +42,7 @@ "homepage": "https://github.com/ProverCoderAI/docker-git#readme", "devDependencies": { "@effect/vitest": "^0.29.0", + "@eslint-community/eslint-plugin-eslint-comments": "^4.7.2", "@eslint/js": "10.0.1", "@types/node": "^25.9.3", "@types/ws": "^8.18.1", @@ -50,6 +52,7 @@ "fast-check": "4.8.0", "globals": "^17.6.0", "typescript": "^6.0.3", + "typescript-eslint": "^8.61.1", "vitest": "^4.1.9" } } diff --git a/packages/api/src/api/contracts.ts b/packages/api/src/api/contracts.ts index 21b652ea..187bcb09 100644 --- a/packages/api/src/api/contracts.ts +++ b/packages/api/src/api/contracts.ts @@ -480,6 +480,7 @@ export type CreateProjectRequest = { readonly forceEnv?: boolean | undefined readonly waitForClone?: boolean | undefined readonly async?: boolean | undefined + readonly clonedOnHostname?: string | undefined } export type AgentEnvVar = { diff --git a/packages/api/src/api/schema.ts b/packages/api/src/api/schema.ts index bc629ab2..7662278d 100644 --- a/packages/api/src/api/schema.ts +++ b/packages/api/src/api/schema.ts @@ -51,7 +51,8 @@ export const CreateProjectRequestSchema = Schema.Struct({ force: OptionalBoolean, forceEnv: OptionalBoolean, waitForClone: OptionalBoolean, - async: OptionalBoolean + async: OptionalBoolean, + clonedOnHostname: OptionalString }) export const GithubAuthLoginRequestSchema = Schema.Struct({ diff --git a/packages/api/src/services/agents.ts b/packages/api/src/services/agents.ts index 0b3bf3e9..158d25f5 100644 --- a/packages/api/src/services/agents.ts +++ b/packages/api/src/services/agents.ts @@ -2,12 +2,15 @@ import { recordProjectRuntimeActivity } from "@effect-template/lib" import { runCommandWithExitCodes } from "@effect-template/lib/shell/command-runner" import { CommandFailedError } from "@effect-template/lib/shell/errors" import { defaultProjectsRoot } from "@effect-template/lib/usecases/path-helpers" +import type { PlatformError } from "@effect/platform/Error" +import * as FileSystem from "@effect/platform/FileSystem" import { NodeContext } from "@effect/platform-node" -import { Effect } from "effect" +import { Effect, Either } from "effect" +import * as ParseResult from "effect/ParseResult" +import * as Schema from "effect/Schema" import { spawn, type ChildProcess } from "node:child_process" import { randomUUID } from "node:crypto" -import { promises as fs } from "node:fs" -import { join } from "node:path" +import { dirname, join } from "node:path" import type { AgentLogLine, @@ -15,6 +18,7 @@ import type { CreateAgentRequest, ProjectDetails } from "../api/contracts.js" +import { AgentSessionSchema } from "../api/schema.js" import { ApiBadRequestError, ApiConflictError, ApiNotFoundError } from "../api/errors.js" import { emitProjectEvent } from "./events.js" @@ -31,11 +35,28 @@ type SnapshotFile = { readonly sessions: ReadonlyArray } +const SnapshotFileSchema = Schema.Struct({ + sessions: Schema.Array(AgentSessionSchema) +}) + +const SnapshotFileJsonSchema = Schema.parseJson(SnapshotFileSchema) + const records: Map = new Map() const projectIndex: Map> = new Map() const maxLogLines = 5000 let initialized = false +export const clearAgentRuntimeForTest = (): void => { + for (const record of records.values()) { + if (record.process !== null && !record.process.killed) { + record.process.kill("SIGTERM") + } + } + records.clear() + projectIndex.clear() + initialized = false +} + const nowIso = (): string => new Date().toISOString() const stateFilePath = (): string => @@ -184,19 +205,32 @@ export const buildAgentDockerExecArgs = ( const trimLogs = (logs: Array): Array => logs.length <= maxLogLines ? logs : logs.slice(logs.length - maxLogLines) -const persistSnapshot = async (): Promise => { - const filePath = stateFilePath() - await fs.mkdir(join(filePath, ".."), { recursive: true }) - const payload: SnapshotFile = { - sessions: [...records.values()].map((record) => record.session) - } - await fs.writeFile(filePath, JSON.stringify(payload, null, 2), "utf8") -} +const emptySnapshotFile = (): SnapshotFile => ({ + sessions: [] +}) -const persistSnapshotBestEffort = (): void => { - void persistSnapshot().catch(() => { - // best effort snapshot persistence +const decodeSnapshotFile = (input: string): SnapshotFile | null => + Either.match(ParseResult.decodeUnknownEither(SnapshotFileJsonSchema)(input), { + onLeft: () => null, + onRight: (value) => value }) + +const persistSnapshot = (): Effect.Effect => + Effect.gen(function*(_) { + const fs = yield* _(FileSystem.FileSystem) + const filePath = stateFilePath() + const payload: SnapshotFile = { + sessions: [...records.values()].map((record) => record.session) + } + yield* _(fs.makeDirectory(dirname(filePath), { recursive: true })) + yield* _(fs.writeFileString(filePath, `${JSON.stringify(payload, null, 2)}\n`)) + }) + +const persistSnapshotBestEffort = (): void => { + Effect.runFork(persistSnapshot().pipe( + Effect.provide(NodeContext.layer), + Effect.catchAll(() => Effect.void) + )) } const recordAgentActivityBestEffort = (projectId: string): void => { @@ -321,50 +355,61 @@ const killAgentScript = (sessionId: string): string => { ].join("\n") } -const hydrateFromSnapshot = async (): Promise => { - const filePath = stateFilePath() - const exists = await fs.stat(filePath).then(() => true).catch(() => false) - if (!exists) { - return - } - - const raw = await fs.readFile(filePath, "utf8") - const parsed = JSON.parse(raw) as SnapshotFile - for (const session of parsed.sessions ?? []) { - const restored: AgentSession = { - ...session, - status: endedStatuses.has(session.status) ? session.status : "exited", - hostPid: null, - stoppedAt: session.stoppedAt ?? nowIso(), - updatedAt: nowIso() +const readSnapshotFile = (): Effect.Effect => + Effect.gen(function*(_) { + const fs = yield* _(FileSystem.FileSystem) + const filePath = stateFilePath() + const exists = yield* _(Effect.either(fs.exists(filePath))) + const fileExists = Either.match(exists, { + onLeft: () => false, + onRight: (value) => value + }) + if (!fileExists) { + return emptySnapshotFile() } + const contents = yield* _(Effect.either(fs.readFileString(filePath))) + return Either.match(contents, { + onLeft: () => emptySnapshotFile(), + onRight: (value) => decodeSnapshotFile(value) ?? emptySnapshotFile() + }) + }).pipe(Effect.catchAll(() => Effect.succeed(emptySnapshotFile()))) - const record: AgentRecord = { - session: restored, - projectDir: "", - logs: [], - process: null, - stdoutRemainder: "", - stderrRemainder: "" - } +const hydrateFromSnapshot = (): Effect.Effect => + Effect.gen(function*(_) { + const parsed = yield* _(readSnapshotFile()) + for (const session of parsed.sessions) { + const restored: AgentSession = { + ...session, + status: endedStatuses.has(session.status) ? session.status : "exited", + hostPid: null, + stoppedAt: session.stoppedAt ?? nowIso(), + updatedAt: nowIso() + } - records.set(restored.id, record) - upsertProjectIndex(restored.projectId, restored.id) - } -} + const record: AgentRecord = { + session: restored, + projectDir: "", + logs: [], + process: null, + stdoutRemainder: "", + stderrRemainder: "" + } + + records.set(restored.id, record) + upsertProjectIndex(restored.projectId, restored.id) + } + }) export const initializeAgentState = () => - Effect.tryPromise({ - try: async () => { - if (initialized) { - return - } - await hydrateFromSnapshot() - initialized = true - }, - catch: (error) => new Error(String(error)) + Effect.gen(function*(_) { + if (initialized) { + return + } + yield* _(hydrateFromSnapshot()) + initialized = true }).pipe( Effect.catchAll(() => Effect.void), + Effect.provide(NodeContext.layer), Effect.asVoid ) diff --git a/packages/api/src/services/federation.ts b/packages/api/src/services/federation.ts index c7985476..325f4b97 100644 --- a/packages/api/src/services/federation.ts +++ b/packages/api/src/services/federation.ts @@ -1474,9 +1474,9 @@ const ensureConfiguredExchangeSubscriptions = ( (target) => ensureExchangeSubscription({ target }, context).pipe( Effect.tapError((error) => - Effect.sync(() => { - console.warn("[ActivityPub] Failed to subscribe to exchange target:", target, error) - }) + Effect.logWarning( + `[ActivityPub] Failed to subscribe to exchange target ${target}: ${String(error)}` + ) ), Effect.ignore ), @@ -1903,9 +1903,7 @@ export const startOutboxPolling = ( const poll = pollExchangeOutboxes({}, context).pipe( Effect.tapError((error) => - Effect.sync(() => { - console.warn("[ActivityPub Polling] poll failed:", error) - }) + Effect.logWarning(`[ActivityPub Polling] poll failed: ${String(error)}`) ), Effect.ignore ) diff --git a/packages/api/src/services/projects.ts b/packages/api/src/services/projects.ts index 58b77baf..b423a51e 100644 --- a/packages/api/src/services/projects.ts +++ b/packages/api/src/services/projects.ts @@ -482,7 +482,8 @@ const toCreateRawOptions = (request: CreateProjectRequest): RawOptions => ({ ...(request.up === undefined ? {} : { up: request.up }), ...(request.openSsh === undefined ? {} : { openSsh: request.openSsh }), ...(request.force === undefined ? {} : { force: request.force }), - ...(request.forceEnv === undefined ? {} : { forceEnv: request.forceEnv }) + ...(request.forceEnv === undefined ? {} : { forceEnv: request.forceEnv }), + ...(request.clonedOnHostname === undefined ? {} : { clonedOnHostname: request.clonedOnHostname }) }) const parseCreateCommandRequest = ( diff --git a/packages/api/src/services/terminal-sessions.ts b/packages/api/src/services/terminal-sessions.ts index e637731e..83488b20 100644 --- a/packages/api/src/services/terminal-sessions.ts +++ b/packages/api/src/services/terminal-sessions.ts @@ -82,6 +82,13 @@ type TerminalSessionStateRuntime = | FileSystem.FileSystem | PlatformPath.Path +type TerminalSessionPersistenceEffect = Effect.Effect + +type TerminalSessionPersistenceQueue = { + readonly items: ReadonlyArray + readonly running: boolean +} + type DurableTerminalSession = { readonly id: string readonly projectId: string @@ -103,7 +110,7 @@ type DurableTerminalSessionFile = { } const records = new Map() -const terminalSessionPersistenceQueues = new Map>() +const terminalSessionPersistenceQueues = new Map() const terminalActivityWrites = new Map() const terminalWsPathPattern = /^(?:\/api)?\/projects\/([^/]+)\/terminal-sessions\/([^/]+)\/ws$/u const terminalWsByKeyPathPattern = /^(?:\/api)?\/projects\/by-key\/([^/]+)\/terminal-sessions\/([^/]+)\/ws$/u @@ -382,35 +389,67 @@ const findDurableSession = ( Effect.map((state) => state.sessions.find((session) => session.id === sessionId) ?? null) ) +const drainTerminalSessionPersistenceQueue = (projectId: string): void => { + const queue = terminalSessionPersistenceQueues.get(projectId) + if (queue === undefined || queue.running || queue.items.length === 0) { + return + } + + const [nextEffect, ...remainingItems] = queue.items + if (nextEffect === undefined) { + return + } + + terminalSessionPersistenceQueues.set(projectId, { + items: remainingItems, + running: true + }) + + Effect.runFork( + nextEffect.pipe( + Effect.provide(NodeContext.layer), + Effect.catchAll((error) => + Effect.logWarning( + `[terminal-sessions] Failed to persist state for project ${projectId}: ${describeUnknown(error)}` + ) + ), + Effect.ensuring( + Effect.sync(() => { + const current = terminalSessionPersistenceQueues.get(projectId) + if (current === undefined) { + return + } + if (current.items.length === 0) { + terminalSessionPersistenceQueues.delete(projectId) + return + } + terminalSessionPersistenceQueues.set(projectId, { + items: current.items, + running: false + }) + drainTerminalSessionPersistenceQueue(projectId) + }) + ) + ) + ) +} + const isAppError = (value: unknown): value is AppError => typeof value === "object" && value !== null && "_tag" in value const runTerminalSessionPersistence = ( projectId: string, - effect: Effect.Effect + effect: TerminalSessionPersistenceEffect ): void => { - const previous = terminalSessionPersistenceQueues.get(projectId) ?? Promise.resolve() - const next = previous - .catch(() => undefined) - .then(() => - Effect.runPromise( - effect.pipe( - Effect.provide(NodeContext.layer), - Effect.catchAll((error) => - Effect.logWarning( - `[terminal-sessions] Failed to persist state for project ${projectId}: ${describeUnknown(error)}` - ) - ) - ) - ) - ) - .catch(() => undefined) - .finally(() => { - if (terminalSessionPersistenceQueues.get(projectId) === next) { - terminalSessionPersistenceQueues.delete(projectId) - } - }) - terminalSessionPersistenceQueues.set(projectId, next) + const current = terminalSessionPersistenceQueues.get(projectId) ?? { + items: [], + running: false + } + terminalSessionPersistenceQueues.set(projectId, { + items: [...current.items, effect], + running: current.running + }) + drainTerminalSessionPersistenceQueue(projectId) } const updateSession = ( diff --git a/packages/api/tests/agents.test.ts b/packages/api/tests/agents.test.ts index 438d4b87..2145ac8c 100644 --- a/packages/api/tests/agents.test.ts +++ b/packages/api/tests/agents.test.ts @@ -1,6 +1,43 @@ -import { describe, expect, it } from "vitest" +import { Effect } from "effect" +import { mkdirSync, mkdtempSync, rmSync, writeFileSync } from "node:fs" +import os from "node:os" +import path from "node:path" +import { afterEach, beforeEach, describe, expect, it } from "vitest" -import { buildAgentDockerExecArgs, buildAgentScript, buildCommand } from "../src/services/agents.js" +import { + buildAgentDockerExecArgs, + buildAgentScript, + buildCommand, + clearAgentRuntimeForTest, + initializeAgentState, + listAgents +} from "../src/services/agents.js" + +const writeAgentSnapshot = (projectsRoot: string, content: string): void => { + const stateDir = path.join(projectsRoot, ".orch", "state") + mkdirSync(stateDir, { recursive: true }) + writeFileSync(path.join(stateDir, "api-agents.json"), content, "utf8") +} + +let projectsRoot = "" +let previousProjectsRoot: string | undefined + +beforeEach(() => { + previousProjectsRoot = process.env["DOCKER_GIT_PROJECTS_ROOT"] + projectsRoot = mkdtempSync(path.join(os.tmpdir(), "docker-git-agents-")) + process.env["DOCKER_GIT_PROJECTS_ROOT"] = projectsRoot + clearAgentRuntimeForTest() +}) + +afterEach(() => { + clearAgentRuntimeForTest() + if (previousProjectsRoot === undefined) { + delete process.env["DOCKER_GIT_PROJECTS_ROOT"] + } else { + process.env["DOCKER_GIT_PROJECTS_ROOT"] = previousProjectsRoot + } + rmSync(projectsRoot, { recursive: true, force: true }) +}) describe("agent service", () => { it("starts default Codex agents with isolated Playwright MCP", () => { @@ -102,4 +139,56 @@ describe("agent service", () => { "echo ok" ]) }) + + it("hydrates persisted agent sessions through typed snapshot decoding", () => + Effect.runPromise( + Effect.sync(() => { + writeAgentSnapshot(projectsRoot, JSON.stringify({ + sessions: [ + { + id: "agent-1", + projectId: "project-1", + provider: "codex", + label: "Codex", + command: "codex", + containerName: "project-container", + status: "running", + source: "provider:codex", + pidFile: "/tmp/docker-git-agent-agent-1.pid", + hostPid: 1234, + startedAt: "2026-06-17T00:00:00.000Z", + updatedAt: "2026-06-17T00:00:00.000Z" + } + ] + })) + }).pipe( + Effect.zipRight(initializeAgentState()), + Effect.tap(() => + Effect.sync(() => { + expect(listAgents("project-1")).toMatchObject([ + { + id: "agent-1", + hostPid: null, + status: "exited", + stoppedAt: expect.any(String) + } + ]) + }) + ) + ) + )) + + it("treats invalid persisted agent snapshots as empty best-effort state", () => + Effect.runPromise( + Effect.sync(() => { + writeAgentSnapshot(projectsRoot, "{ invalid json") + }).pipe( + Effect.zipRight(initializeAgentState()), + Effect.tap(() => + Effect.sync(() => { + expect(listAgents("project-1")).toEqual([]) + }) + ) + ) + )) }) diff --git a/packages/app/src/docker-git/frontend-lib/core/command-builders-shared.ts b/packages/app/src/docker-git/frontend-lib/core/command-builders-shared.ts index 291450c4..580378a0 100644 --- a/packages/app/src/docker-git/frontend-lib/core/command-builders-shared.ts +++ b/packages/app/src/docker-git/frontend-lib/core/command-builders-shared.ts @@ -70,6 +70,29 @@ export const trimTrailingPathSeparators = (value: string): string => { return value.slice(0, end) } +/** + * Expands POSIX home shorthand for paths inside the generated project container. + * + * @param sshUser - Validated container user name. + * @param value - Raw target path candidate. + * @returns The path with `~` expanded to `/home/${sshUser}`. + * @pure true + * @effect none; CORE helper only transforms provided strings. + * @invariant result is a deterministic function of `(sshUser, value)`. + * @precondition sshUser was validated by parseSshUser. + * @postcondition `~` and `~/x` no longer contain home shorthand. + * @complexity O(n) time / O(n) space where n = |value|. + */ +export const expandContainerHome = (sshUser: string, value: string): string => { + if (value === "~") { + return `/home/${sshUser}` + } + if (value.startsWith("~/")) { + return `/home/${sshUser}${value.slice(1)}` + } + return value +} + /** * Parses a raw SSH port value into the valid Docker host-port range. * diff --git a/packages/app/src/docker-git/frontend-lib/core/command-builders.ts b/packages/app/src/docker-git/frontend-lib/core/command-builders.ts index 45f04f9c..ab42aaa4 100644 --- a/packages/app/src/docker-git/frontend-lib/core/command-builders.ts +++ b/packages/app/src/docker-git/frontend-lib/core/command-builders.ts @@ -1,9 +1,9 @@ /* jscpd:ignore-start */ import { Either } from "effect" -import { expandContainerHome } from "../usecases/scrap-path.js" import { resolveAutoAgentFlags } from "./auto-agent-flags.js" import { + expandContainerHome, nonEmpty, parseDockerNetworkMode, parseGpuMode, @@ -269,7 +269,8 @@ export const buildCreateCommand = ( skipGithubAuth: behavior.skipGithubAuth, enableMcpPlaywright: behavior.enableMcpPlaywright, agentMode, - agentAuto: isAgentAuto + agentAuto: isAgentAuto, + clonedOnHostname: raw.clonedOnHostname }) } }) diff --git a/packages/app/src/docker-git/frontend-lib/core/command-options.ts b/packages/app/src/docker-git/frontend-lib/core/command-options.ts index d0d1ec3a..3883d63d 100644 --- a/packages/app/src/docker-git/frontend-lib/core/command-options.ts +++ b/packages/app/src/docker-git/frontend-lib/core/command-options.ts @@ -60,6 +60,7 @@ export interface RawOptions { readonly force?: boolean readonly forceEnv?: boolean readonly agentAutoMode?: string + readonly clonedOnHostname?: string } // CHANGE: helper type alias for builder signatures that produce parse errors diff --git a/packages/app/src/docker-git/frontend-lib/usecases/scrap-path.ts b/packages/app/src/docker-git/frontend-lib/usecases/scrap-path.ts index d2d947a8..8ba8de2e 100644 --- a/packages/app/src/docker-git/frontend-lib/usecases/scrap-path.ts +++ b/packages/app/src/docker-git/frontend-lib/usecases/scrap-path.ts @@ -1,20 +1,11 @@ /* jscpd:ignore-start */ import { Either } from "effect" +import { expandContainerHome } from "../core/command-builders-shared.js" import { ScrapTargetDirUnsupportedError } from "../shell/errors.js" const normalizeContainerPath = (value: string): string => value.replaceAll("\\", "/").trim() -export const expandContainerHome = (sshUser: string, value: string): string => { - if (value === "~") { - return `/home/${sshUser}` - } - if (value.startsWith("~/")) { - return `/home/${sshUser}${value.slice(1)}` - } - return value -} - const trimTrailingPosixSlashes = (value: string): string => { let end = value.length while (end > 0 && value[end - 1] === "/") { @@ -70,3 +61,5 @@ export const deriveScrapWorkspaceRelativePath = ( return Either.right(relative) } /* jscpd:ignore-end */ + +export { expandContainerHome } from "../core/command-builders-shared.js" diff --git a/packages/docker-git-session-sync/eslint.effect-ts-check.config.mjs b/packages/docker-git-session-sync/eslint.effect-ts-check.config.mjs new file mode 100644 index 00000000..6273c4ad --- /dev/null +++ b/packages/docker-git-session-sync/eslint.effect-ts-check.config.mjs @@ -0,0 +1,78 @@ +// CHANGE: Add a package-local Effect-TS compliance scan. +// WHY: This package is mid-migration; hard failures catch unsafe bypasses while warnings expose remaining shell blockers. +// QUOTE(TZ): "добавить package deps/scripts/config" +// REF: user-request-2026-06-17-effect-compliance-session-sync +// SOURCE: n/a +// FORMAT THEOREM: findings(src) = errors(unsafe bypasses) ∪ warnings(effect migration blockers) +// PURITY: SHELL +// EFFECT: eslint config +// INVARIANT: lint configuration does not change runtime behavior. +// COMPLEXITY: O(1)/O(1) +import eslintComments from "@eslint-community/eslint-plugin-eslint-comments" +import globals from "globals" +import tseslint from "typescript-eslint" + +const effectMigrationWarnings = [ + { + selector: "TryStatement", + message: "Effect migration blocker: replace try/catch with Effect.try / Effect.catch* at shell boundaries." + }, + { + selector: "SwitchStatement", + message: "Effect migration blocker: use Match.exhaustive instead of switch." + }, + { + selector: "AwaitExpression", + message: "Effect migration blocker: use Effect.gen / Effect.flatMap instead of await." + }, + { + selector: "FunctionDeclaration[async=true], FunctionExpression[async=true], ArrowFunctionExpression[async=true]", + message: "Effect migration blocker: use Effect.gen / Effect.tryPromise instead of async functions." + }, + { + selector: "NewExpression[callee.name='Promise']", + message: "Effect migration blocker: use Effect.async / Effect.tryPromise instead of new Promise." + }, + { + selector: "CallExpression[callee.object.name='Promise']", + message: "Effect migration blocker: use Effect combinators instead of Promise.*." + } +] + +export default tseslint.config({ + name: "docker-git-session-sync-effect-ts-compliance", + files: ["src/**/*.ts", "tests/**/*.ts"], + languageOptions: { + parser: tseslint.parser, + globals: { ...globals.node } + }, + plugins: { + "@typescript-eslint": tseslint.plugin, + "eslint-comments": eslintComments + }, + rules: { + "@typescript-eslint/ban-ts-comment": ["error", { + "ts-check": false, + "ts-expect-error": true, + "ts-ignore": true, + "ts-nocheck": true + }], + "@typescript-eslint/no-explicit-any": "error", + "@typescript-eslint/no-restricted-types": ["error", { + types: { + Promise: { + message: "Avoid Promise in public types. Use Effect.Effect." + }, + "Promise<*>": { + message: "Avoid Promise. Use Effect.Effect." + } + } + }], + "eslint-comments/disable-enable-pair": "error", + "eslint-comments/no-unlimited-disable": "error", + "eslint-comments/no-unused-disable": "error", + "eslint-comments/no-use": "error", + "no-console": "error", + "no-restricted-syntax": ["warn", ...effectMigrationWarnings] + } +}) diff --git a/packages/docker-git-session-sync/package.json b/packages/docker-git-session-sync/package.json index 77537899..c91fcd6d 100644 --- a/packages/docker-git-session-sync/package.json +++ b/packages/docker-git-session-sync/package.json @@ -12,6 +12,7 @@ "scripts": { "build": "vite build && bun ../../scripts/mark-executable.mjs dist/docker-git-session-sync.js", "check": "bun run typecheck", + "lint:effect": "eslint --config eslint.effect-ts-check.config.mjs .", "prepack": "bun run build", "test": "vitest run --passWithNoTests", "typecheck": "tsc --noEmit -p tsconfig.json" @@ -36,11 +37,21 @@ }, "homepage": "https://github.com/ProverCoderAI/docker-git#readme", "packageManager": "bun@1.3.11", + "dependencies": { + "@effect/platform": "^0.96.1", + "@effect/platform-node": "^0.107.0", + "@effect/schema": "^0.75.5", + "effect": "^3.21.3" + }, "devDependencies": { "@effect/vitest": "^0.29.0", + "@eslint-community/eslint-plugin-eslint-comments": "^4.7.2", "@types/node": "^25.9.3", "@vitejs/plugin-react": "^6.0.2", + "eslint": "^10.5.0", + "globals": "^17.6.0", "typescript": "^6.0.3", + "typescript-eslint": "^8.61.1", "vite": "^8.0.16", "vitest": "^4.1.9" } diff --git a/packages/docker-git-session-sync/src/backup.ts b/packages/docker-git-session-sync/src/backup.ts index cec8e619..e45b1e91 100644 --- a/packages/docker-git-session-sync/src/backup.ts +++ b/packages/docker-git-session-sync/src/backup.ts @@ -31,7 +31,12 @@ import { updatePrComment, uploadSnapshot } from "./shell.js" -import { errorMessage, isRecord, numberField, recordField, stringField } from "./json.js" +import { errorMessage } from "./json.js" +import { + decodeBackgroundReadyState, + decodeSessionUploadContext, + type BackgroundReadyState +} from "./schemas.js" import type { GhEnv, Log, PrComment, SessionFile, SourceInfo, UploadEntry } from "./types.js" export interface BackupOptions { @@ -325,7 +330,7 @@ export const collectSessionFiles = (dirPath: string, baseName: string, verbose: type PrContext = { readonly repo: string; readonly prNumber: number } -type PrCommentContext = { +export type PrCommentContext = { readonly repo: string readonly comment: PrComment } @@ -348,75 +353,8 @@ export type SessionUploadContext = { readonly verbose: boolean } -const nullableStringField = (value: unknown, key: string): string | null | undefined => { - if (!isRecord(value)) { - return undefined - } - const field = value[key] - return typeof field === "string" || field === null ? field : undefined -} - -const nullableNumberField = (value: unknown, key: string): number | null | undefined => { - if (!isRecord(value)) { - return undefined - } - const field = value[key] - return typeof field === "number" || field === null ? field : undefined -} - -const booleanField = (value: unknown, key: string): boolean | null => { - if (!isRecord(value)) { - return null - } - const field = value[key] - return typeof field === "boolean" ? field : null -} - -const parseSourceInfo = (value: unknown): SourceInfo | null => { - const repo = stringField(value, "repo") - const branch = stringField(value, "branch") - const prNumber = nullableNumberField(value, "prNumber") - const commitSha = stringField(value, "commitSha") - const createdAt = stringField(value, "createdAt") - return repo === null || branch === null || prNumber === undefined || commitSha === null || createdAt === null - ? null - : { repo, branch, prNumber, commitSha, createdAt } -} - -const parsePrCommentContext = (value: unknown): PrCommentContext | null => { - if (value === null) { - return null - } - const repo = stringField(value, "repo") - const comment = recordField(value, "comment") - const id = numberField(comment, "id") - const url = stringField(comment, "url") - return repo === null || id === null || url === null ? null : { repo, comment: { id, url } } -} - -export const parseUploadContext = (value: unknown): SessionUploadContext | null => { - const version = numberField(value, "version") - const cwd = stringField(value, "cwd") - const sessionDir = nullableStringField(value, "sessionDir") - const source = parseSourceInfo(recordField(value, "source")) - const snapshotRef = stringField(value, "snapshotRef") - const gitStatus = nullableStringField(value, "gitStatus") - const prComment = parsePrCommentContext(isRecord(value) ? value["prComment"] : undefined) - const verbose = booleanField(value, "verbose") - if ( - version !== 1 || - cwd === null || - sessionDir === undefined || - source === null || - snapshotRef === null || - gitStatus === undefined || - prComment === null && isRecord(value) && value["prComment"] !== null || - verbose === null - ) { - return null - } - return { version, cwd, sessionDir, source, snapshotRef, gitStatus, prComment, verbose } -} +export const parseUploadContext = (value: unknown): SessionUploadContext | null => + decodeSessionUploadContext(value) const resolveBackupContext = ( options: BackupOptions, @@ -630,10 +568,6 @@ const currentEntrypointPath = (): string | null => { return entrypoint === undefined || entrypoint.length === 0 ? null : entrypoint } -type BackgroundReadyState = - | { readonly state: "started" } - | { readonly state: "failed"; readonly message: string } - const backgroundReadyTimeoutMs = 10_000 const backgroundReadyPollMs = 50 @@ -653,21 +587,9 @@ const writeBackgroundReadyState = (readyFilePath: string | null, state: Backgrou } } -const parseBackgroundReadyState = (value: unknown): BackgroundReadyState | null => { - const state = stringField(value, "state") - if (state === "started") { - return { state } - } - if (state === "failed") { - const message = stringField(value, "message") - return message === null ? null : { state, message } - } - return null -} - const readBackgroundReadyState = (readyFilePath: string): BackgroundReadyState | null => { try { - return parseBackgroundReadyState(JSON.parse(fs.readFileSync(readyFilePath, "utf8"))) + return decodeBackgroundReadyState(JSON.parse(fs.readFileSync(readyFilePath, "utf8"))) } catch { return null } diff --git a/packages/docker-git-session-sync/src/main.ts b/packages/docker-git-session-sync/src/main.ts index 96086d16..12fba0da 100644 --- a/packages/docker-git-session-sync/src/main.ts +++ b/packages/docker-git-session-sync/src/main.ts @@ -1,12 +1,39 @@ +import { NodeContext, NodeRuntime } from "@effect/platform-node" +import { Effect, pipe } from "effect" + import { runCli } from "./cli.js" +import { errorMessage } from "./json.js" + +// CHANGE: Run the CLI entrypoint through the Effect Node runtime. +// WHY: Establish a controlled SHELL boundary while preserving the existing synchronous CLI behavior. +// QUOTE(ТЗ): "entrypoint через Effect runtime" +// REF: user-request-2026-06-17-effect-compliance-session-sync +// SOURCE: n/a +// FORMAT THEOREM: runCli(args,cwd)=0 -> exitCode unchanged; runCli(args,cwd)=n>0 -> process.exitCode=n; throws(e) -> stderr=e ∧ exitCode=1 +// PURITY: SHELL +// EFFECT: Effect +// INVARIANT: CLI observable exit code and stderr semantics are preserved. +// COMPLEXITY: O(n)/O(1), where n is delegated CLI work. +const main = pipe( + Effect.try({ + try: () => runCli(process.argv.slice(2), process.cwd()), + catch: errorMessage + }), + Effect.match({ + onFailure: (message) => + Effect.sync(() => { + process.stderr.write(`${message}\n`) + process.exitCode = 1 + }), + onSuccess: (exitCode) => + Effect.sync(() => { + if (exitCode !== 0) { + process.exitCode = exitCode + } + }) + }), + Effect.flatten, + Effect.provide(NodeContext.layer) +) -try { - const exitCode = runCli(process.argv.slice(2), process.cwd()) - if (exitCode !== 0) { - process.exitCode = exitCode - } -} catch (error) { - const message = error instanceof Error ? error.message : String(error) - process.stderr.write(`${message}\n`) - process.exitCode = 1 -} +NodeRuntime.runMain(main) diff --git a/packages/docker-git-session-sync/src/schemas.ts b/packages/docker-git-session-sync/src/schemas.ts new file mode 100644 index 00000000..c900490a --- /dev/null +++ b/packages/docker-git-session-sync/src/schemas.ts @@ -0,0 +1,155 @@ +import * as ParseResult from "@effect/schema/ParseResult" +import * as Schema from "@effect/schema/Schema" +import { Either } from "effect" + +import type { PrCommentContext, SessionUploadContext } from "./backup.js" +import type { PrComment, TreeEntry } from "./types.js" + +const NullableString = Schema.NullOr(Schema.String) +const NullableNumber = Schema.NullOr(Schema.Number) + +const SourceInfoSchema = Schema.Struct({ + repo: Schema.String, + branch: Schema.String, + prNumber: NullableNumber, + commitSha: Schema.String, + createdAt: Schema.String +}) + +const PrCommentSchema: Schema.Schema = Schema.Struct({ + id: Schema.Number, + url: Schema.String +}) + +const PrCommentContextSchema: Schema.Schema = Schema.Struct({ + repo: Schema.String, + comment: PrCommentSchema +}) + +const SessionUploadContextSchema: Schema.Schema = Schema.Struct({ + version: Schema.Literal(1), + cwd: Schema.String, + sessionDir: NullableString, + source: SourceInfoSchema, + snapshotRef: Schema.String, + gitStatus: NullableString, + prComment: Schema.NullOr(PrCommentContextSchema), + verbose: Schema.Boolean +}) + +const BackgroundReadyStateSchema = Schema.Union( + Schema.Struct({ + state: Schema.Literal("started") + }), + Schema.Struct({ + state: Schema.Literal("failed"), + message: Schema.String + }) +) + +export type BackgroundReadyState = Schema.Schema.Type + +const GitHubRepoInfoSchema = Schema.Struct({ + default_branch: Schema.optional(Schema.NullOr(Schema.String)), + html_url: Schema.optional(Schema.NullOr(Schema.String)) +}) + +export type GitHubRepoInfo = { + readonly defaultBranch: string | null + readonly htmlUrl: string | null +} + +const GitHubPrCommentResponseSchema = Schema.Struct({ + id: Schema.Number, + html_url: Schema.String +}) + +const GitHubContentResponseSchema = Schema.Struct({ + encoding: Schema.String, + content: Schema.String +}) + +export type GitHubContentResponse = Schema.Schema.Type + +const GitHubShaResponseSchema = Schema.Struct({ + sha: Schema.String +}) + +const TreeEntrySchema: Schema.Schema = Schema.Struct({ + path: Schema.String, + mode: Schema.String, + type: Schema.String, + sha: Schema.String +}) + +const GitHubTreeResponseSchema = Schema.Struct({ + tree: Schema.Array(Schema.Unknown) +}) + +// CHANGE: Decode background upload context at the JSON boundary with @effect/schema. +// WHY: Once decoded, upload code receives a typed context instead of ad hoc unknown field probes. +// QUOTE(ТЗ): "заменить самые явные try/catch/unknown JSON boundaries на typed Schema декодирование" +// REF: user-request-2026-06-17-effect-compliance-session-sync +// SOURCE: n/a +// FORMAT THEOREM: decode(x)=ctx -> ctx.version=1 ∧ ctx.source.repo∈String ∧ ctx.verbose∈Boolean +// PURITY: SHELL +// EFFECT: none +// INVARIANT: invalid or incomplete context decodes to null. +// COMPLEXITY: O(n)/O(n), where n is encoded field count. +export const decodeSessionUploadContext = (value: unknown): SessionUploadContext | null => + Either.match(ParseResult.decodeUnknownEither(SessionUploadContextSchema)(value), { + onLeft: () => null, + onRight: (context) => context + }) + +export const decodeBackgroundReadyState = (value: unknown): BackgroundReadyState | null => + Either.match(ParseResult.decodeUnknownEither(BackgroundReadyStateSchema)(value), { + onLeft: () => null, + onRight: (state) => state + }) + +export const decodeGitHubRepoInfo = (value: unknown): GitHubRepoInfo | null => + Either.match(ParseResult.decodeUnknownEither(GitHubRepoInfoSchema)(value), { + onLeft: () => null, + onRight: (repo) => ({ + defaultBranch: repo.default_branch ?? null, + htmlUrl: repo.html_url ?? null + }) + }) + +export const decodeGitHubPrComment = (value: unknown): PrComment | null => + Either.match(ParseResult.decodeUnknownEither(GitHubPrCommentResponseSchema)(value), { + onLeft: () => null, + onRight: (comment) => ({ + id: comment.id, + url: comment.html_url + }) + }) + +export const decodeGitHubContentResponse = (value: unknown): GitHubContentResponse | null => + Either.match(ParseResult.decodeUnknownEither(GitHubContentResponseSchema)(value), { + onLeft: () => null, + onRight: (content) => content + }) + +export const decodeGitHubSha = (value: unknown): string | null => + Either.match(ParseResult.decodeUnknownEither(GitHubShaResponseSchema)(value), { + onLeft: () => null, + onRight: (response) => response.sha + }) + +const decodeTreeEntry = (value: unknown): TreeEntry | null => + Either.match(ParseResult.decodeUnknownEither(TreeEntrySchema)(value), { + onLeft: () => null, + onRight: (entry) => entry + }) + +export const decodeGitHubTreeEntries = (value: unknown): ReadonlyArray => + Either.match(ParseResult.decodeUnknownEither(GitHubTreeResponseSchema)(value), { + onLeft: () => [], + onRight: (response) => + response.tree.flatMap((entry) => { + const decoded = decodeTreeEntry(entry) + return decoded === null ? [] : [decoded] + }) + }) diff --git a/packages/docker-git-session-sync/src/shell.ts b/packages/docker-git-session-sync/src/shell.ts index 712e2cc2..624ac29d 100644 --- a/packages/docker-git-session-sync/src/shell.ts +++ b/packages/docker-git-session-sync/src/shell.ts @@ -15,7 +15,14 @@ import { githubEnvKeys, maxRepoFileSize } from "./core.js" -import { arrayField, errorMessage, isRecord, numberField, recordField, stringField } from "./json.js" +import { errorMessage, recordField, stringField } from "./json.js" +import { + decodeGitHubContentResponse, + decodeGitHubPrComment, + decodeGitHubRepoInfo, + decodeGitHubSha, + decodeGitHubTreeEntries +} from "./schemas.js" import type { BackupRepo, GhEnv, @@ -166,12 +173,16 @@ export const ensureBackupRepo = (ghEnv: GhEnv, log: Log, createIfMissing: boolea if (!repoResult.success || repoResult.json === null) { return null } + const repoInfo = decodeGitHubRepoInfo(repoResult.json) + if (repoInfo === null) { + return null + } return { owner: login, repo: backupRepoName, fullName: repoFullName, - defaultBranch: stringField(repoResult.json, "default_branch") ?? backupDefaultBranch, - htmlUrl: stringField(repoResult.json, "html_url") ?? `https://github.com/${repoFullName}` + defaultBranch: repoInfo.defaultBranch ?? backupDefaultBranch, + htmlUrl: repoInfo.htmlUrl ?? `https://github.com/${repoFullName}` } } @@ -187,18 +198,6 @@ const getCommitTreeSha = (repoFullName: string, commitSha: string, ghEnv: GhEnv) `failed to resolve tree for commit ${commitSha}` ).stdout -const isTreeEntry = (value: unknown): value is TreeEntry => { - if (!isRecord(value)) { - return false - } - return ( - typeof value["path"] === "string" && - typeof value["mode"] === "string" && - typeof value["type"] === "string" && - typeof value["sha"] === "string" - ) -} - export const getTreeEntries = (repoFullName: string, branch: string, ghEnv: GhEnv): TreeSnapshot => { const headSha = getBranchHeadSha(repoFullName, branch, ghEnv) const treeSha = getCommitTreeSha(repoFullName, headSha, ghEnv) @@ -209,7 +208,7 @@ export const getTreeEntries = (repoFullName: string, branch: string, ghEnv: GhEn return { headSha, treeSha, - entries: arrayField(result.json, "tree").filter(isTreeEntry) + entries: decodeGitHubTreeEntries(result.json) } } @@ -223,20 +222,14 @@ export const getFileContent = ( ghApiJson(`/repos/${repoFullName}/contents/${repoPath}?ref=${encodeURIComponent(ref)}`, ghEnv), `failed to fetch ${repoFullName}:${repoPath}` ) - const encoding = stringField(result.json, "encoding") - const content = stringField(result.json, "content")?.replace(/\n/gu, "") ?? "" - if (encoding !== "base64" || content.length === 0) { + const contentResponse = decodeGitHubContentResponse(result.json) + const content = contentResponse?.content.replace(/\n/gu, "") ?? "" + if (contentResponse?.encoding !== "base64" || content.length === 0) { throw new Error(`unexpected content payload for ${repoFullName}:${repoPath}`) } return Buffer.from(content, "base64") } -const parsePrComment = (value: unknown): PrComment | null => { - const id = numberField(value, "id") - const url = stringField(value, "html_url") - return id === null || url === null ? null : { id, url } -} - export const createPrComment = ( repoFullName: string, prNumber: number, @@ -247,7 +240,7 @@ export const createPrComment = ( method: "POST", body: { body } }) - return result.success ? parsePrComment(result.json) : null + return result.success ? decodeGitHubPrComment(result.json) : null } export const updatePrComment = ( @@ -545,7 +538,7 @@ const createGitBlob = (repoFullName: string, entry: UploadEntry, ghEnv: GhEnv): }), `failed to create blob for ${repoFullName}:${entry.repoPath}` ) - const sha = stringField(result.json, "sha") + const sha = decodeGitHubSha(result.json) if (sha === null) { throw new Error(`GitHub blob response missing sha for ${entry.repoPath}`) } @@ -571,7 +564,7 @@ const createGitTree = ( }), `failed to create tree in ${repoFullName}` ) - const sha = stringField(result.json, "sha") + const sha = decodeGitHubSha(result.json) if (sha === null) { throw new Error(`GitHub tree response missing sha for ${repoFullName}`) } @@ -603,7 +596,7 @@ const createGitCommit = ( }), `failed to create commit in ${backupRepo.fullName}` ) - const sha = stringField(result.json, "sha") + const sha = decodeGitHubSha(result.json) if (sha === null) { throw new Error(`GitHub commit response missing sha for ${backupRepo.fullName}`) } diff --git a/packages/docker-git-session-sync/tests/session-files.test.ts b/packages/docker-git-session-sync/tests/session-files.test.ts index e0cad180..448e5166 100644 --- a/packages/docker-git-session-sync/tests/session-files.test.ts +++ b/packages/docker-git-session-sync/tests/session-files.test.ts @@ -330,6 +330,31 @@ describe("CLI parser", () => { expect(parsed?.prComment?.comment.id).toBe(1001) }) + it("rejects malformed background upload context metadata", () => { + expect(parseUploadContext({ + version: 1, + cwd: "/workspace", + sessionDir: null, + source: { + repo: "org/repo", + branch: "issue-230", + prNumber: 230, + commitSha: "0123456789abcdef", + createdAt: "2026-04-27T00:00:00.000Z" + }, + snapshotRef: "org/repo/pr-230/current", + gitStatus: "dirty", + prComment: { + repo: "org/repo", + comment: { + id: "1001", + url: "https://example.test/comment" + } + }, + verbose: true + })).toBeNull() + }) + it("rejects missing snapshot refs", () => { expect(parseArgs(["view"])).toEqual({ _tag: "Error", message: "view requires " }) expect(parseArgs(["download"])).toEqual({ _tag: "Error", message: "download requires " }) diff --git a/packages/lib/src/core/command-builders-shared.ts b/packages/lib/src/core/command-builders-shared.ts index 9d399321..66f950e2 100644 --- a/packages/lib/src/core/command-builders-shared.ts +++ b/packages/lib/src/core/command-builders-shared.ts @@ -69,6 +69,29 @@ export const trimTrailingPathSeparators = (value: string): string => { return value.slice(0, end) } +/** + * Expands POSIX home shorthand for paths inside the generated project container. + * + * @param sshUser - Validated container user name. + * @param value - Raw target path candidate. + * @returns The path with `~` expanded to `/home/${sshUser}`. + * @pure true + * @effect none; CORE helper only transforms provided strings. + * @invariant result is a deterministic function of `(sshUser, value)`. + * @precondition sshUser was validated by parseSshUser. + * @postcondition `~` and `~/x` no longer contain home shorthand. + * @complexity O(n) time / O(n) space where n = |value|. + */ +export const expandContainerHome = (sshUser: string, value: string): string => { + if (value === "~") { + return `/home/${sshUser}` + } + if (value.startsWith("~/")) { + return `/home/${sshUser}${value.slice(1)}` + } + return value +} + /** * Parses a raw SSH port value into the valid Docker host-port range. * diff --git a/packages/lib/src/core/command-builders.ts b/packages/lib/src/core/command-builders.ts index a91974a5..37430751 100644 --- a/packages/lib/src/core/command-builders.ts +++ b/packages/lib/src/core/command-builders.ts @@ -1,9 +1,8 @@ import { Either } from "effect" -import { hostname } from "node:os" -import { expandContainerHome } from "../usecases/scrap-path.js" import { resolveAutoAgentFlags } from "./auto-agent-flags.js" import { + expandContainerHome, nonEmpty, parseDockerNetworkMode, parseGpuMode, @@ -270,7 +269,7 @@ export const buildCreateCommand = ( enableMcpPlaywright: behavior.enableMcpPlaywright, agentMode, agentAuto: isAgentAuto, - clonedOnHostname: hostname() + clonedOnHostname: raw.clonedOnHostname }) } }) diff --git a/packages/lib/src/core/command-options.ts b/packages/lib/src/core/command-options.ts index 8e445d10..8ba09533 100644 --- a/packages/lib/src/core/command-options.ts +++ b/packages/lib/src/core/command-options.ts @@ -59,6 +59,7 @@ export interface RawOptions { readonly force?: boolean readonly forceEnv?: boolean readonly agentAutoMode?: string + readonly clonedOnHostname?: string // Session gist options (issue-143) readonly prNumber?: string readonly repo?: string diff --git a/packages/lib/src/usecases/scrap-path.ts b/packages/lib/src/usecases/scrap-path.ts index 9352f566..4ec935d6 100644 --- a/packages/lib/src/usecases/scrap-path.ts +++ b/packages/lib/src/usecases/scrap-path.ts @@ -1,19 +1,10 @@ import { Either } from "effect" +import { expandContainerHome } from "../core/command-builders-shared.js" import { ScrapTargetDirUnsupportedError } from "../shell/errors.js" const normalizeContainerPath = (value: string): string => value.replaceAll("\\", "/").trim() -export const expandContainerHome = (sshUser: string, value: string): string => { - if (value === "~") { - return `/home/${sshUser}` - } - if (value.startsWith("~/")) { - return `/home/${sshUser}${value.slice(1)}` - } - return value -} - const trimTrailingPosixSlashes = (value: string): string => { let end = value.length while (end > 0 && value[end - 1] === "/") { @@ -68,3 +59,5 @@ export const deriveScrapWorkspaceRelativePath = ( return Either.right(relative) } + +export { expandContainerHome } from "../core/command-builders-shared.js" From 24dac9327feccf6fb99492518b75be7399ba5a21 Mon Sep 17 00:00:00 2001 From: skulidropek <66840575+skulidropek@users.noreply.github.com> Date: Wed, 17 Jun 2026 19:35:45 +0000 Subject: [PATCH 2/3] fix(effect): address PR review compliance findings - Validate clonedOnHostname at API/config boundaries and keep command builders pure by passing host identity explicitly. - Serialize terminal session state mutations per project to prevent stale read/write races. - Harden session-sync GitHub decoders, readiness polling, PR comment logging, and regression/property tests. Proof of fix: - Cause: review found weak schema contracts, unsynchronized durable terminal writes, silent best-effort failures, and incomplete regression tests. - Solution: moved shared Effect lint rules into one module, strengthened decoders and hostname schemas, added project-scoped semaphores around state writers, and added focused unit/property tests. - Verification: api test/typecheck/lint, root typecheck/test/lint, session-sync typecheck/test, lint:effect, and git diff --check passed locally. --- bun.lock | 1 + eslint.effect-ts-shared.mjs | 37 +++++ .../api/eslint.effect-ts-check.config.mjs | 40 +----- packages/api/src/api/contracts.ts | 17 ++- packages/api/src/api/schema.ts | 7 +- packages/api/src/http.ts | 6 +- packages/api/src/services/agents.ts | 70 ++++++--- packages/api/src/services/projects.ts | 4 +- .../api/src/services/terminal-sessions.ts | 99 +++++-------- packages/api/tests/agents.test.ts | 19 +-- packages/api/tests/schema.test.ts | 13 ++ packages/api/tests/terminal-sessions.test.ts | 17 +++ .../frontend-lib/core/command-builders.ts | 9 ++ .../frontend-lib/core/command-options.ts | 1 + .../eslint.effect-ts-check.config.mjs | 38 +---- packages/docker-git-session-sync/package.json | 1 + .../docker-git-session-sync/src/backup.ts | 59 +++++--- packages/docker-git-session-sync/src/main.ts | 2 +- .../docker-git-session-sync/src/schemas.ts | 123 +++++++++++++--- packages/docker-git-session-sync/src/shell.ts | 53 ++++--- .../tests/session-files.test.ts | 136 ++++++++++++++++++ packages/lib/src/core/command-builders.ts | 9 ++ packages/lib/src/core/command-options.ts | 1 + packages/lib/src/shell/config.ts | 10 +- 24 files changed, 537 insertions(+), 235 deletions(-) create mode 100644 eslint.effect-ts-shared.mjs diff --git a/bun.lock b/bun.lock index 7d3e9eb2..53f655b2 100644 --- a/bun.lock +++ b/bun.lock @@ -169,6 +169,7 @@ "@types/node": "^25.9.3", "@vitejs/plugin-react": "^6.0.2", "eslint": "^10.5.0", + "fast-check": "4.8.0", "globals": "^17.6.0", "typescript": "^6.0.3", "typescript-eslint": "^8.61.1", diff --git a/eslint.effect-ts-shared.mjs b/eslint.effect-ts-shared.mjs new file mode 100644 index 00000000..ed5c37af --- /dev/null +++ b/eslint.effect-ts-shared.mjs @@ -0,0 +1,37 @@ +export const effectMigrationWarnings = [ + { + selector: "SwitchStatement", + message: "Effect migration blocker: use Match.exhaustive instead of switch." + }, + { + selector: "TryStatement", + message: "Effect migration blocker: use Effect.try / Effect.catch* instead of try/catch." + }, + { + selector: "AwaitExpression", + message: "Effect migration blocker: use Effect.gen / Effect.flatMap instead of await." + }, + { + selector: "FunctionDeclaration[async=true], FunctionExpression[async=true], ArrowFunctionExpression[async=true]", + message: "Effect migration blocker: use Effect.gen / Effect.tryPromise instead of async functions." + }, + { + selector: "NewExpression[callee.name='Promise']", + message: "Effect migration blocker: use Effect.async / Effect.tryPromise instead of new Promise." + }, + { + selector: "CallExpression[callee.object.name='Promise']", + message: "Effect migration blocker: use Effect combinators instead of Promise.*." + } +] + +export const effectPromiseRestrictedTypes = { + types: { + Promise: { + message: "Effect migration blocker: avoid Promise in public types. Use Effect.Effect." + }, + "Promise<*>": { + message: "Effect migration blocker: avoid Promise. Use Effect.Effect." + } + } +} diff --git a/packages/api/eslint.effect-ts-check.config.mjs b/packages/api/eslint.effect-ts-check.config.mjs index 3bc0d6ed..9b7f70b8 100644 --- a/packages/api/eslint.effect-ts-check.config.mjs +++ b/packages/api/eslint.effect-ts-check.config.mjs @@ -12,32 +12,7 @@ import eslintComments from "@eslint-community/eslint-plugin-eslint-comments" import globals from "globals" import tseslint from "typescript-eslint" -const migrationBlockers = [ - { - selector: "SwitchStatement", - message: "Effect migration blocker: use Match.exhaustive instead of switch." - }, - { - selector: "TryStatement", - message: "Effect migration blocker: use Effect.try / Effect.catch* instead of try/catch." - }, - { - selector: "AwaitExpression", - message: "Effect migration blocker: use Effect.gen / Effect.flatMap instead of await." - }, - { - selector: "FunctionDeclaration[async=true], FunctionExpression[async=true], ArrowFunctionExpression[async=true]", - message: "Effect migration blocker: use Effect.gen / Effect.tryPromise instead of async functions." - }, - { - selector: "NewExpression[callee.name='Promise']", - message: "Effect migration blocker: use Effect.async / Effect.tryPromise instead of new Promise." - }, - { - selector: "CallExpression[callee.object.name='Promise']", - message: "Effect migration blocker: use Effect combinators instead of Promise.*." - } -] +import { effectMigrationWarnings, effectPromiseRestrictedTypes } from "../../eslint.effect-ts-shared.mjs" export default tseslint.config({ name: "api-effect-ts-compliance", @@ -58,21 +33,12 @@ export default tseslint.config({ "ts-nocheck": true }], "@typescript-eslint/no-explicit-any": "error", - "@typescript-eslint/no-restricted-types": ["warn", { - types: { - Promise: { - message: "Avoid Promise in public types. Use Effect.Effect." - }, - "Promise<*>": { - message: "Avoid Promise. Use Effect.Effect." - } - } - }], + "@typescript-eslint/no-restricted-types": ["warn", effectPromiseRestrictedTypes], "eslint-comments/disable-enable-pair": "error", "eslint-comments/no-unlimited-disable": "error", "eslint-comments/no-unused-disable": "error", "eslint-comments/no-use": "error", "no-console": "error", - "no-restricted-syntax": ["warn", ...migrationBlockers] + "no-restricted-syntax": ["warn", ...effectMigrationWarnings] } }) diff --git a/packages/api/src/api/contracts.ts b/packages/api/src/api/contracts.ts index 187bcb09..8d9dc3ca 100644 --- a/packages/api/src/api/contracts.ts +++ b/packages/api/src/api/contracts.ts @@ -26,7 +26,7 @@ export type ProjectSummary = { readonly sshSessions: number readonly startedAtIso: string | null readonly startedAtEpochMs: number | null - readonly clonedOnHostname?: string | undefined + readonly clonedOnHostname?: string } export type ProjectDetails = ProjectSummary & { @@ -480,7 +480,20 @@ export type CreateProjectRequest = { readonly forceEnv?: boolean | undefined readonly waitForClone?: boolean | undefined readonly async?: boolean | undefined - readonly clonedOnHostname?: string | undefined + /** + * Hostname of the machine where the project was cloned. + * + * CHANGE: add explicit clone-origin hostname to the create-project contract. + * WHY: keeps command builders pure by passing host identity as immutable input instead of reading OS state. + * QUOTE(ТЗ): "CORE: Исключительно чистые функции, неизменяемые данные, математические операции" + * REF: pr-420-coderabbit-review-4518791377 + * SOURCE: n/a + * FORMAT THEOREM: ∀h ∈ Hostname: request(h) -> build(request).config.clonedOnHostname = h + * PURITY: CORE - immutable request data. + * INVARIANT: if present, value satisfies API HostnameSchema. + * COMPLEXITY: O(1)/O(1) + */ + readonly clonedOnHostname?: string } export type AgentEnvVar = { diff --git a/packages/api/src/api/schema.ts b/packages/api/src/api/schema.ts index 7662278d..6412840b 100644 --- a/packages/api/src/api/schema.ts +++ b/packages/api/src/api/schema.ts @@ -12,6 +12,11 @@ export { const OptionalString = Schema.optional(Schema.String) const OptionalBoolean = Schema.optional(Schema.Boolean) const OptionalNullableString = Schema.optional(Schema.NullOr(Schema.String)) +const HostnameSchema = Schema.String.pipe( + Schema.minLength(1), + Schema.maxLength(253), + Schema.pattern(/^(?:[A-Za-z0-9](?:[A-Za-z0-9-]{0,61}[A-Za-z0-9])?)(?:\.[A-Za-z0-9](?:[A-Za-z0-9-]{0,61}[A-Za-z0-9])?)*$/u) +) export const CreateProjectRequestSchema = Schema.Struct({ repoUrl: OptionalString, @@ -52,7 +57,7 @@ export const CreateProjectRequestSchema = Schema.Struct({ forceEnv: OptionalBoolean, waitForClone: OptionalBoolean, async: OptionalBoolean, - clonedOnHostname: OptionalString + clonedOnHostname: Schema.optional(HostnameSchema) }) export const GithubAuthLoginRequestSchema = Schema.Struct({ diff --git a/packages/api/src/http.ts b/packages/api/src/http.ts index 2d5d9100..0118efac 100644 --- a/packages/api/src/http.ts +++ b/packages/api/src/http.ts @@ -1426,7 +1426,11 @@ export const makeRouter = () => { "/projects", Effect.gen(function*(_) { const request = yield* _(readCreateProjectRequest()) - const result = yield* _(createProjectFromRequest(request)) + const { clonedOnHostname, ...requestWithoutCloneHost } = request + const result = yield* _(createProjectFromRequest({ + ...requestWithoutCloneHost, + ...(clonedOnHostname === undefined ? {} : { clonedOnHostname }) + })) return yield* _( "accepted" in result && result.accepted === true ? jsonResponse(result, 202) diff --git a/packages/api/src/services/agents.ts b/packages/api/src/services/agents.ts index 158d25f5..2cc6bfec 100644 --- a/packages/api/src/services/agents.ts +++ b/packages/api/src/services/agents.ts @@ -35,7 +35,7 @@ type SnapshotFile = { readonly sessions: ReadonlyArray } -const SnapshotFileSchema = Schema.Struct({ +const SnapshotFileSchema: Schema.Schema = Schema.Struct({ sessions: Schema.Array(AgentSessionSchema) }) @@ -46,16 +46,43 @@ const projectIndex: Map> = new Map() const maxLogLines = 5000 let initialized = false -export const clearAgentRuntimeForTest = (): void => { - for (const record of records.values()) { - if (record.process !== null && !record.process.killed) { - record.process.kill("SIGTERM") +const waitForProcessExit = (child: ChildProcess): Effect.Effect => + Effect.async((resume) => { + const finish = (): void => { + clearTimeout(timeout) + child.off("exit", finish) + child.off("close", finish) + resume(Effect.void) } - } - records.clear() - projectIndex.clear() - initialized = false -} + const timeout = setTimeout(finish, 1_000) + if (child.exitCode !== null || child.signalCode !== null) { + finish() + return + } + child.once("exit", finish) + child.once("close", finish) + }) + +export const clearAgentRuntimeForTest = (): Effect.Effect => + Effect.gen(function*(_) { + const children = [...records.values()] + .map((record) => record.process) + .filter((process): process is ChildProcess => process !== null) + + for (const child of children) { + if (!child.killed) { + child.kill("SIGTERM") + } + } + + yield* _(Effect.forEach(children, waitForProcessExit, { + concurrency: "unbounded", + discard: true + })) + records.clear() + projectIndex.clear() + initialized = false + }) const nowIso = (): string => new Date().toISOString() @@ -229,7 +256,9 @@ const persistSnapshot = (): Effect.Effect { Effect.runFork(persistSnapshot().pipe( Effect.provide(NodeContext.layer), - Effect.catchAll(() => Effect.void) + Effect.catchAll((error) => + Effect.logWarning(`[agents] Failed to persist snapshot: ${String(error)}`) + ) )) } @@ -359,19 +388,16 @@ const readSnapshotFile = (): Effect.Effect false, - onRight: (value) => value - }) + const fileExists = yield* _(fs.exists(filePath).pipe( + Effect.orElse(() => Effect.succeed(false)) + )) if (!fileExists) { return emptySnapshotFile() } - const contents = yield* _(Effect.either(fs.readFileString(filePath))) - return Either.match(contents, { - onLeft: () => emptySnapshotFile(), - onRight: (value) => decodeSnapshotFile(value) ?? emptySnapshotFile() - }) + const contents = yield* _(fs.readFileString(filePath).pipe( + Effect.orElse(() => Effect.succeed("")) + )) + return decodeSnapshotFile(contents) ?? emptySnapshotFile() }).pipe(Effect.catchAll(() => Effect.succeed(emptySnapshotFile()))) const hydrateFromSnapshot = (): Effect.Effect => @@ -386,6 +412,8 @@ const hydrateFromSnapshot = (): Effect.Effect -type TerminalSessionPersistenceQueue = { - readonly items: ReadonlyArray - readonly running: boolean -} - type DurableTerminalSession = { readonly id: string readonly projectId: string @@ -110,7 +105,7 @@ type DurableTerminalSessionFile = { } const records = new Map() -const terminalSessionPersistenceQueues = new Map() +const terminalSessionStateLocks = new Map() const terminalActivityWrites = new Map() const terminalWsPathPattern = /^(?:\/api)?\/projects\/([^/]+)\/terminal-sessions\/([^/]+)\/ws$/u const terminalWsByKeyPathPattern = /^(?:\/api)?\/projects\/by-key\/([^/]+)\/terminal-sessions\/([^/]+)\/ws$/u @@ -155,7 +150,7 @@ export const clearTerminalSessionRuntimeForTest = (): void => { closeRecordSockets(record) } records.clear() - terminalSessionPersistenceQueues.clear() + terminalSessionStateLocks.clear() terminalActivityWrites.clear() } @@ -298,6 +293,22 @@ const durableFromSession = ( ...(args.session.closedAt === undefined ? {} : { closedAt: args.session.closedAt }) }) +const terminalSessionStateLock = (projectId: string): Effect.Semaphore => { + const existing = terminalSessionStateLocks.get(projectId) + if (existing !== undefined) { + return existing + } + const lock = Effect.unsafeMakeSemaphore(1) + terminalSessionStateLocks.set(projectId, lock) + return lock +} + +const serializeTerminalSessionState = ( + projectId: string, + effect: Effect.Effect +): Effect.Effect => + terminalSessionStateLock(projectId).withPermits(1)(effect) + const upsertDurableSession = ( projectId: string, durable: DurableTerminalSession, @@ -305,7 +316,7 @@ const upsertDurableSession = ( readonly activate?: boolean } = {} ): Effect.Effect => - Effect.gen(function*(_) { + serializeTerminalSessionState(projectId, Effect.gen(function*(_) { const state = yield* _(readTerminalSessionFile(projectId)) const sessions = state.sessions.filter((session) => session.id !== durable.id) yield* _(writeTerminalSessionFile(projectId, { @@ -313,13 +324,13 @@ const upsertDurableSession = ( schemaVersion: 1, sessions: [...sessions, durable] })) - }) + })) const patchDurableSession = ( record: TerminalRecord, patch: Partial ): Effect.Effect => - Effect.gen(function*(_) { + serializeTerminalSessionState(record.projectId, Effect.gen(function*(_) { const state = yield* _(readTerminalSessionFile(record.projectId)) const updatedAt = nowIso() const sessions = state.sessions.map((session) => @@ -341,13 +352,13 @@ const patchDurableSession = ( schemaVersion: 1, sessions })) - }) + })) const deleteDurableSession = ( projectId: string, sessionId: string ): Effect.Effect => - Effect.gen(function*(_) { + serializeTerminalSessionState(projectId, Effect.gen(function*(_) { const state = yield* _(readTerminalSessionFile(projectId)) const sessions = state.sessions.filter((session) => session.id !== sessionId) if (sessions.length === state.sessions.length) { @@ -361,13 +372,13 @@ const deleteDurableSession = ( sessions })) return true - }) + })) const setActiveDurableSession = ( projectId: string, sessionId: string ): Effect.Effect => - Effect.gen(function*(_) { + serializeTerminalSessionState(projectId, Effect.gen(function*(_) { const state = yield* _(readTerminalSessionFile(projectId)) const durable = state.sessions.find((session) => session.id === sessionId) if (durable === undefined) { @@ -379,7 +390,7 @@ const setActiveDurableSession = ( sessions: state.sessions })) return durable - }) + })) const findDurableSession = ( projectId: string, @@ -389,69 +400,25 @@ const findDurableSession = ( Effect.map((state) => state.sessions.find((session) => session.id === sessionId) ?? null) ) -const drainTerminalSessionPersistenceQueue = (projectId: string): void => { - const queue = terminalSessionPersistenceQueues.get(projectId) - if (queue === undefined || queue.running || queue.items.length === 0) { - return - } - - const [nextEffect, ...remainingItems] = queue.items - if (nextEffect === undefined) { - return - } - - terminalSessionPersistenceQueues.set(projectId, { - items: remainingItems, - running: true - }) +const isAppError = (value: unknown): value is AppError => + typeof value === "object" && value !== null && "_tag" in value +const runTerminalSessionPersistence = ( + projectId: string, + effect: TerminalSessionPersistenceEffect +): void => { Effect.runFork( - nextEffect.pipe( + effect.pipe( Effect.provide(NodeContext.layer), Effect.catchAll((error) => Effect.logWarning( `[terminal-sessions] Failed to persist state for project ${projectId}: ${describeUnknown(error)}` ) - ), - Effect.ensuring( - Effect.sync(() => { - const current = terminalSessionPersistenceQueues.get(projectId) - if (current === undefined) { - return - } - if (current.items.length === 0) { - terminalSessionPersistenceQueues.delete(projectId) - return - } - terminalSessionPersistenceQueues.set(projectId, { - items: current.items, - running: false - }) - drainTerminalSessionPersistenceQueue(projectId) - }) ) ) ) } -const isAppError = (value: unknown): value is AppError => - typeof value === "object" && value !== null && "_tag" in value - -const runTerminalSessionPersistence = ( - projectId: string, - effect: TerminalSessionPersistenceEffect -): void => { - const current = terminalSessionPersistenceQueues.get(projectId) ?? { - items: [], - running: false - } - terminalSessionPersistenceQueues.set(projectId, { - items: [...current.items, effect], - running: current.running - }) - drainTerminalSessionPersistenceQueue(projectId) -} - const updateSession = ( record: TerminalRecord, patch: Partial diff --git a/packages/api/tests/agents.test.ts b/packages/api/tests/agents.test.ts index 2145ac8c..13694c7d 100644 --- a/packages/api/tests/agents.test.ts +++ b/packages/api/tests/agents.test.ts @@ -26,17 +26,20 @@ beforeEach(() => { previousProjectsRoot = process.env["DOCKER_GIT_PROJECTS_ROOT"] projectsRoot = mkdtempSync(path.join(os.tmpdir(), "docker-git-agents-")) process.env["DOCKER_GIT_PROJECTS_ROOT"] = projectsRoot - clearAgentRuntimeForTest() + return Effect.runPromise(clearAgentRuntimeForTest()) }) afterEach(() => { - clearAgentRuntimeForTest() - if (previousProjectsRoot === undefined) { - delete process.env["DOCKER_GIT_PROJECTS_ROOT"] - } else { - process.env["DOCKER_GIT_PROJECTS_ROOT"] = previousProjectsRoot - } - rmSync(projectsRoot, { recursive: true, force: true }) + return Effect.runPromise(clearAgentRuntimeForTest().pipe( + Effect.ensuring(Effect.sync(() => { + if (previousProjectsRoot === undefined) { + delete process.env["DOCKER_GIT_PROJECTS_ROOT"] + } else { + process.env["DOCKER_GIT_PROJECTS_ROOT"] = previousProjectsRoot + } + rmSync(projectsRoot, { recursive: true, force: true }) + })) + )) }) describe("agent service", () => { diff --git a/packages/api/tests/schema.test.ts b/packages/api/tests/schema.test.ts index 3e26801a..fedda00e 100644 --- a/packages/api/tests/schema.test.ts +++ b/packages/api/tests/schema.test.ts @@ -60,6 +60,19 @@ describe("api schemas", () => { }) })) + it.effect("validates create project clonedOnHostname as RFC1123 hostname", () => + Effect.sync(() => { + const valid = Schema.decodeUnknownEither(CreateProjectRequestSchema)({ + clonedOnHostname: "host-01.example.test" + }) + const invalid = Schema.decodeUnknownEither(CreateProjectRequestSchema)({ + clonedOnHostname: "host\ninjected" + }) + + expect(Either.isRight(valid)).toBe(true) + expect(Either.isLeft(invalid)).toBe(true) + })) + it.effect("decodes apply project playwright resource limits", () => Effect.sync(() => { const result = Schema.decodeUnknownEither(ApplyProjectRequestSchema)({ diff --git a/packages/api/tests/terminal-sessions.test.ts b/packages/api/tests/terminal-sessions.test.ts index 017a92f1..ca3b8d33 100644 --- a/packages/api/tests/terminal-sessions.test.ts +++ b/packages/api/tests/terminal-sessions.test.ts @@ -380,6 +380,23 @@ describe("terminal sessions service", () => { }) }) + it("serializes foreground terminal state writes without resurrecting deleted sessions", async () => { + probeProjectSshReadyMock.mockImplementation(() => Effect.succeed(true)) + getProjectMock.mockImplementation(() => Effect.succeed(projectDetails)) + + const first = await runTestEffect(createTerminalSession(projectId)) + const second = await runTestEffect(createTerminalSession(projectId)) + clearTerminalSessionRuntimeForTest() + + await runTestEffect(Effect.all([ + setProjectActiveTerminalSession(projectId, first.session.id), + deleteTerminalSession(projectId, second.session.id) + ], { concurrency: "unbounded", discard: true })) + + expect(readPersistedSessionIds()).toEqual([first.session.id]) + expect(readPersistedActiveSessionId()).toBe(first.session.id) + }) + it("rejects active terminal selection for a missing project session", async () => { probeProjectSshReadyMock.mockImplementation(() => Effect.succeed(true)) getProjectMock.mockImplementation(() => Effect.succeed(projectDetails)) diff --git a/packages/app/src/docker-git/frontend-lib/core/command-builders.ts b/packages/app/src/docker-git/frontend-lib/core/command-builders.ts index ab42aaa4..5715ce52 100644 --- a/packages/app/src/docker-git/frontend-lib/core/command-builders.ts +++ b/packages/app/src/docker-git/frontend-lib/core/command-builders.ts @@ -24,6 +24,15 @@ import { import { resolveResourceLimitsIntent } from "./resource-limits.js" import { normalizeAuthLabel, normalizeGitTokenLabel } from "./token-labels.js" +// CHANGE: clonedOnHostname is explicit input instead of an OS hostname() read. +// WHY: buildCreateCommand belongs to FUNCTIONAL CORE and must be deterministic for identical RawOptions. +// QUOTE(ТЗ): "CORE: Исключительно чистые функции, неизменяемые данные, математические операции" +// REF: pr-420-coderabbit-review-4518791377 +// SOURCE: n/a +// FORMAT THEOREM: ∀raw: buildCreateCommand(raw) is independent of host OS state. +// PURITY: CORE +// INVARIANT: clone-origin host identity flows through raw.clonedOnHostname only. +// COMPLEXITY: O(1)/O(1) export { nonEmpty } from "./command-builders-shared.js" const normalizeSecretsRoot = trimTrailingPathSeparators diff --git a/packages/app/src/docker-git/frontend-lib/core/command-options.ts b/packages/app/src/docker-git/frontend-lib/core/command-options.ts index 3883d63d..918ceea1 100644 --- a/packages/app/src/docker-git/frontend-lib/core/command-options.ts +++ b/packages/app/src/docker-git/frontend-lib/core/command-options.ts @@ -60,6 +60,7 @@ export interface RawOptions { readonly force?: boolean readonly forceEnv?: boolean readonly agentAutoMode?: string + // Hostname where the project was cloned; passed explicitly to keep command builders pure. readonly clonedOnHostname?: string } diff --git a/packages/docker-git-session-sync/eslint.effect-ts-check.config.mjs b/packages/docker-git-session-sync/eslint.effect-ts-check.config.mjs index 6273c4ad..a6262535 100644 --- a/packages/docker-git-session-sync/eslint.effect-ts-check.config.mjs +++ b/packages/docker-git-session-sync/eslint.effect-ts-check.config.mjs @@ -12,32 +12,7 @@ import eslintComments from "@eslint-community/eslint-plugin-eslint-comments" import globals from "globals" import tseslint from "typescript-eslint" -const effectMigrationWarnings = [ - { - selector: "TryStatement", - message: "Effect migration blocker: replace try/catch with Effect.try / Effect.catch* at shell boundaries." - }, - { - selector: "SwitchStatement", - message: "Effect migration blocker: use Match.exhaustive instead of switch." - }, - { - selector: "AwaitExpression", - message: "Effect migration blocker: use Effect.gen / Effect.flatMap instead of await." - }, - { - selector: "FunctionDeclaration[async=true], FunctionExpression[async=true], ArrowFunctionExpression[async=true]", - message: "Effect migration blocker: use Effect.gen / Effect.tryPromise instead of async functions." - }, - { - selector: "NewExpression[callee.name='Promise']", - message: "Effect migration blocker: use Effect.async / Effect.tryPromise instead of new Promise." - }, - { - selector: "CallExpression[callee.object.name='Promise']", - message: "Effect migration blocker: use Effect combinators instead of Promise.*." - } -] +import { effectMigrationWarnings, effectPromiseRestrictedTypes } from "../../eslint.effect-ts-shared.mjs" export default tseslint.config({ name: "docker-git-session-sync-effect-ts-compliance", @@ -58,16 +33,7 @@ export default tseslint.config({ "ts-nocheck": true }], "@typescript-eslint/no-explicit-any": "error", - "@typescript-eslint/no-restricted-types": ["error", { - types: { - Promise: { - message: "Avoid Promise in public types. Use Effect.Effect." - }, - "Promise<*>": { - message: "Avoid Promise. Use Effect.Effect." - } - } - }], + "@typescript-eslint/no-restricted-types": ["warn", effectPromiseRestrictedTypes], "eslint-comments/disable-enable-pair": "error", "eslint-comments/no-unlimited-disable": "error", "eslint-comments/no-unused-disable": "error", diff --git a/packages/docker-git-session-sync/package.json b/packages/docker-git-session-sync/package.json index c91fcd6d..3da75b58 100644 --- a/packages/docker-git-session-sync/package.json +++ b/packages/docker-git-session-sync/package.json @@ -49,6 +49,7 @@ "@types/node": "^25.9.3", "@vitejs/plugin-react": "^6.0.2", "eslint": "^10.5.0", + "fast-check": "4.8.0", "globals": "^17.6.0", "typescript": "^6.0.3", "typescript-eslint": "^8.61.1", diff --git a/packages/docker-git-session-sync/src/backup.ts b/packages/docker-git-session-sync/src/backup.ts index e45b1e91..01b483a8 100644 --- a/packages/docker-git-session-sync/src/backup.ts +++ b/packages/docker-git-session-sync/src/backup.ts @@ -2,6 +2,7 @@ import fs from "node:fs" import os from "node:os" import path from "node:path" import { spawn, spawnSync } from "node:child_process" +import { Effect } from "effect" import { buildBlobUrl, @@ -353,6 +354,16 @@ export type SessionUploadContext = { readonly verbose: boolean } +// CHANGE: Keep the historical parser API while delegating validation to the schema boundary. +// WHY: CLI/tests import parseUploadContext; preserving the wrapper avoids churn outside the decoder module. +// QUOTE(ТЗ): "parseUploadContext wrapper comment or removal" +// REF: user-request-pr-420-coderabbit-session-sync +// SOURCE: n/a +// FORMAT THEOREM: parseUploadContext(x)=decodeSessionUploadContext(x) +// PURITY: SHELL +// EFFECT: none +// INVARIANT: invalid upload context returns null; valid context preserves all decoded fields. +// COMPLEXITY: O(n)/O(n), where n is the encoded context field count. export const parseUploadContext = (value: unknown): SessionUploadContext | null => decodeSessionUploadContext(value) @@ -433,7 +444,8 @@ const createQueuedComment = ( resolved.prContext.repo, resolved.prContext.prNumber, buildCommentBody({ source: resolved.source, upload: { state: "queued" }, gitStatus: resolved.gitStatus }), - ghEnv + ghEnv, + (message) => output.err(`[session-backup] ${message}`) ) if (comment === null) { output.err("[session-backup] Failed to post PR comment with git status") @@ -587,26 +599,35 @@ const writeBackgroundReadyState = (readyFilePath: string | null, state: Backgrou } } -const readBackgroundReadyState = (readyFilePath: string): BackgroundReadyState | null => { - try { - return decodeBackgroundReadyState(JSON.parse(fs.readFileSync(readyFilePath, "utf8"))) - } catch { - return null - } -} +const readBackgroundReadyState = ( + readyFilePath: string +): Effect.Effect => + Effect.try({ + try: () => decodeBackgroundReadyState(JSON.parse(fs.readFileSync(readyFilePath, "utf8"))), + catch: errorMessage + }).pipe(Effect.catchAll(() => Effect.succeed(null))) -const waitForBackgroundReady = (readyFilePath: string): BackgroundReadyState | null => { - const deadline = Date.now() + backgroundReadyTimeoutMs - while (Date.now() < deadline) { - if (fs.existsSync(readyFilePath)) { - const state = readBackgroundReadyState(readyFilePath) - if (state !== null) { - return state - } +const waitForBackgroundReady = (readyFilePath: string): Effect.Effect => { + const continuePolling = (deadline: number): Effect.Effect => + Effect.sync(() => { + sleepSync(Math.min(backgroundReadyPollMs, Math.max(1, deadline - Date.now()))) + }).pipe( + Effect.zipRight(Effect.suspend(() => poll(deadline))) + ) + + const poll = (deadline: number): Effect.Effect => { + if (Date.now() >= deadline) { + return Effect.succeed(null) + } + if (!fs.existsSync(readyFilePath)) { + return continuePolling(deadline) } - sleepSync(Math.min(backgroundReadyPollMs, Math.max(1, deadline - Date.now()))) + return readBackgroundReadyState(readyFilePath).pipe( + Effect.flatMap((state) => state === null ? continuePolling(deadline) : Effect.succeed(state)) + ) } - return null + + return Effect.suspend(() => poll(Date.now() + backgroundReadyTimeoutMs)) } const spawnBackgroundUpload = (context: SessionUploadContext, output: Output): boolean => { @@ -630,7 +651,7 @@ const spawnBackgroundUpload = (context: SessionUploadContext, output: Output): b child.once("error", (error) => { output.err(`[session-backup] Background upload process error: ${errorMessage(error)}`) }) - const readyState = waitForBackgroundReady(readyFilePath) + const readyState = Effect.runSync(waitForBackgroundReady(readyFilePath)) fs.rmSync(readyFilePath, { force: true }) if (readyState === null) { output.err("[session-backup] Background upload did not report readiness") diff --git a/packages/docker-git-session-sync/src/main.ts b/packages/docker-git-session-sync/src/main.ts index 12fba0da..862b74ab 100644 --- a/packages/docker-git-session-sync/src/main.ts +++ b/packages/docker-git-session-sync/src/main.ts @@ -11,7 +11,7 @@ import { errorMessage } from "./json.js" // SOURCE: n/a // FORMAT THEOREM: runCli(args,cwd)=0 -> exitCode unchanged; runCli(args,cwd)=n>0 -> process.exitCode=n; throws(e) -> stderr=e ∧ exitCode=1 // PURITY: SHELL -// EFFECT: Effect +// EFFECT: Effect // INVARIANT: CLI observable exit code and stderr semantics are preserved. // COMPLEXITY: O(n)/O(1), where n is delegated CLI work. const main = pipe( diff --git a/packages/docker-git-session-sync/src/schemas.ts b/packages/docker-git-session-sync/src/schemas.ts index c900490a..44e1bc6c 100644 --- a/packages/docker-git-session-sync/src/schemas.ts +++ b/packages/docker-git-session-sync/src/schemas.ts @@ -65,14 +65,18 @@ const GitHubPrCommentResponseSchema = Schema.Struct({ }) const GitHubContentResponseSchema = Schema.Struct({ - encoding: Schema.String, + encoding: Schema.Literal("base64"), content: Schema.String }) export type GitHubContentResponse = Schema.Schema.Type +const GitHubShaSchema = Schema.String.pipe( + Schema.pattern(/^[0-9a-f]{40}$/iu) +) + const GitHubShaResponseSchema = Schema.Struct({ - sha: Schema.String + sha: GitHubShaSchema }) const TreeEntrySchema: Schema.Schema = Schema.Struct({ @@ -82,10 +86,26 @@ const TreeEntrySchema: Schema.Schema = Schema.Struct({ sha: Schema.String }) -const GitHubTreeResponseSchema = Schema.Struct({ - tree: Schema.Array(Schema.Unknown) +type GitHubTreeResponse = { + readonly tree: ReadonlyArray +} + +const GitHubTreeResponseSchema: Schema.Schema = Schema.Struct({ + tree: Schema.Array(TreeEntrySchema) }) +/** + * Decodes persisted background upload context at the JSON boundary. + * + * @param value - Unknown JSON value read from the upload context file. + * @returns A typed session upload context, or null when required fields are invalid. + * @pure false - boundary decoder for persisted JSON. + * @effect none - synchronous schema decode only. + * @invariant decode(x)=ctx -> ctx.version=1 and ctx.verbose is boolean. + * @precondition value may be any JSON-compatible value. + * @postcondition returned contexts preserve source, snapshotRef, gitStatus, and PR comment metadata. + * @complexity O(n) time / O(n) space where n is the encoded field count. + */ // CHANGE: Decode background upload context at the JSON boundary with @effect/schema. // WHY: Once decoded, upload code receives a typed context instead of ad hoc unknown field probes. // QUOTE(ТЗ): "заменить самые явные try/catch/unknown JSON boundaries на typed Schema декодирование" @@ -102,12 +122,36 @@ export const decodeSessionUploadContext = (value: unknown): SessionUploadContext onRight: (context) => context }) +/** + * Decodes the child-process readiness handshake payload. + * + * @param value - Unknown JSON value read from the readiness file. + * @returns A typed readiness state, or null when the payload violates the schema. + * @pure false - boundary decoder for JSON produced by another process. + * @effect none - synchronous schema decode only. + * @invariant decode(valid started|failed state) != null; decode(invalid) = null. + * @precondition value may be any JSON-compatible value. + * @postcondition returned failed states always carry a string message. + * @complexity O(n) time / O(n) space where n is the decoded field count. + */ export const decodeBackgroundReadyState = (value: unknown): BackgroundReadyState | null => Either.match(ParseResult.decodeUnknownEither(BackgroundReadyStateSchema)(value), { onLeft: () => null, onRight: (state) => state }) +/** + * Decodes GitHub repository metadata used by the backup repository resolver. + * + * @param value - Unknown GitHub repository API response. + * @returns Normalized repository info, or null when the response shape is invalid. + * @pure false - boundary decoder for GitHub API JSON. + * @effect none - synchronous schema decode only. + * @invariant missing nullable fields normalize to null; invalid objects decode to null. + * @precondition value may be any GitHub API JSON response. + * @postcondition defaultBranch and htmlUrl are either strings or null. + * @complexity O(n) time / O(n) space where n is the decoded field count. + */ export const decodeGitHubRepoInfo = (value: unknown): GitHubRepoInfo | null => Either.match(ParseResult.decodeUnknownEither(GitHubRepoInfoSchema)(value), { onLeft: () => null, @@ -117,6 +161,18 @@ export const decodeGitHubRepoInfo = (value: unknown): GitHubRepoInfo | null => }) }) +/** + * Decodes GitHub issue-comment creation responses into the internal PR comment type. + * + * @param value - Unknown GitHub issue comment API response. + * @returns A typed PR comment reference, or null when the response shape is invalid. + * @pure false - boundary decoder for GitHub API JSON. + * @effect none - synchronous schema decode only. + * @invariant decode(valid response) preserves id and html_url as id and url. + * @precondition value may be any GitHub API JSON response. + * @postcondition returned comments always contain a numeric id and URL string. + * @complexity O(n) time / O(n) space where n is the decoded field count. + */ export const decodeGitHubPrComment = (value: unknown): PrComment | null => Either.match(ParseResult.decodeUnknownEither(GitHubPrCommentResponseSchema)(value), { onLeft: () => null, @@ -126,30 +182,59 @@ export const decodeGitHubPrComment = (value: unknown): PrComment | null => }) }) +/** + * Decodes GitHub file-content responses that are contractually base64 encoded. + * + * @param value - Unknown GitHub contents API response. + * @returns A base64 content response, or null when encoding/content are invalid. + * @pure false - boundary decoder for GitHub API JSON. + * @effect none - synchronous schema decode only. + * @invariant decode(value) != null -> value.encoding = "base64". + * @precondition value may be any GitHub API JSON response. + * @postcondition returned content is a string and may be empty for empty files. + * @complexity O(n) time / O(n) space where n is the decoded field count. + */ export const decodeGitHubContentResponse = (value: unknown): GitHubContentResponse | null => Either.match(ParseResult.decodeUnknownEither(GitHubContentResponseSchema)(value), { onLeft: () => null, onRight: (content) => content }) -export const decodeGitHubSha = (value: unknown): string | null => +/** + * Decodes GitHub object SHA responses and fails fast on contract violations. + * + * @param value - Unknown GitHub API response containing a sha field. + * @param context - Human-readable GitHub operation context for parse errors. + * @returns A 40-character hexadecimal Git object SHA. + * @pure false - boundary decoder for GitHub API JSON. + * @effect throws Error when the GitHub response is missing or has an invalid SHA. + * @invariant returned sha matches /^[0-9a-f]{40}$/i. + * @precondition value may be any GitHub API JSON response. + * @postcondition callers never receive null or malformed SHAs. + * @complexity O(n) time / O(n) space where n is the decoded field count. + */ +export const decodeGitHubSha = (value: unknown, context: string = "GitHub response"): string => Either.match(ParseResult.decodeUnknownEither(GitHubShaResponseSchema)(value), { - onLeft: () => null, + onLeft: () => { + throw new Error(`${context} missing valid sha`) + }, onRight: (response) => response.sha }) -const decodeTreeEntry = (value: unknown): TreeEntry | null => - Either.match(ParseResult.decodeUnknownEither(TreeEntrySchema)(value), { - onLeft: () => null, - onRight: (entry) => entry - }) - -export const decodeGitHubTreeEntries = (value: unknown): ReadonlyArray => +/** + * Decodes GitHub recursive tree responses without silently dropping invalid entries. + * + * @param value - Unknown GitHub tree API response. + * @returns A readonly collection of tree entries, or null when the response is invalid. + * @pure false - boundary decoder for GitHub API JSON. + * @effect none - synchronous schema decode only. + * @invariant invalid tree payloads decode to null, never to []. + * @precondition value may be any GitHub API JSON response. + * @postcondition every returned entry contains path, mode, type, and sha strings. + * @complexity O(n) time / O(n) space where n is the number of tree entries. + */ +export const decodeGitHubTreeEntries = (value: unknown): ReadonlyArray | null => Either.match(ParseResult.decodeUnknownEither(GitHubTreeResponseSchema)(value), { - onLeft: () => [], - onRight: (response) => - response.tree.flatMap((entry) => { - const decoded = decodeTreeEntry(entry) - return decoded === null ? [] : [decoded] - }) + onLeft: () => null, + onRight: (response) => response.tree }) diff --git a/packages/docker-git-session-sync/src/shell.ts b/packages/docker-git-session-sync/src/shell.ts index 624ac29d..106b4270 100644 --- a/packages/docker-git-session-sync/src/shell.ts +++ b/packages/docker-git-session-sync/src/shell.ts @@ -175,14 +175,23 @@ export const ensureBackupRepo = (ghEnv: GhEnv, log: Log, createIfMissing: boolea } const repoInfo = decodeGitHubRepoInfo(repoResult.json) if (repoInfo === null) { + log(`GitHub repository response for ${repoFullName} was invalid`) return null } + const defaultBranch = repoInfo.defaultBranch ?? backupDefaultBranch + const htmlUrl = repoInfo.htmlUrl ?? `https://github.com/${repoFullName}` + if (repoInfo.defaultBranch === null) { + log(`GitHub repository response for ${repoFullName} missing default_branch; using ${defaultBranch}`) + } + if (repoInfo.htmlUrl === null) { + log(`GitHub repository response for ${repoFullName} missing html_url; using ${htmlUrl}`) + } return { owner: login, repo: backupRepoName, fullName: repoFullName, - defaultBranch: repoInfo.defaultBranch ?? backupDefaultBranch, - htmlUrl: repoInfo.htmlUrl ?? `https://github.com/${repoFullName}` + defaultBranch, + htmlUrl } } @@ -205,10 +214,14 @@ export const getTreeEntries = (repoFullName: string, branch: string, ghEnv: GhEn ghApiJson(`/repos/${repoFullName}/git/trees/${treeSha}?recursive=1`, ghEnv), `failed to list tree for ${repoFullName}@${branch}` ) + const entries = decodeGitHubTreeEntries(result.json) + if (entries === null) { + throw new Error(`GitHub tree response invalid for ${repoFullName}@${branch}`) + } return { headSha, treeSha, - entries: decodeGitHubTreeEntries(result.json) + entries } } @@ -223,10 +236,10 @@ export const getFileContent = ( `failed to fetch ${repoFullName}:${repoPath}` ) const contentResponse = decodeGitHubContentResponse(result.json) - const content = contentResponse?.content.replace(/\n/gu, "") ?? "" - if (contentResponse?.encoding !== "base64" || content.length === 0) { + if (contentResponse === null) { throw new Error(`unexpected content payload for ${repoFullName}:${repoPath}`) } + const content = contentResponse.content.replace(/\n/gu, "") return Buffer.from(content, "base64") } @@ -234,13 +247,22 @@ export const createPrComment = ( repoFullName: string, prNumber: number, body: string, - ghEnv: GhEnv + ghEnv: GhEnv, + log: Log = () => undefined ): PrComment | null => { const result = ghApiJson(`/repos/${repoFullName}/issues/${prNumber}/comments`, ghEnv, { method: "POST", body: { body } }) - return result.success ? decodeGitHubPrComment(result.json) : null + if (!result.success) { + log(`GitHub PR comment API failed for ${repoFullName}#${prNumber}: ${result.stderr || result.stdout || `exit ${result.status}`}`) + return null + } + const comment = decodeGitHubPrComment(result.json) + if (comment === null) { + log(`GitHub PR comment response invalid for ${repoFullName}#${prNumber}`) + } + return comment } export const updatePrComment = ( @@ -538,10 +560,7 @@ const createGitBlob = (repoFullName: string, entry: UploadEntry, ghEnv: GhEnv): }), `failed to create blob for ${repoFullName}:${entry.repoPath}` ) - const sha = decodeGitHubSha(result.json) - if (sha === null) { - throw new Error(`GitHub blob response missing sha for ${entry.repoPath}`) - } + const sha = decodeGitHubSha(result.json, `GitHub blob response for ${entry.repoPath}`) if (sha !== entry.blobSha) { throw new Error(`GitHub blob sha mismatch for ${entry.repoPath}`) } @@ -564,11 +583,7 @@ const createGitTree = ( }), `failed to create tree in ${repoFullName}` ) - const sha = decodeGitHubSha(result.json) - if (sha === null) { - throw new Error(`GitHub tree response missing sha for ${repoFullName}`) - } - return sha + return decodeGitHubSha(result.json, `GitHub tree response for ${repoFullName}`) } const createGitCommit = ( @@ -596,11 +611,7 @@ const createGitCommit = ( }), `failed to create commit in ${backupRepo.fullName}` ) - const sha = decodeGitHubSha(result.json) - if (sha === null) { - throw new Error(`GitHub commit response missing sha for ${backupRepo.fullName}`) - } - return sha + return decodeGitHubSha(result.json, `GitHub commit response for ${backupRepo.fullName}`) } const updateGitRef = (repoFullName: string, branch: string, commitSha: string, ghEnv: GhEnv): CommandResult => diff --git a/packages/docker-git-session-sync/tests/session-files.test.ts b/packages/docker-git-session-sync/tests/session-files.test.ts index 448e5166..7ffe2d29 100644 --- a/packages/docker-git-session-sync/tests/session-files.test.ts +++ b/packages/docker-git-session-sync/tests/session-files.test.ts @@ -1,6 +1,7 @@ import fs from "node:fs" import os from "node:os" import path from "node:path" +import fc from "fast-check" import { afterEach, beforeEach, describe, expect, it } from "vitest" import { @@ -14,6 +15,11 @@ import { } from "../src/core.js" import { collectSessionFiles, parseUploadContext, uploadFromContext, type Output } from "../src/backup.js" import { parseArgs } from "../src/cli.js" +import { + decodeGitHubContentResponse, + decodeGitHubSha, + decodeGitHubTreeEntries +} from "../src/schemas.js" import { buildUploadTreeChanges, gitBlobShaForBuffer, @@ -27,6 +33,50 @@ const output: Output = { err: () => undefined } +const hexShaArbitrary = fc.array( + fc.constantFrom("0", "1", "2", "3", "4", "5", "6", "7", "8", "9", "a", "b", "c", "d", "e", "f"), + { minLength: 40, maxLength: 40 } +).map((chars) => chars.join("")) + +const pathPartArbitrary = fc.array( + fc.constantFrom("a", "b", "c", "d", "e", "f", "0", "1", "2", "3", "x", "y", "z", "-", "_"), + { minLength: 1, maxLength: 10 } +).map((chars) => chars.join("")) + +const staleSessionPathArbitrary = fc.tuple(pathPartArbitrary, pathPartArbitrary).map( + ([scope, name]) => `org/repo/pr-230/current/.codex/sessions/stale-${scope}/${name}.jsonl` +) + +const sourceInfoArbitrary = fc.record({ + repo: fc.constantFrom("org/repo", "prover/docker-git", "backup-owner/docker-git"), + branch: pathPartArbitrary, + prNumber: fc.option(fc.integer({ min: 1, max: 999_999 }), { nil: null }), + commitSha: hexShaArbitrary, + createdAt: fc.integer({ + min: Date.parse("2026-01-01T00:00:00.000Z"), + max: Date.parse("2026-12-31T23:59:59.999Z") + }).map((time) => new Date(time).toISOString()) +}) + +const prCommentContextArbitrary = fc.record({ + repo: fc.constantFrom("org/repo", "prover/docker-git"), + comment: fc.record({ + id: fc.integer({ min: 1, max: 2_147_483_647 }), + url: pathPartArbitrary.map((part) => `https://example.test/comments/${part}`) + }) +}) + +const sessionUploadContextArbitrary = fc.record({ + version: fc.constant(1), + cwd: pathPartArbitrary.map((part) => `/workspace/${part}`), + sessionDir: fc.option(pathPartArbitrary.map((part) => `/home/dev/${part}`), { nil: null }), + source: sourceInfoArbitrary, + snapshotRef: pathPartArbitrary.map((part) => `org/repo/pr-230/current/${part}`), + gitStatus: fc.option(fc.string({ maxLength: 120 }), { nil: null }), + prComment: fc.option(prCommentContextArbitrary, { nil: null }), + verbose: fc.boolean() +}) + let tmpDir = "" beforeEach(() => { @@ -124,6 +174,42 @@ describe("upload artifacts", () => { }) describe("snapshot tree updates", () => { + it("rejects invalid GitHub tree responses instead of silently dropping entries", () => { + expect(decodeGitHubTreeEntries({ tree: [{ path: "file.txt", mode: "100644", type: "blob" }] })).toBeNull() + }) + + it("decodes valid GitHub tree responses as readonly entries", () => { + expect(decodeGitHubTreeEntries({ + tree: [ + { + path: "file.txt", + mode: "100644", + type: "blob", + sha: "0123456789abcdef0123456789abcdef01234567" + } + ] + })).toEqual([ + { + path: "file.txt", + mode: "100644", + type: "blob", + sha: "0123456789abcdef0123456789abcdef01234567" + } + ]) + }) + + it("validates GitHub content encoding and SHA contracts at the schema boundary", () => { + expect(decodeGitHubContentResponse({ encoding: "utf-8", content: "" })).toBeNull() + expect(decodeGitHubContentResponse({ encoding: "base64", content: "" })).toEqual({ + encoding: "base64", + content: "" + }) + expect(decodeGitHubSha({ sha: "0123456789abcdef0123456789abcdef01234567" })).toBe( + "0123456789abcdef0123456789abcdef01234567" + ) + expect(() => decodeGitHubSha({ sha: "not-a-sha" })).toThrow("GitHub response missing valid sha") + }) + it("keeps stale remote session files untouched", () => { const entries: ReadonlyArray = [ { @@ -170,6 +256,44 @@ describe("snapshot tree updates", () => { expect(hasChangedUploadEntries(entries, desiredEntries)).toBe(false) expect(buildUploadTreeChanges("backup-owner/docker-git-sessions", entries, desiredEntries, {})).toEqual([]) }) + + it("preserves every stale remote session entry when desired content is already current", () => { + const desiredRepoPath = "org/repo/pr-230/current/.codex/sessions/live/current.jsonl" + fc.assert( + fc.property( + hexShaArbitrary, + fc.uniqueArray(staleSessionPathArbitrary, { minLength: 1, maxLength: 20 }), + (desiredSha, stalePaths) => { + const entries: ReadonlyArray = [ + { + path: desiredRepoPath, + mode: "100644", + type: "blob", + sha: desiredSha + }, + ...stalePaths.map((stalePath, index) => ({ + path: stalePath, + mode: "100644", + type: "blob", + sha: `${index}`.padStart(40, "0") + })) + ] + const desiredEntries: ReadonlyArray = [ + { + repoPath: desiredRepoPath, + sourcePath: path.join(tmpDir, "unused.jsonl"), + type: "file", + size: 4, + blobSha: desiredSha + } + ] + const changes = buildUploadTreeChanges("backup-owner/docker-git-sessions", entries, desiredEntries, {}) + return !hasChangedUploadEntries(entries, desiredEntries) + && changes.every((change) => !stalePaths.includes(change.path)) + } + ) + ) + }) }) describe("PR comment body", () => { @@ -330,6 +454,18 @@ describe("CLI parser", () => { expect(parsed?.prComment?.comment.id).toBe(1001) }) + it("decodes every schema-valid background upload context with version one", () => { + fc.assert( + fc.property(sessionUploadContextArbitrary, (context) => { + const parsed = parseUploadContext(context) + return parsed !== null + && parsed.version === 1 + && parsed.source.repo === context.source.repo + && parsed.snapshotRef === context.snapshotRef + }) + ) + }) + it("rejects malformed background upload context metadata", () => { expect(parseUploadContext({ version: 1, diff --git a/packages/lib/src/core/command-builders.ts b/packages/lib/src/core/command-builders.ts index 37430751..99c5a058 100644 --- a/packages/lib/src/core/command-builders.ts +++ b/packages/lib/src/core/command-builders.ts @@ -23,6 +23,15 @@ import { import { resolveResourceLimitsIntent } from "./resource-limits.js" import { normalizeAuthLabel, normalizeGitTokenLabel } from "./token-labels.js" +// CHANGE: clonedOnHostname is explicit input instead of an OS hostname() read. +// WHY: buildCreateCommand belongs to FUNCTIONAL CORE and must be deterministic for identical RawOptions. +// QUOTE(ТЗ): "CORE: Исключительно чистые функции, неизменяемые данные, математические операции" +// REF: pr-420-coderabbit-review-4518791377 +// SOURCE: n/a +// FORMAT THEOREM: ∀raw: buildCreateCommand(raw) is independent of host OS state. +// PURITY: CORE +// INVARIANT: clone-origin host identity flows through raw.clonedOnHostname only. +// COMPLEXITY: O(1)/O(1) export { nonEmpty } from "./command-builders-shared.js" const normalizeSecretsRoot = trimTrailingPathSeparators diff --git a/packages/lib/src/core/command-options.ts b/packages/lib/src/core/command-options.ts index 8ba09533..f3073dfc 100644 --- a/packages/lib/src/core/command-options.ts +++ b/packages/lib/src/core/command-options.ts @@ -59,6 +59,7 @@ export interface RawOptions { readonly force?: boolean readonly forceEnv?: boolean readonly agentAutoMode?: string + // Hostname where the project was cloned; passed explicitly to keep command builders pure. readonly clonedOnHostname?: string // Session gist options (issue-143) readonly prNumber?: string diff --git a/packages/lib/src/shell/config.ts b/packages/lib/src/shell/config.ts index 68e997b8..d165a0a9 100644 --- a/packages/lib/src/shell/config.ts +++ b/packages/lib/src/shell/config.ts @@ -15,6 +15,14 @@ import { import { ConfigDecodeError, ConfigNotFoundError } from "./errors.js" import { resolveBaseDir } from "./paths.js" +const HostnameSchema = Schema.String.pipe( + Schema.minLength(1), + Schema.maxLength(253), + Schema.pattern( + /^(?:[A-Za-z0-9](?:[A-Za-z0-9-]{0,61}[A-Za-z0-9])?)(?:\.[A-Za-z0-9](?:[A-Za-z0-9-]{0,61}[A-Za-z0-9])?)*$/u + ) +) + const TemplateConfigInputSchema = Schema.Struct({ containerName: Schema.String, serviceName: Schema.String, @@ -79,7 +87,7 @@ const TemplateConfigInputSchema = Schema.Struct({ }), bunVersion: Schema.optional(Schema.String), pnpmVersion: Schema.optional(Schema.String), - clonedOnHostname: Schema.optional(Schema.String) + clonedOnHostname: Schema.optional(HostnameSchema) }) type DecodedProjectConfigInput = Schema.Schema.Type From 18b068f7218027af4c77e18f4427398e6145cc56 Mon Sep 17 00:00:00 2001 From: skulidropek <66840575+skulidropek@users.noreply.github.com> Date: Wed, 17 Jun 2026 20:22:49 +0000 Subject: [PATCH 3/3] fix(effect): close requirements alignment gaps - migrate session-sync JSON decoders to typed Effect DecodeError contracts - add visible terminal persistence race regression coverage - log best-effort snapshot persistence failures and type readonly snapshot schema - document clonedOnHostname public contracts --- packages/api/src/api/contracts.ts | 14 ++ packages/api/src/services/agents.ts | 12 +- packages/api/tests/terminal-sessions.test.ts | 2 +- .../app/src/docker-git/api-project-codec.ts | 10 ++ .../core/command-builders-template.ts | 10 ++ .../frontend-lib/core/command-options.ts | 11 +- .../docker-git/frontend-lib/core/domain.ts | 10 ++ .../app/src/docker-git/menu-select-filter.ts | 10 ++ .../src/docker-git/menu-select-presenter.ts | 10 ++ .../docker-git-session-sync/src/backup.ts | 14 +- .../docker-git-session-sync/src/schemas.ts | 131 ++++++++++-------- packages/docker-git-session-sync/src/shell.ts | 44 ++++-- .../tests/session-files.test.ts | 24 +++- .../lib/src/core/command-builders-template.ts | 10 ++ packages/lib/src/core/command-options.ts | 11 +- packages/lib/src/usecases/projects-core.ts | 10 ++ packages/lib/src/usecases/ssh-access.ts | 26 ++++ 17 files changed, 270 insertions(+), 89 deletions(-) diff --git a/packages/api/src/api/contracts.ts b/packages/api/src/api/contracts.ts index 8d9dc3ca..e6efd92d 100644 --- a/packages/api/src/api/contracts.ts +++ b/packages/api/src/api/contracts.ts @@ -26,6 +26,16 @@ export type ProjectSummary = { readonly sshSessions: number readonly startedAtIso: string | null readonly startedAtEpochMs: number | null + /** + * Hostname of the machine where the project was originally cloned, when known. + * + * @pure true - immutable API DTO field. + * @effect none + * @invariant if present, the value was decoded by the API/config hostname schema. + * @precondition producers omit the field when clone host identity is unknown. + * @postcondition consumers can group projects by clone-origin host without reading OS state. + * @complexity O(1)/O(1) + */ readonly clonedOnHostname?: string } @@ -490,7 +500,11 @@ export type CreateProjectRequest = { * SOURCE: n/a * FORMAT THEOREM: ∀h ∈ Hostname: request(h) -> build(request).config.clonedOnHostname = h * PURITY: CORE - immutable request data. + * @pure true - immutable API request DTO field. + * @effect none * INVARIANT: if present, value satisfies API HostnameSchema. + * @precondition callers omit the field when clone host identity is unknown. + * @postcondition command builders receive host identity as data, not by reading OS state. * COMPLEXITY: O(1)/O(1) */ readonly clonedOnHostname?: string diff --git a/packages/api/src/services/agents.ts b/packages/api/src/services/agents.ts index 2cc6bfec..66480ef9 100644 --- a/packages/api/src/services/agents.ts +++ b/packages/api/src/services/agents.ts @@ -35,8 +35,11 @@ type SnapshotFile = { readonly sessions: ReadonlyArray } +// Schema.Array already decodes to ReadonlyArray in effect@3.21.x; Schema.ReadonlyArray is not exported. +const SnapshotSessionsSchema: Schema.Schema> = Schema.Array(AgentSessionSchema) + const SnapshotFileSchema: Schema.Schema = Schema.Struct({ - sessions: Schema.Array(AgentSessionSchema) + sessions: SnapshotSessionsSchema }) const SnapshotFileJsonSchema = Schema.parseJson(SnapshotFileSchema) @@ -253,12 +256,13 @@ const persistSnapshot = (): Effect.Effect => + Effect.logWarning(`[agents] persistSnapshotBestEffort failed: ${String(error)}`) + const persistSnapshotBestEffort = (): void => { Effect.runFork(persistSnapshot().pipe( Effect.provide(NodeContext.layer), - Effect.catchAll((error) => - Effect.logWarning(`[agents] Failed to persist snapshot: ${String(error)}`) - ) + Effect.catchAll(logSnapshotPersistenceFailure) )) } diff --git a/packages/api/tests/terminal-sessions.test.ts b/packages/api/tests/terminal-sessions.test.ts index ca3b8d33..2c79d969 100644 --- a/packages/api/tests/terminal-sessions.test.ts +++ b/packages/api/tests/terminal-sessions.test.ts @@ -380,7 +380,7 @@ describe("terminal sessions service", () => { }) }) - it("serializes foreground terminal state writes without resurrecting deleted sessions", async () => { + it("regression: serializes terminal persistence race between active selection and delete", async () => { probeProjectSshReadyMock.mockImplementation(() => Effect.succeed(true)) getProjectMock.mockImplementation(() => Effect.succeed(projectDetails)) diff --git a/packages/app/src/docker-git/api-project-codec.ts b/packages/app/src/docker-git/api-project-codec.ts index 9efb34c8..1f109e56 100644 --- a/packages/app/src/docker-git/api-project-codec.ts +++ b/packages/app/src/docker-git/api-project-codec.ts @@ -11,6 +11,16 @@ export type ApiProjectSummary = { readonly sshSessions: number readonly startedAtIso: string | null readonly startedAtEpochMs: number | null + /** + * Hostname of the clone-origin machine, if the API supplied it. + * + * @pure true - immutable decoded API field. + * @effect none + * @invariant if present, codec preserves the API hostname string unchanged. + * @precondition API response may omit the value for older projects. + * @postcondition web UI can filter or display clone-origin host without local OS reads. + * @complexity O(1)/O(1) + */ readonly clonedOnHostname?: string | undefined } diff --git a/packages/app/src/docker-git/frontend-lib/core/command-builders-template.ts b/packages/app/src/docker-git/frontend-lib/core/command-builders-template.ts index f54a214d..152732f1 100644 --- a/packages/app/src/docker-git/frontend-lib/core/command-builders-template.ts +++ b/packages/app/src/docker-git/frontend-lib/core/command-builders-template.ts @@ -22,6 +22,16 @@ export type BuildTemplateConfigInput = { readonly enableMcpPlaywright: boolean readonly agentMode: AgentMode | undefined readonly agentAuto: boolean + /** + * Hostname where the source project was cloned. + * + * @pure true - immutable template-builder input. + * @effect none + * @invariant if present, template config preserves this value without reading OS hostname. + * @precondition boundary validation rejects malformed hostnames before constructing this input. + * @postcondition buildTemplateConfig propagates the value into docker-git.json. + * @complexity O(1)/O(1) + */ readonly clonedOnHostname?: string | undefined } diff --git a/packages/app/src/docker-git/frontend-lib/core/command-options.ts b/packages/app/src/docker-git/frontend-lib/core/command-options.ts index 918ceea1..8dbbf70f 100644 --- a/packages/app/src/docker-git/frontend-lib/core/command-options.ts +++ b/packages/app/src/docker-git/frontend-lib/core/command-options.ts @@ -60,7 +60,16 @@ export interface RawOptions { readonly force?: boolean readonly forceEnv?: boolean readonly agentAutoMode?: string - // Hostname where the project was cloned; passed explicitly to keep command builders pure. + /** + * Hostname where the project was cloned; passed explicitly to keep command builders pure. + * + * @pure true - immutable command-builder input. + * @effect none + * @invariant if present, command config preserves this value without reading OS hostname. + * @precondition boundary validation rejects malformed hostnames before constructing RawOptions. + * @postcondition buildCreateCommand propagates the value to docker-git.json. + * @complexity O(1)/O(1) + */ readonly clonedOnHostname?: string } diff --git a/packages/app/src/docker-git/frontend-lib/core/domain.ts b/packages/app/src/docker-git/frontend-lib/core/domain.ts index ce944092..525e1f28 100644 --- a/packages/app/src/docker-git/frontend-lib/core/domain.ts +++ b/packages/app/src/docker-git/frontend-lib/core/domain.ts @@ -117,6 +117,16 @@ export interface TemplateConfig { readonly bunVersion: string readonly agentMode?: AgentMode | undefined readonly agentAuto?: boolean | undefined + /** + * Hostname of the machine where the project was cloned, when available. + * + * @pure true - immutable project config field. + * @effect none + * @invariant if present, UI/domain consumers preserve this value as data only. + * @precondition omitted when clone host identity is unknown. + * @postcondition UI can show clone-origin hints without reading OS hostname. + * @complexity O(1)/O(1) + */ readonly clonedOnHostname?: string | undefined } diff --git a/packages/app/src/docker-git/menu-select-filter.ts b/packages/app/src/docker-git/menu-select-filter.ts index 8153313c..6b286398 100644 --- a/packages/app/src/docker-git/menu-select-filter.ts +++ b/packages/app/src/docker-git/menu-select-filter.ts @@ -1,6 +1,16 @@ import type { ProjectItem } from "./project-item.js" export type SelectSearchAccessors = { + /** + * Reads the clone-origin hostname from a selectable item. + * + * @pure true - accessor contract for immutable item data. + * @effect none + * @invariant accessor returns undefined exactly when host identity is unavailable. + * @precondition item belongs to the project selection domain. + * @postcondition search can match host affinity without direct OS access. + * @complexity O(1)/O(1) + */ readonly clonedOnHostname: (item: A) => string | undefined readonly containerName: (item: A) => string | undefined readonly displayName: (item: A) => string diff --git a/packages/app/src/docker-git/menu-select-presenter.ts b/packages/app/src/docker-git/menu-select-presenter.ts index 48340256..e89296a2 100644 --- a/packages/app/src/docker-git/menu-select-presenter.ts +++ b/packages/app/src/docker-git/menu-select-presenter.ts @@ -8,6 +8,16 @@ export type SelectListProject = { readonly displayName: string readonly repoRef: string readonly projectDir: string + /** + * Hostname where this selectable project was cloned, when known. + * + * @pure true - immutable presenter input field. + * @effect none + * @invariant presenter preserves this field for display/search only. + * @precondition omitted when clone host identity is unknown. + * @postcondition selection views can show host affinity without reading OS state. + * @complexity O(1)/O(1) + */ readonly clonedOnHostname?: string | undefined } diff --git a/packages/docker-git-session-sync/src/backup.ts b/packages/docker-git-session-sync/src/backup.ts index 01b483a8..50e3040e 100644 --- a/packages/docker-git-session-sync/src/backup.ts +++ b/packages/docker-git-session-sync/src/backup.ts @@ -2,7 +2,7 @@ import fs from "node:fs" import os from "node:os" import path from "node:path" import { spawn, spawnSync } from "node:child_process" -import { Effect } from "effect" +import { Effect, Either } from "effect" import { buildBlobUrl, @@ -365,7 +365,10 @@ export type SessionUploadContext = { // INVARIANT: invalid upload context returns null; valid context preserves all decoded fields. // COMPLEXITY: O(n)/O(n), where n is the encoded context field count. export const parseUploadContext = (value: unknown): SessionUploadContext | null => - decodeSessionUploadContext(value) + Either.match(Effect.runSync(Effect.either(decodeSessionUploadContext(value))), { + onLeft: () => null, + onRight: (context) => context + }) const resolveBackupContext = ( options: BackupOptions, @@ -603,9 +606,12 @@ const readBackgroundReadyState = ( readyFilePath: string ): Effect.Effect => Effect.try({ - try: () => decodeBackgroundReadyState(JSON.parse(fs.readFileSync(readyFilePath, "utf8"))), + try: (): unknown => JSON.parse(fs.readFileSync(readyFilePath, "utf8")), catch: errorMessage - }).pipe(Effect.catchAll(() => Effect.succeed(null))) + }).pipe( + Effect.flatMap(decodeBackgroundReadyState), + Effect.catchAll(() => Effect.succeed(null)) + ) const waitForBackgroundReady = (readyFilePath: string): Effect.Effect => { const continuePolling = (deadline: number): Effect.Effect => diff --git a/packages/docker-git-session-sync/src/schemas.ts b/packages/docker-git-session-sync/src/schemas.ts index 44e1bc6c..983b63d4 100644 --- a/packages/docker-git-session-sync/src/schemas.ts +++ b/packages/docker-git-session-sync/src/schemas.ts @@ -1,6 +1,6 @@ -import * as ParseResult from "@effect/schema/ParseResult" import * as Schema from "@effect/schema/Schema" -import { Either } from "effect" +import * as TreeFormatter from "@effect/schema/TreeFormatter" +import { Data, Effect } from "effect" import type { PrCommentContext, SessionUploadContext } from "./backup.js" import type { PrComment, TreeEntry } from "./types.js" @@ -94,13 +94,34 @@ const GitHubTreeResponseSchema: Schema.Schema = Schema.Struc tree: Schema.Array(TreeEntrySchema) }) +export class DecodeError extends Data.TaggedError("DecodeError")<{ + readonly context: string + readonly message: string + readonly cause?: unknown +}> {} + +const decodeUnknownEffect = ( + schema: Schema.Schema, + context: string, + value: unknown +): Effect.Effect => + Schema.decodeUnknown(schema)(value).pipe( + Effect.mapError((error) => + new DecodeError({ + cause: error, + context, + message: TreeFormatter.formatErrorSync(error) + }) + ) + ) + /** * Decodes persisted background upload context at the JSON boundary. * * @param value - Unknown JSON value read from the upload context file. - * @returns A typed session upload context, or null when required fields are invalid. + * @returns Effect with a typed session upload context, or DecodeError when required fields are invalid. * @pure false - boundary decoder for persisted JSON. - * @effect none - synchronous schema decode only. + * @effect Effect * @invariant decode(x)=ctx -> ctx.version=1 and ctx.verbose is boolean. * @precondition value may be any JSON-compatible value. * @postcondition returned contexts preserve source, snapshotRef, gitStatus, and PR comment metadata. @@ -113,128 +134,116 @@ const GitHubTreeResponseSchema: Schema.Schema = Schema.Struc // SOURCE: n/a // FORMAT THEOREM: decode(x)=ctx -> ctx.version=1 ∧ ctx.source.repo∈String ∧ ctx.verbose∈Boolean // PURITY: SHELL -// EFFECT: none -// INVARIANT: invalid or incomplete context decodes to null. +// EFFECT: Effect +// INVARIANT: invalid or incomplete context fails with DecodeError. // COMPLEXITY: O(n)/O(n), where n is encoded field count. -export const decodeSessionUploadContext = (value: unknown): SessionUploadContext | null => - Either.match(ParseResult.decodeUnknownEither(SessionUploadContextSchema)(value), { - onLeft: () => null, - onRight: (context) => context - }) +export const decodeSessionUploadContext = (value: unknown): Effect.Effect => + decodeUnknownEffect(SessionUploadContextSchema, "session upload context", value) /** * Decodes the child-process readiness handshake payload. * * @param value - Unknown JSON value read from the readiness file. - * @returns A typed readiness state, or null when the payload violates the schema. + * @returns Effect with a typed readiness state, or DecodeError when the payload violates the schema. * @pure false - boundary decoder for JSON produced by another process. - * @effect none - synchronous schema decode only. - * @invariant decode(valid started|failed state) != null; decode(invalid) = null. + * @effect Effect + * @invariant decode(valid started|failed state) succeeds; decode(invalid) fails with DecodeError. * @precondition value may be any JSON-compatible value. * @postcondition returned failed states always carry a string message. * @complexity O(n) time / O(n) space where n is the decoded field count. */ -export const decodeBackgroundReadyState = (value: unknown): BackgroundReadyState | null => - Either.match(ParseResult.decodeUnknownEither(BackgroundReadyStateSchema)(value), { - onLeft: () => null, - onRight: (state) => state - }) +export const decodeBackgroundReadyState = (value: unknown): Effect.Effect => + decodeUnknownEffect(BackgroundReadyStateSchema, "background ready state", value) /** * Decodes GitHub repository metadata used by the backup repository resolver. * * @param value - Unknown GitHub repository API response. - * @returns Normalized repository info, or null when the response shape is invalid. + * @returns Effect with normalized repository info, or DecodeError when the response shape is invalid. * @pure false - boundary decoder for GitHub API JSON. - * @effect none - synchronous schema decode only. - * @invariant missing nullable fields normalize to null; invalid objects decode to null. + * @effect Effect + * @invariant missing nullable fields normalize to null; invalid objects fail with DecodeError. * @precondition value may be any GitHub API JSON response. * @postcondition defaultBranch and htmlUrl are either strings or null. * @complexity O(n) time / O(n) space where n is the decoded field count. */ -export const decodeGitHubRepoInfo = (value: unknown): GitHubRepoInfo | null => - Either.match(ParseResult.decodeUnknownEither(GitHubRepoInfoSchema)(value), { - onLeft: () => null, - onRight: (repo) => ({ +export const decodeGitHubRepoInfo = (value: unknown): Effect.Effect => + decodeUnknownEffect(GitHubRepoInfoSchema, "GitHub repository response", value).pipe( + Effect.map((repo) => ({ defaultBranch: repo.default_branch ?? null, htmlUrl: repo.html_url ?? null - }) - }) + })) + ) /** * Decodes GitHub issue-comment creation responses into the internal PR comment type. * * @param value - Unknown GitHub issue comment API response. - * @returns A typed PR comment reference, or null when the response shape is invalid. + * @returns Effect with a typed PR comment reference, or DecodeError when the response shape is invalid. * @pure false - boundary decoder for GitHub API JSON. - * @effect none - synchronous schema decode only. + * @effect Effect * @invariant decode(valid response) preserves id and html_url as id and url. * @precondition value may be any GitHub API JSON response. * @postcondition returned comments always contain a numeric id and URL string. * @complexity O(n) time / O(n) space where n is the decoded field count. */ -export const decodeGitHubPrComment = (value: unknown): PrComment | null => - Either.match(ParseResult.decodeUnknownEither(GitHubPrCommentResponseSchema)(value), { - onLeft: () => null, - onRight: (comment) => ({ +export const decodeGitHubPrComment = (value: unknown): Effect.Effect => + decodeUnknownEffect(GitHubPrCommentResponseSchema, "GitHub PR comment response", value).pipe( + Effect.map((comment) => ({ id: comment.id, url: comment.html_url - }) - }) + })) + ) /** * Decodes GitHub file-content responses that are contractually base64 encoded. * * @param value - Unknown GitHub contents API response. - * @returns A base64 content response, or null when encoding/content are invalid. + * @returns Effect with a base64 content response, or DecodeError when encoding/content are invalid. * @pure false - boundary decoder for GitHub API JSON. - * @effect none - synchronous schema decode only. - * @invariant decode(value) != null -> value.encoding = "base64". + * @effect Effect + * @invariant decode(value) succeeds -> value.encoding = "base64". * @precondition value may be any GitHub API JSON response. * @postcondition returned content is a string and may be empty for empty files. * @complexity O(n) time / O(n) space where n is the decoded field count. */ -export const decodeGitHubContentResponse = (value: unknown): GitHubContentResponse | null => - Either.match(ParseResult.decodeUnknownEither(GitHubContentResponseSchema)(value), { - onLeft: () => null, - onRight: (content) => content - }) +export const decodeGitHubContentResponse = (value: unknown): Effect.Effect => + decodeUnknownEffect(GitHubContentResponseSchema, "GitHub content response", value) /** * Decodes GitHub object SHA responses and fails fast on contract violations. * * @param value - Unknown GitHub API response containing a sha field. * @param context - Human-readable GitHub operation context for parse errors. - * @returns A 40-character hexadecimal Git object SHA. + * @returns Effect with a 40-character hexadecimal Git object SHA. * @pure false - boundary decoder for GitHub API JSON. - * @effect throws Error when the GitHub response is missing or has an invalid SHA. + * @effect Effect * @invariant returned sha matches /^[0-9a-f]{40}$/i. * @precondition value may be any GitHub API JSON response. * @postcondition callers never receive null or malformed SHAs. * @complexity O(n) time / O(n) space where n is the decoded field count. */ -export const decodeGitHubSha = (value: unknown, context: string = "GitHub response"): string => - Either.match(ParseResult.decodeUnknownEither(GitHubShaResponseSchema)(value), { - onLeft: () => { - throw new Error(`${context} missing valid sha`) - }, - onRight: (response) => response.sha - }) +export const decodeGitHubSha = ( + value: unknown, + context: string = "GitHub response" +): Effect.Effect => + decodeUnknownEffect(GitHubShaResponseSchema, context, value).pipe( + Effect.map((response) => response.sha) + ) /** * Decodes GitHub recursive tree responses without silently dropping invalid entries. * * @param value - Unknown GitHub tree API response. - * @returns A readonly collection of tree entries, or null when the response is invalid. + * @returns Effect with a readonly collection of tree entries, or DecodeError when the response is invalid. * @pure false - boundary decoder for GitHub API JSON. - * @effect none - synchronous schema decode only. - * @invariant invalid tree payloads decode to null, never to []. + * @effect Effect, DecodeError, never> + * @invariant invalid tree payloads fail with DecodeError, never silently decode to []. * @precondition value may be any GitHub API JSON response. * @postcondition every returned entry contains path, mode, type, and sha strings. * @complexity O(n) time / O(n) space where n is the number of tree entries. */ -export const decodeGitHubTreeEntries = (value: unknown): ReadonlyArray | null => - Either.match(ParseResult.decodeUnknownEither(GitHubTreeResponseSchema)(value), { - onLeft: () => null, - onRight: (response) => response.tree - }) +export const decodeGitHubTreeEntries = (value: unknown): Effect.Effect, DecodeError> => + decodeUnknownEffect(GitHubTreeResponseSchema, "GitHub tree response", value).pipe( + Effect.map((response) => response.tree) + ) diff --git a/packages/docker-git-session-sync/src/shell.ts b/packages/docker-git-session-sync/src/shell.ts index 106b4270..e5f5bc92 100644 --- a/packages/docker-git-session-sync/src/shell.ts +++ b/packages/docker-git-session-sync/src/shell.ts @@ -3,6 +3,7 @@ import os from "node:os" import path from "node:path" import { spawnSync } from "node:child_process" import { createHash } from "node:crypto" +import { Effect, Either } from "effect" import { backupDefaultBranch, @@ -134,6 +135,20 @@ const ghApiJson = (endpoint: string, ghEnv: GhEnv, options: Parameters(effect: Effect.Effect): A | null => + Either.match(Effect.runSync(Effect.either(effect)), { + onLeft: () => null, + onRight: (value) => value + }) + +const decodeOrThrow = (effect: Effect.Effect, context: string): A => + Either.match(Effect.runSync(Effect.either(effect)), { + onLeft: (error) => { + throw new Error(`${context}: ${errorMessage(error)}`) + }, + onRight: (value) => value + }) + export const runGitCapture = ( cwd: string, args: ReadonlyArray, @@ -173,7 +188,7 @@ export const ensureBackupRepo = (ghEnv: GhEnv, log: Log, createIfMissing: boolea if (!repoResult.success || repoResult.json === null) { return null } - const repoInfo = decodeGitHubRepoInfo(repoResult.json) + const repoInfo = decodeOrNull(decodeGitHubRepoInfo(repoResult.json)) if (repoInfo === null) { log(`GitHub repository response for ${repoFullName} was invalid`) return null @@ -214,10 +229,10 @@ export const getTreeEntries = (repoFullName: string, branch: string, ghEnv: GhEn ghApiJson(`/repos/${repoFullName}/git/trees/${treeSha}?recursive=1`, ghEnv), `failed to list tree for ${repoFullName}@${branch}` ) - const entries = decodeGitHubTreeEntries(result.json) - if (entries === null) { - throw new Error(`GitHub tree response invalid for ${repoFullName}@${branch}`) - } + const entries = decodeOrThrow( + decodeGitHubTreeEntries(result.json), + `GitHub tree response invalid for ${repoFullName}@${branch}` + ) return { headSha, treeSha, @@ -235,7 +250,7 @@ export const getFileContent = ( ghApiJson(`/repos/${repoFullName}/contents/${repoPath}?ref=${encodeURIComponent(ref)}`, ghEnv), `failed to fetch ${repoFullName}:${repoPath}` ) - const contentResponse = decodeGitHubContentResponse(result.json) + const contentResponse = decodeOrNull(decodeGitHubContentResponse(result.json)) if (contentResponse === null) { throw new Error(`unexpected content payload for ${repoFullName}:${repoPath}`) } @@ -258,7 +273,7 @@ export const createPrComment = ( log(`GitHub PR comment API failed for ${repoFullName}#${prNumber}: ${result.stderr || result.stdout || `exit ${result.status}`}`) return null } - const comment = decodeGitHubPrComment(result.json) + const comment = decodeOrNull(decodeGitHubPrComment(result.json)) if (comment === null) { log(`GitHub PR comment response invalid for ${repoFullName}#${prNumber}`) } @@ -560,7 +575,10 @@ const createGitBlob = (repoFullName: string, entry: UploadEntry, ghEnv: GhEnv): }), `failed to create blob for ${repoFullName}:${entry.repoPath}` ) - const sha = decodeGitHubSha(result.json, `GitHub blob response for ${entry.repoPath}`) + const sha = decodeOrThrow( + decodeGitHubSha(result.json, `GitHub blob response for ${entry.repoPath}`), + `GitHub blob response invalid for ${entry.repoPath}` + ) if (sha !== entry.blobSha) { throw new Error(`GitHub blob sha mismatch for ${entry.repoPath}`) } @@ -583,7 +601,10 @@ const createGitTree = ( }), `failed to create tree in ${repoFullName}` ) - return decodeGitHubSha(result.json, `GitHub tree response for ${repoFullName}`) + return decodeOrThrow( + decodeGitHubSha(result.json, `GitHub tree response for ${repoFullName}`), + `GitHub tree response invalid for ${repoFullName}` + ) } const createGitCommit = ( @@ -611,7 +632,10 @@ const createGitCommit = ( }), `failed to create commit in ${backupRepo.fullName}` ) - return decodeGitHubSha(result.json, `GitHub commit response for ${backupRepo.fullName}`) + return decodeOrThrow( + decodeGitHubSha(result.json, `GitHub commit response for ${backupRepo.fullName}`), + `GitHub commit response invalid for ${backupRepo.fullName}` + ) } const updateGitRef = (repoFullName: string, branch: string, commitSha: string, ghEnv: GhEnv): CommandResult => diff --git a/packages/docker-git-session-sync/tests/session-files.test.ts b/packages/docker-git-session-sync/tests/session-files.test.ts index 7ffe2d29..9dbf47c7 100644 --- a/packages/docker-git-session-sync/tests/session-files.test.ts +++ b/packages/docker-git-session-sync/tests/session-files.test.ts @@ -2,6 +2,7 @@ import fs from "node:fs" import os from "node:os" import path from "node:path" import fc from "fast-check" +import { Effect, Either } from "effect" import { afterEach, beforeEach, describe, expect, it } from "vitest" import { @@ -33,6 +34,14 @@ const output: Output = { err: () => undefined } +const decodeSync = (effect: Effect.Effect): A => Effect.runSync(effect) + +const decodeFailureTag = (effect: Effect.Effect): string => + Either.match(Effect.runSync(Effect.either(effect)), { + onLeft: (error) => error._tag, + onRight: () => "Right" + }) + const hexShaArbitrary = fc.array( fc.constantFrom("0", "1", "2", "3", "4", "5", "6", "7", "8", "9", "a", "b", "c", "d", "e", "f"), { minLength: 40, maxLength: 40 } @@ -175,11 +184,12 @@ describe("upload artifacts", () => { describe("snapshot tree updates", () => { it("rejects invalid GitHub tree responses instead of silently dropping entries", () => { - expect(decodeGitHubTreeEntries({ tree: [{ path: "file.txt", mode: "100644", type: "blob" }] })).toBeNull() + expect(decodeFailureTag(decodeGitHubTreeEntries({ tree: [{ path: "file.txt", mode: "100644", type: "blob" }] }))) + .toBe("DecodeError") }) it("decodes valid GitHub tree responses as readonly entries", () => { - expect(decodeGitHubTreeEntries({ + expect(decodeSync(decodeGitHubTreeEntries({ tree: [ { path: "file.txt", @@ -188,7 +198,7 @@ describe("snapshot tree updates", () => { sha: "0123456789abcdef0123456789abcdef01234567" } ] - })).toEqual([ + }))).toEqual([ { path: "file.txt", mode: "100644", @@ -199,15 +209,15 @@ describe("snapshot tree updates", () => { }) it("validates GitHub content encoding and SHA contracts at the schema boundary", () => { - expect(decodeGitHubContentResponse({ encoding: "utf-8", content: "" })).toBeNull() - expect(decodeGitHubContentResponse({ encoding: "base64", content: "" })).toEqual({ + expect(decodeFailureTag(decodeGitHubContentResponse({ encoding: "utf-8", content: "" }))).toBe("DecodeError") + expect(decodeSync(decodeGitHubContentResponse({ encoding: "base64", content: "" }))).toEqual({ encoding: "base64", content: "" }) - expect(decodeGitHubSha({ sha: "0123456789abcdef0123456789abcdef01234567" })).toBe( + expect(decodeSync(decodeGitHubSha({ sha: "0123456789abcdef0123456789abcdef01234567" }))).toBe( "0123456789abcdef0123456789abcdef01234567" ) - expect(() => decodeGitHubSha({ sha: "not-a-sha" })).toThrow("GitHub response missing valid sha") + expect(decodeFailureTag(decodeGitHubSha({ sha: "not-a-sha" }))).toBe("DecodeError") }) it("keeps stale remote session files untouched", () => { diff --git a/packages/lib/src/core/command-builders-template.ts b/packages/lib/src/core/command-builders-template.ts index 7d5f9bdb..d0bcce72 100644 --- a/packages/lib/src/core/command-builders-template.ts +++ b/packages/lib/src/core/command-builders-template.ts @@ -21,6 +21,16 @@ export type BuildTemplateConfigInput = { readonly enableMcpPlaywright: boolean readonly agentMode: AgentMode | undefined readonly agentAuto: boolean + /** + * Hostname where the source project was cloned. + * + * @pure true - immutable template-builder input. + * @effect none + * @invariant if present, template config preserves this value without reading OS hostname. + * @precondition boundary validation rejects malformed hostnames before constructing this input. + * @postcondition buildTemplateConfig propagates the value into docker-git.json. + * @complexity O(1)/O(1) + */ readonly clonedOnHostname?: string | undefined } diff --git a/packages/lib/src/core/command-options.ts b/packages/lib/src/core/command-options.ts index f3073dfc..036e43df 100644 --- a/packages/lib/src/core/command-options.ts +++ b/packages/lib/src/core/command-options.ts @@ -59,7 +59,16 @@ export interface RawOptions { readonly force?: boolean readonly forceEnv?: boolean readonly agentAutoMode?: string - // Hostname where the project was cloned; passed explicitly to keep command builders pure. + /** + * Hostname where the project was cloned; passed explicitly to keep command builders pure. + * + * @pure true - immutable command-builder input. + * @effect none + * @invariant if present, command config preserves this value without reading OS hostname. + * @precondition boundary validation rejects malformed hostnames before constructing RawOptions. + * @postcondition buildCreateCommand propagates the value to docker-git.json. + * @complexity O(1)/O(1) + */ readonly clonedOnHostname?: string // Session gist options (issue-143) readonly prNumber?: string diff --git a/packages/lib/src/usecases/projects-core.ts b/packages/lib/src/usecases/projects-core.ts index 7ea13ea3..e48afa1b 100644 --- a/packages/lib/src/usecases/projects-core.ts +++ b/packages/lib/src/usecases/projects-core.ts @@ -54,6 +54,16 @@ export type ProjectItem = { readonly envProjectPath: string readonly codexAuthPath: string readonly codexHome: string + /** + * Hostname of the machine where this project was cloned, when known. + * + * @pure true - immutable project item DTO field. + * @effect none + * @invariant if present, the value originated from validated project config. + * @precondition omitted when clone host identity is unknown. + * @postcondition list/search callers can display host affinity without reading OS state. + * @complexity O(1)/O(1) + */ readonly clonedOnHostname?: string | undefined readonly lastStartedAtIso: string | null readonly lastStartedAtEpochMs: number | null diff --git a/packages/lib/src/usecases/ssh-access.ts b/packages/lib/src/usecases/ssh-access.ts index 6b0b5c34..23fa225a 100644 --- a/packages/lib/src/usecases/ssh-access.ts +++ b/packages/lib/src/usecases/ssh-access.ts @@ -129,6 +129,19 @@ export const buildEditorSshAccess = ( } } +/** + * Formats a compact editor SSH access summary. + * + * @param access - Validated SSH access details for the project container. + * @param clonedOnHostname - Optional clone-origin hostname used for first-hop guidance. + * @returns Human-readable SSH access summary. + * @pure true - deterministic formatting only. + * @effect none + * @invariant clonedOnHostname is rendered only when present. + * @precondition access fields are already resolved by SSH access discovery. + * @postcondition output contains no OS-derived hostname reads. + * @complexity O(n)/O(n), where n is the rendered access text length. + */ export const formatEditorSshAccessSummary = ( access: EditorSshAccess, clonedOnHostname?: string @@ -141,6 +154,19 @@ Terminal shortcut: ${access.terminalShortcut} Remote workspace: ${access.workspacePath}${firstHopLine}` } +/** + * Formats detailed editor SSH access instructions. + * + * @param access - Validated SSH access details for the project container. + * @param clonedOnHostname - Optional clone-origin hostname used for first-hop guidance. + * @returns Human-readable SSH access details. + * @pure true - deterministic formatting only. + * @effect none + * @invariant clonedOnHostname is rendered only when present. + * @precondition access fields are already resolved by SSH access discovery. + * @postcondition output contains no OS-derived hostname reads. + * @complexity O(n)/O(n), where n is the rendered access text length. + */ export const formatEditorSshAccessDetails = ( access: EditorSshAccess, clonedOnHostname?: string