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

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
63 changes: 63 additions & 0 deletions daemon/main.ts
Original file line number Diff line number Diff line change
Expand Up @@ -33,16 +33,21 @@ import {
} from "./monitor.ts";
import { downloadCache } from "./cache.ts";
import { createAIHandlers } from "./ai/handlers.ts";
import { phaseEnd, phaseStart, phaseSummary } from "./phases.ts";
import { createSandboxHandlers, type DeployParams } from "./sandbox.ts";
import { register, type TunnelConnection } from "./tunnel.ts";
import {
createWorker,
isWorkerEverReady,
resetWorkerState,
waitForFirstReady,
worker,
type WorkerOptions,
} from "./worker.ts";
import { portPool } from "./workers/portpool.ts";

phaseStart("daemon_boot");

const parsedArgs = parseArgs(Deno.args, {
string: ["build-cmd"],
});
Expand Down Expand Up @@ -283,6 +288,7 @@ const createDeps = (
throw new Error("Cannot initialize deps: site name not set");
}
let start = performance.now();
phaseStart("git_setup");
try {
await ensureGit({
site: siteName,
Expand All @@ -291,9 +297,11 @@ const createDeps = (
});
readyResolve();
} catch (err) {
phaseEnd("git_setup", "fail");
readyReject(err);
throw err;
}
phaseEnd("git_setup");
logs.push({
level: "info",
message: `${colors.bold("[step 1/4]")}: Git setup took ${
Expand All @@ -305,9 +313,11 @@ const createDeps = (

if (SANDBOX_MODE) {
start = performance.now();
phaseStart("cache_download");
await downloadCache(siteName).catch((err) => {
console.warn(`[cache] Failed to download build cache: ${err.message}`);
});
phaseEnd("cache_download");
Comment on lines 314 to +320

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 | ⚡ Quick win

Mark failed cache downloads explicitly.

The cache download error is swallowed, but the phase is still ended with the default ok outcome. That makes the startup metrics report success even when the cache was unavailable.

Suggested fix
-      await downloadCache(siteName).catch((err) => {
-        console.warn(`[cache] Failed to download build cache: ${err.message}`);
-      });
-      phaseEnd("cache_download");
+      const cacheOutcome = await downloadCache(siteName)
+        .then(() => "ok" as const)
+        .catch((err) => {
+          console.warn(
+            `[cache] Failed to download build cache: ${err.message}`,
+          );
+          return "fail" as const;
+        });
+      phaseEnd("cache_download", cacheOutcome);
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@daemon/main.ts` around lines 314 - 320, The cache download errors are being
swallowed and phaseEnd("cache_download") is still called as if successful;
update the SANDBOX_MODE block around downloadCache(siteName) so the catch
captures the error, logs it, and calls phaseEnd with a non-ok outcome (e.g.,
phaseEnd("cache_download", "failure") or the appropriate failure payload your
phaseEnd accepts) instead of the default success; reference SANDBOX_MODE,
downloadCache, phaseStart, and phaseEnd to locate and change the code.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

P2: The cache download error is caught and swallowed, but phaseEnd("cache_download") still runs with the default "ok" outcome. This causes startup metrics to report a successful cache download even when it failed. Capture the outcome from the .catch() and pass it to phaseEnd.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At daemon/main.ts, line 320:

<comment>The cache download error is caught and swallowed, but `phaseEnd("cache_download")` still runs with the default `"ok"` outcome. This causes startup metrics to report a successful cache download even when it failed. Capture the outcome from the `.catch()` and pass it to `phaseEnd`.</comment>

<file context>
@@ -305,9 +313,11 @@ const createDeps = (
       await downloadCache(siteName).catch((err) => {
         console.warn(`[cache] Failed to download build cache: ${err.message}`);
       });
+      phaseEnd("cache_download");
       logs.push({
         level: "info",
</file context>

logs.push({
level: "info",
message: `${colors.bold("[step 1.5/4]")}: Cache download took ${
Expand All @@ -317,7 +327,9 @@ const createDeps = (
}

start = performance.now();
phaseStart("manifest_gen");
await genManifestTS();
phaseEnd("manifest_gen");
logs.push({
level: "info",
message: `${colors.bold("[step 2/4]")}: Manifest generation took ${
Expand All @@ -328,7 +340,9 @@ const createDeps = (
});

start = performance.now();
phaseStart("blocks_metadata");
await genBlocksJSON();
phaseEnd("blocks_metadata");
logs.push({
level: "info",
message: `${colors.bold("[step 3/4]")}: Blocks metadata generation took ${
Expand Down Expand Up @@ -550,6 +564,54 @@ app.get("/_healthcheck", (c) => {
},
});
});

/**
* /_ready waits server-side until the inner worker has actually responded
* to traffic (dispatchWorkerState("ready") fires after meta.ts's first
* successful /deco/meta fetch). Unlike /_healthcheck — which goes 200 the
* moment the daemon's Hono server binds — this is a true readiness signal
* for the user-facing Fresh app.
*
* Returns { ready, version, phases } so the admin can record startup
* timings without parsing logs. Honors ?timeout=<ms> (default 30s).
*/
const READY_DEFAULT_TIMEOUT_MS = 30_000;
const READY_MAX_TIMEOUT_MS = 120_000;

app.get("/_ready", async (c) => {
const requested = Number(c.req.query("timeout"));
const timeoutMs = Number.isFinite(requested) && requested > 0
? Math.min(requested, READY_MAX_TIMEOUT_MS)
: READY_DEFAULT_TIMEOUT_MS;

const headers = {
"Access-Control-Allow-Origin": "*",
"Access-Control-Allow-Methods": "GET",
"Access-Control-Allow-Headers": "Content-Type",
"Cache-Control": "no-store",
"Content-Type": "application/json",
};

const respond = (ready: boolean) =>
new Response(
JSON.stringify({
ready,
version: denoJSON.version,
phases: phaseSummary(),
}),
{ status: ready ? 200 : 503, headers },
);

if (isWorkerEverReady()) return respond(true);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

P2: /_ready can report a false positive after sandbox redeploys because it short-circuits on a global everReady flag that is never reset by resetWorkerState().

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At daemon/main.ts, line 605:

<comment>`/_ready` can report a false positive after sandbox redeploys because it short-circuits on a global `everReady` flag that is never reset by `resetWorkerState()`.</comment>

<file context>
@@ -550,6 +564,54 @@ app.get("/_healthcheck", (c) => {
+      { status: ready ? 200 : 503, headers },
+    );
+
+  if (isWorkerEverReady()) return respond(true);
+
+  const ready = await Promise.race([
</file context>


const ready = await Promise.race([
waitForFirstReady().then(() => true as const),
delay(timeoutMs).then(() => false as const),
]);

return respond(ready);
});

// k8s liveness probe
app.get("/deco/_liveness", () => new Response("OK", { status: 200 }));

Expand Down Expand Up @@ -844,6 +906,7 @@ Deno.serve(
{
port,
onListen: async (addr) => {
phaseEnd("daemon_boot");
try {
const siteName = !SANDBOX_MODE ? getSiteName() : undefined;
const tunnel = siteName ? await registerTunnel(siteName) : null;
Expand Down
95 changes: 95 additions & 0 deletions daemon/phases.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,95 @@
/**
* Tracks daemon startup phases with millisecond timestamps and emits a single
* `[phase] phase=<name> event=start|end ts=<ms> [duration_ms=<n>] [...]`
* log line per transition. The same line format is used by the env app
* container's startup script in deco-sites/admin, so a single grep across
* pod logs reconstructs the full timeline.
*
* Phase data is also exposed through the daemon's /_ready endpoint so the
* admin can record per-startup timings without parsing logs.
*/

interface PhaseRecord {
startMs: number;
endMs?: number;
durationMs?: number;
outcome?: string;
}

const phases: Record<string, PhaseRecord> = {};
const bootStartMs = Date.now();

const formatExtra = (extra?: Record<string, unknown>): string => {
if (!extra) return "";
const parts: string[] = [];
for (const [k, v] of Object.entries(extra)) {
if (v === undefined || v === null) continue;
parts.push(`${k}=${v}`);
}
return parts.length > 0 ? " " + parts.join(" ") : "";
};

export const phaseStart = (
name: string,
extra?: Record<string, unknown>,
): void => {
const ts = Date.now();
phases[name] = { startMs: ts };
console.log(
`[phase] phase=${name} event=start ts=${ts}${formatExtra(extra)}`,
);
};

export const phaseEnd = (
name: string,
outcome: "ok" | "fail" | "skipped" = "ok",
extra?: Record<string, unknown>,
): void => {
const ts = Date.now();
const rec = phases[name];
if (!rec) {
phases[name] = {
startMs: ts,
endMs: ts,
durationMs: 0,
outcome,
};
console.log(
`[phase] phase=${name} event=end ts=${ts} duration_ms=0 outcome=${outcome}${
formatExtra(extra)
}`,
);
return;
}
rec.endMs = ts;
rec.durationMs = ts - rec.startMs;
rec.outcome = outcome;
console.log(
`[phase] phase=${name} event=end ts=${ts} duration_ms=${rec.durationMs} outcome=${outcome}${
formatExtra(extra)
}`,
);
};

export interface PhaseSummary {
bootStartMs: number;
uptimeMs: number;
phases: Record<
string,
{ durationMs?: number; outcome?: string; pending?: boolean }
>;
}

export const phaseSummary = (): PhaseSummary => {
const out: PhaseSummary["phases"] = {};
for (const [name, rec] of Object.entries(phases)) {
out[name] = rec.endMs === undefined
? { pending: true }
: { durationMs: rec.durationMs, outcome: rec.outcome };
}
return {
bootStartMs,
uptimeMs: Date.now() - bootStartMs,
phases: out,
};
};
21 changes: 21 additions & 0 deletions daemon/worker.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import { Hono } from "@hono/hono";
import { phaseEnd } from "./phases.ts";
import { broadcast } from "./sse/channel.ts";
import { DenoRun } from "./workers/denoRun.ts";

Expand All @@ -21,9 +22,29 @@ export type WorkerStatus = { state: "updating" | "ready" };

const workerState: WorkerStatus = { state: "updating" };

const firstReady = Promise.withResolvers<void>();
// Prevent unhandled rejection if the daemon shuts down before the worker
// ever becomes ready — callers handle the race against their own timeouts.
firstReady.promise.catch(() => {});
let everReady = false;
Comment on lines +25 to +29

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 | ⚡ Quick win

Reset the readiness state on undeploy.

resetWorkerState() only recreates wp; firstReady and everReady stay latched across deploys. After a sandbox undeploy/redeploy, / _ready will short-circuit to 200 before the new worker has actually become ready.

Suggested fix
-const firstReady = Promise.withResolvers<void>();
+let firstReady = Promise.withResolvers<void>();
 ...
 export const resetWorkerState = () => {
   workerInitFailed = false;
   wp = makeWp();
+  firstReady = Promise.withResolvers<void>();
+  firstReady.promise.catch(() => {});
+  everReady = false;
 };

Also applies to: 72-75

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

In `@daemon/worker.ts` around lines 25 - 29, resetWorkerState() currently only
recreates the worker proxy (wp) but leaves the readiness latch variables
firstReady and everReady unchanged, causing /_ready to short-circuit after
undeploy/redeploy; update resetWorkerState() to reinitialize firstReady to a new
Promise.withResolvers<void>() and set everReady = false, and immediately attach
firstReady.promise.catch(() => {}) to prevent unhandled rejections so the
readiness logic (firstReady, everReady, and any code that resolves firstReady)
reflects the new worker lifecycle.


/**
* Resolves the first time the worker is observed serving traffic
* (dispatchWorkerState("ready") fired by meta.ts after a successful
* /deco/meta fetch). Subsequent transitions don't re-resolve.
*/
export const waitForFirstReady = (): Promise<void> => firstReady.promise;

export const isWorkerEverReady = (): boolean => everReady;

export const dispatchWorkerState = (state: "ready" | "updating") => {
workerState.state = state;
broadcast({ type: "worker-status", detail: workerState });
if (state === "ready" && !everReady) {
everReady = true;
phaseEnd("worker_ready");

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

P2: phaseEnd("worker_ready") is called here without a corresponding phaseStart("worker_ready") anywhere. The phaseEnd implementation bootstraps a missing record with durationMs: 0, so the readiness phase will always report zero duration instead of actual time-to-ready. Add a phaseStart("worker_ready") call at the point the worker is created/started.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At daemon/worker.ts, line 45:

<comment>`phaseEnd("worker_ready")` is called here without a corresponding `phaseStart("worker_ready")` anywhere. The `phaseEnd` implementation bootstraps a missing record with `durationMs: 0`, so the readiness phase will always report zero duration instead of actual time-to-ready. Add a `phaseStart("worker_ready")` call at the point the worker is created/started.</comment>

<file context>
@@ -21,9 +22,29 @@ export type WorkerStatus = { state: "updating" | "ready" };
   broadcast({ type: "worker-status", detail: workerState });
+  if (state === "ready" && !everReady) {
+    everReady = true;
+    phaseEnd("worker_ready");
+    firstReady.resolve();
+  }
</file context>

firstReady.resolve();
}
Comment on lines +43 to +47

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 | ⚡ Quick win

Start worker_ready before ending it.

phaseEnd("worker_ready") currently runs without a matching phaseStart(), and phaseEnd() bootstraps missing records with durationMs: 0. That makes the new readiness payload report a zero-length ready phase instead of time-to-ready.

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

In `@daemon/worker.ts` around lines 43 - 47, The ready phase is being ended
without ever being started, so change the block that handles state === "ready"
&& !everReady to call phaseStart("worker_ready") before calling
phaseEnd("worker_ready") (i.e., invoke phaseStart("worker_ready") then
phaseEnd("worker_ready") and only then set everReady = true and call
firstReady.resolve()); this ensures the ready phase records a real duration
instead of bootstrapping a zero-length record; reference the variables/state
names state, everReady, phaseStart, phaseEnd, and firstReady.resolve in the
worker.ts block.

};

export const start = (): WorkerStatusEvent => ({
Expand Down
Loading