Skip to content
Merged
Show file tree
Hide file tree
Changes from 12 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,8 @@ We are very very early in this project. Expect bugs.

We are not accepting contributions yet.

Observability guide: [docs/observability.md](./docs/observability.md)

## If you REALLY want to contribute still.... read this first

Read [CONTRIBUTING.md](./CONTRIBUTING.md) before opening an issue or PR.
Expand Down
35 changes: 34 additions & 1 deletion apps/desktop/src/main.ts
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ import { autoUpdater } from "electron-updater";
import type { ContextMenuItem } from "@t3tools/contracts";
import { NetService } from "@t3tools/shared/Net";
import { RotatingFileSink } from "@t3tools/shared/logging";
import { parsePersistedServerObservabilitySettings } from "@t3tools/shared/serverSettings";
import { showDesktopConfirmDialog } from "./confirmDialog";
import { syncShellEnvironment } from "./syncShellEnvironment";
import { getAutoUpdateDisabledReason, shouldBroadcastDownloadProgress } from "./updateState";
Expand Down Expand Up @@ -79,6 +80,7 @@ const LOG_DIR = Path.join(STATE_DIR, "logs");
const LOG_FILE_MAX_BYTES = 10 * 1024 * 1024;
const LOG_FILE_MAX_FILES = 10;
const APP_RUN_ID = Crypto.randomBytes(6).toString("hex");
const SERVER_SETTINGS_PATH = Path.join(STATE_DIR, "settings.json");
const AUTO_UPDATE_STARTUP_DELAY_MS = 15_000;
const AUTO_UPDATE_POLL_INTERVAL_MS = 4 * 60 * 60 * 1000;
const DESKTOP_UPDATE_CHANNEL = "latest";
Expand All @@ -102,8 +104,10 @@ let aboutCommitHashCache: string | null | undefined;
let desktopLogSink: RotatingFileSink | null = null;
let backendLogSink: RotatingFileSink | null = null;
let restoreStdIoCapture: (() => void) | null = null;
let backendObservabilitySettings = readPersistedBackendObservabilitySettings();

let destructiveMenuIconCache: Electron.NativeImage | null | undefined;
const expectedBackendExitChildren = new WeakSet<ChildProcess.ChildProcess>();
const desktopRuntimeInfo = resolveDesktopRuntimeInfo({
platform: process.platform,
processArch: process.arch,
Expand All @@ -124,6 +128,21 @@ function sanitizeLogValue(value: string): string {
return value.replace(/\s+/g, " ").trim();
}

function readPersistedBackendObservabilitySettings(): {
readonly otlpTracesUrl: string | undefined;
readonly otlpMetricsUrl: string | undefined;
} {
try {
if (!FS.existsSync(SERVER_SETTINGS_PATH)) {
return { otlpTracesUrl: undefined, otlpMetricsUrl: undefined };
}
return parsePersistedServerObservabilitySettings(FS.readFileSync(SERVER_SETTINGS_PATH, "utf8"));
} catch (error) {
console.warn("[desktop] failed to read persisted backend observability settings", error);
return { otlpTracesUrl: undefined, otlpMetricsUrl: undefined };
}
}

function backendChildEnv(): NodeJS.ProcessEnv {
const env = { ...process.env };
delete env.T3CODE_PORT;
Expand Down Expand Up @@ -964,6 +983,7 @@ function scheduleBackendRestart(reason: string): void {
function startBackend(): void {
if (isQuitting || backendProcess) return;

backendObservabilitySettings = readPersistedBackendObservabilitySettings();
const backendEntry = resolveBackendEntry();
if (!FS.existsSync(backendEntry)) {
scheduleBackendRestart(`missing server entry at ${backendEntry}`);
Expand Down Expand Up @@ -992,6 +1012,12 @@ function startBackend(): void {
port: backendPort,
t3Home: BASE_DIR,
authToken: backendAuthToken,
...(backendObservabilitySettings.otlpTracesUrl
? { otlpTracesUrl: backendObservabilitySettings.otlpTracesUrl }
: {}),
...(backendObservabilitySettings.otlpMetricsUrl
? { otlpMetricsUrl: backendObservabilitySettings.otlpMetricsUrl }
: {}),
})}\n`,
);
bootstrapStream.end();
Expand All @@ -1018,21 +1044,26 @@ function startBackend(): void {
});

child.on("error", (error) => {
const wasExpected = expectedBackendExitChildren.has(child);
if (backendProcess === child) {
backendProcess = null;
}
closeBackendSession(`pid=${child.pid ?? "unknown"} error=${error.message}`);
if (wasExpected) {
return;
}
scheduleBackendRestart(error.message);
});

child.on("exit", (code, signal) => {
const wasExpected = expectedBackendExitChildren.has(child);
if (backendProcess === child) {
backendProcess = null;
}
closeBackendSession(
`pid=${child.pid ?? "unknown"} code=${code ?? "null"} signal=${signal ?? "null"}`,
);
if (isQuitting) return;
if (isQuitting || wasExpected) return;
const reason = `code=${code ?? "null"} signal=${signal ?? "null"}`;
scheduleBackendRestart(reason);
});
Expand All @@ -1049,6 +1080,7 @@ function stopBackend(): void {
if (!child) return;

if (child.exitCode === null && child.signalCode === null) {
expectedBackendExitChildren.add(child);
child.kill("SIGTERM");
setTimeout(() => {
if (child.exitCode === null && child.signalCode === null) {
Expand All @@ -1069,6 +1101,7 @@ async function stopBackendAndWaitForExit(timeoutMs = 5_000): Promise<void> {
if (!child) return;
const backendChild = child;
if (backendChild.exitCode !== null || backendChild.signalCode !== null) return;
expectedBackendExitChildren.add(backendChild);

await new Promise<void>((resolve) => {
let settled = false;
Expand Down
84 changes: 84 additions & 0 deletions apps/server/src/cli-config.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,18 @@ import { deriveServerPaths } from "./config";
import { resolveServerConfig } from "./cli";

it.layer(NodeServices.layer)("cli config resolution", (it) => {
const defaultObservabilityConfig = {
traceMinLevel: "Info",
traceTimingEnabled: true,
traceBatchWindowMs: 200,
traceMaxBytes: 10 * 1024 * 1024,
traceMaxFiles: 10,
otlpTracesUrl: undefined,
otlpMetricsUrl: undefined,
otlpExportIntervalMs: 10_000,
otlpServiceName: "t3-server",
} as const;

const openBootstrapFd = Effect.fn(function* (payload: Record<string, unknown>) {
const fs = yield* FileSystem.FileSystem;
const filePath = yield* fs.makeTempFileScoped({ prefix: "t3-bootstrap-", suffix: ".ndjson" });
Expand Down Expand Up @@ -62,6 +74,7 @@ it.layer(NodeServices.layer)("cli config resolution", (it) => {

expect(resolved).toEqual({
logLevel: "Warn",
...defaultObservabilityConfig,
mode: "desktop",
port: 4001,
cwd: process.cwd(),
Expand Down Expand Up @@ -123,6 +136,7 @@ it.layer(NodeServices.layer)("cli config resolution", (it) => {

expect(resolved).toEqual({
logLevel: "Debug",
...defaultObservabilityConfig,
mode: "web",
port: 8788,
cwd: process.cwd(),
Expand Down Expand Up @@ -153,6 +167,8 @@ it.layer(NodeServices.layer)("cli config resolution", (it) => {
authToken: "bootstrap-token",
autoBootstrapProjectFromCwd: false,
logWebSocketEvents: true,
otlpTracesUrl: "http://localhost:4318/v1/traces",
otlpMetricsUrl: "http://localhost:4318/v1/metrics",
});
const derivedPaths = yield* deriveServerPaths(baseDir, new URL("http://127.0.0.1:5173"));

Expand Down Expand Up @@ -187,6 +203,9 @@ it.layer(NodeServices.layer)("cli config resolution", (it) => {

expect(resolved).toEqual({
logLevel: "Info",
...defaultObservabilityConfig,
otlpTracesUrl: "http://localhost:4318/v1/traces",
otlpMetricsUrl: "http://localhost:4318/v1/metrics",
mode: "desktop",
port: 4888,
cwd: process.cwd(),
Expand Down Expand Up @@ -241,6 +260,7 @@ it.layer(NodeServices.layer)("cli config resolution", (it) => {
resolved.attachmentsDir,
resolved.worktreesDir,
path.dirname(resolved.serverLogPath),
path.dirname(resolved.serverTracePath),
]) {
expect(yield* fs.exists(directory)).toBe(true);
}
Expand Down Expand Up @@ -300,6 +320,7 @@ it.layer(NodeServices.layer)("cli config resolution", (it) => {

expect(resolved).toEqual({
logLevel: "Debug",
...defaultObservabilityConfig,
mode: "web",
port: 8788,
cwd: process.cwd(),
Expand All @@ -315,4 +336,67 @@ it.layer(NodeServices.layer)("cli config resolution", (it) => {
});
}),
);

it.effect("falls back to persisted observability settings when env vars are absent", () =>
Effect.gen(function* () {
const fs = yield* FileSystem.FileSystem;
const path = yield* Path.Path;
const baseDir = yield* fs.makeTempDirectoryScoped({ prefix: "t3-cli-config-settings-" });
const derivedPaths = yield* deriveServerPaths(baseDir, undefined);
yield* fs.makeDirectory(path.dirname(derivedPaths.settingsPath), { recursive: true });
yield* fs.writeFileString(
derivedPaths.settingsPath,
`${JSON.stringify({
observability: {
otlpTracesUrl: "http://localhost:4318/v1/traces",
otlpMetricsUrl: "http://localhost:4318/v1/metrics",
},
})}\n`,
);

const resolved = yield* resolveServerConfig(
{
mode: Option.some("desktop"),
port: Option.some(4888),
host: Option.none(),
baseDir: Option.some(baseDir),
devUrl: Option.none(),
noBrowser: Option.none(),
authToken: Option.none(),
bootstrapFd: Option.none(),
autoBootstrapProjectFromCwd: Option.none(),
logWebSocketEvents: Option.none(),
},
Option.none(),
).pipe(
Effect.provide(
Layer.mergeAll(
ConfigProvider.layer(ConfigProvider.fromEnv({ env: {} })),
NetService.layer,
),
),
);

expect(resolved.otlpTracesUrl).toBe("http://localhost:4318/v1/traces");
expect(resolved.otlpMetricsUrl).toBe("http://localhost:4318/v1/metrics");
expect(resolved).toEqual({
logLevel: "Info",
...defaultObservabilityConfig,
otlpTracesUrl: "http://localhost:4318/v1/traces",
otlpMetricsUrl: "http://localhost:4318/v1/metrics",
mode: "desktop",
port: 4888,
cwd: process.cwd(),
baseDir,
...derivedPaths,
host: "127.0.0.1",
staticDir: resolved.staticDir,
devUrl: undefined,
noBrowser: true,
authToken: undefined,
autoBootstrapProjectFromCwd: false,
logWebSocketEvents: false,
});
}),
);
});
68 changes: 67 additions & 1 deletion apps/server/src/cli.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import { NetService } from "@t3tools/shared/Net";
import { Config, Effect, LogLevel, Option, Schema } from "effect";
import { parsePersistedServerObservabilitySettings } from "@t3tools/shared/serverSettings";
import { Config, Effect, FileSystem, LogLevel, Option, Path, Schema } from "effect";
import { Command, Flag, GlobalFlag } from "effect/unstable/cli";

import {
Expand Down Expand Up @@ -27,6 +28,8 @@ const BootstrapEnvelopeSchema = Schema.Struct({
authToken: Schema.optional(Schema.String),
autoBootstrapProjectFromCwd: Schema.optional(Schema.Boolean),
logWebSocketEvents: Schema.optional(Schema.Boolean),
otlpTracesUrl: Schema.optional(Schema.String),
otlpMetricsUrl: Schema.optional(Schema.String),
Comment on lines +31 to +32
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Validate the new observability settings at the boundary.

otlpTracesUrl / otlpMetricsUrl are accepted as arbitrary strings, and the new sizing/interval knobs have no lower-bound checks. A malformed endpoint or negative interval/rotation value now gets through config resolution and only fails later in the tracer/exporter.

Also applies to: 93-107

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@apps/server/src/cli.ts` around lines 31 - 32, Validate otlpTracesUrl and
otlpMetricsUrl at config boundary instead of Schema.String: replace
Schema.optional(Schema.String) with a URL-aware validator (e.g.,
Schema.optional(Schema.Url) or Schema.optional(Schema.String.refine(s => { try {
new URL(s); return true } catch { return false } }))) so malformed endpoints are
rejected early, and add numeric lower-bound checks for the sizing/interval knobs
(use Schema.Number or Schema.Int with .min(0) or .greaterThan(0) as appropriate)
so negative or zero rotation/interval values are rejected during config parsing;
update the config schema definitions where otlpTracesUrl and otlpMetricsUrl are
declared and where the interval/rotation knobs are defined so validation fails
fast.

});

const modeFlag = Flag.choice("mode", RuntimeMode.literals).pipe(
Expand Down Expand Up @@ -81,6 +84,27 @@ const logWebSocketEventsFlag = Flag.boolean("log-websocket-events").pipe(

const EnvServerConfig = Config.all({
logLevel: Config.logLevel("T3CODE_LOG_LEVEL").pipe(Config.withDefault("Info")),
traceMinLevel: Config.logLevel("T3CODE_TRACE_MIN_LEVEL").pipe(Config.withDefault("Info")),
traceTimingEnabled: Config.boolean("T3CODE_TRACE_TIMING_ENABLED").pipe(Config.withDefault(true)),
traceFile: Config.string("T3CODE_TRACE_FILE").pipe(
Config.option,
Config.map(Option.getOrUndefined),
),
traceMaxBytes: Config.int("T3CODE_TRACE_MAX_BYTES").pipe(Config.withDefault(10 * 1024 * 1024)),
traceMaxFiles: Config.int("T3CODE_TRACE_MAX_FILES").pipe(Config.withDefault(10)),
traceBatchWindowMs: Config.int("T3CODE_TRACE_BATCH_WINDOW_MS").pipe(Config.withDefault(200)),
otlpTracesUrl: Config.string("T3CODE_OTLP_TRACES_URL").pipe(
Config.option,
Config.map(Option.getOrUndefined),
),
otlpMetricsUrl: Config.string("T3CODE_OTLP_METRICS_URL").pipe(
Config.option,
Config.map(Option.getOrUndefined),
),
otlpExportIntervalMs: Config.int("T3CODE_OTLP_EXPORT_INTERVAL_MS").pipe(
Config.withDefault(10_000),
),
otlpServiceName: Config.string("T3CODE_OTLP_SERVICE_NAME").pipe(Config.withDefault("t3-server")),
mode: Config.schema(RuntimeMode, "T3CODE_MODE").pipe(
Config.option,
Config.map(Option.getOrUndefined),
Expand Down Expand Up @@ -131,12 +155,25 @@ const resolveOptionPrecedence = <Value>(
...values: ReadonlyArray<Option.Option<Value>>
): Option.Option<Value> => Option.firstSomeOf(values);

const loadPersistedObservabilitySettings = Effect.fn(function* (settingsPath: string) {
const fs = yield* FileSystem.FileSystem;
const exists = yield* fs.exists(settingsPath).pipe(Effect.orElseSucceed(() => false));
if (!exists) {
return { otlpTracesUrl: undefined, otlpMetricsUrl: undefined };
}

const raw = yield* fs.readFileString(settingsPath).pipe(Effect.orElseSucceed(() => ""));
return parsePersistedServerObservabilitySettings(raw);
Comment on lines +158 to +166
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Don't silently discard persisted OTLP settings on read failures.

After Line 160 confirms the file exists, Line 165 turns every readFileString failure into "". Permission problems or TOCTOU races then look like “no settings”, which breaks the restart-persistence guarantee and hides the real fault.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@apps/server/src/cli.ts` around lines 158 - 166, The function
loadPersistedObservabilitySettings currently masks all read errors by converting
readFileString failures to an empty string, which hides permission/toctou errors
after exists returned true; change the error handling so that after confirming
the file exists you do not swallow read errors — for example, remove the
.pipe(Effect.orElseSucceed(() => "")) on fs.readFileString and instead let the
Effect fail (or convert it to a descriptive error via Effect.mapError) so
callers can observe/report the real failure; locate the read call to
FileSystem.readFileString inside loadPersistedObservabilitySettings and replace
the silent fallback with error propagation or a mapped error that includes the
path.

});

export const resolveServerConfig = (
flags: CliServerFlags,
cliLogLevel: Option.Option<LogLevel.LogLevel>,
) =>
Effect.gen(function* () {
const { findAvailablePort } = yield* NetService;
const path = yield* Path.Path;
const fs = yield* FileSystem.FileSystem;
const env = yield* EnvServerConfig;
const bootstrapFd = Option.getOrUndefined(flags.bootstrapFd) ?? env.bootstrapFd;
const bootstrapEnvelope =
Expand Down Expand Up @@ -190,6 +227,11 @@ export const resolveServerConfig = (
);
const derivedPaths = yield* deriveServerPaths(baseDir, devUrl);
yield* ensureServerDirectories(derivedPaths);
const persistedObservabilitySettings = yield* loadPersistedObservabilitySettings(
derivedPaths.settingsPath,
);
const serverTracePath = env.traceFile ?? derivedPaths.serverTracePath;
yield* fs.makeDirectory(path.dirname(serverTracePath), { recursive: true });
const noBrowser = resolveBooleanFlag(
flags.noBrowser,
Option.getOrElse(
Expand Down Expand Up @@ -248,11 +290,35 @@ export const resolveServerConfig = (

const config: ServerConfigShape = {
logLevel,
traceMinLevel: env.traceMinLevel,
traceTimingEnabled: env.traceTimingEnabled,
traceBatchWindowMs: env.traceBatchWindowMs,
traceMaxBytes: env.traceMaxBytes,
traceMaxFiles: env.traceMaxFiles,
otlpTracesUrl:
env.otlpTracesUrl ??
Option.getOrUndefined(
Option.flatMap(bootstrapEnvelope, (bootstrap) =>
Option.fromUndefinedOr(bootstrap.otlpTracesUrl),
),
) ??
persistedObservabilitySettings.otlpTracesUrl,
otlpMetricsUrl:
env.otlpMetricsUrl ??
Option.getOrUndefined(
Option.flatMap(bootstrapEnvelope, (bootstrap) =>
Option.fromUndefinedOr(bootstrap.otlpMetricsUrl),
),
) ??
persistedObservabilitySettings.otlpMetricsUrl,
otlpExportIntervalMs: env.otlpExportIntervalMs,
otlpServiceName: env.otlpServiceName,
mode,
port,
cwd: process.cwd(),
baseDir,
...derivedPaths,
serverTracePath,
host,
staticDir,
devUrl,
Expand Down
Loading
Loading