feat: Refactor goal advancement logic and integrate cron scheduling

Author: gandazgul

.gitignore

@@ -19,6 +19,7 @@ dist/
 
 # OS
 .DS_Store
+.history/
 
 # playwright
 .playwright-cli/

Containerfile

@@ -52,20 +52,15 @@ COPY --from=builder --chown=nonroot:nonroot /app/deno.json /app/deno.lock ./
 # Copy migrations for runtime execution
 COPY --from=builder --chown=nonroot:nonroot /app/src/db/migrations ./src/db/migrations
 
-# Copy the advance cron script for runtime execution
-COPY --from=builder --chown=nonroot:nonroot /app/src/server/advance.js ./src/server/advance.js
-
 # Copy node_modules for native bindings compatibility
 COPY --from=builder --chown=nonroot:nonroot /app/node_modules ./node_modules
 
-RUN mkdir -p /app/logs
-
 ENV HOST=0.0.0.0
 ENV PORT=8080
 
 EXPOSE 8080
 
-# Run the Astro server with all permissions inside the container sandbox.
-# The advance cron runs in the background; nohup ensures it survives
-# if the main process restarts (container lifecycle events).
-CMD ["/bin/sh", "-c", "nohup /bin/deno run -A /app/src/server/advance.js > /app/logs/advance.log 2>&1 & /bin/deno run -A /app/server/entry.mjs"]
+# The daily advance cron is bootstrapped from src/middleware.js inside the
+# Astro server process — no sidecar / shell wrapper needed (distroless/cc
+# has no /bin/sh).
+CMD ["/bin/deno", "run", "-A", "/app/server/entry.mjs"]

deno.json

@@ -22,8 +22,8 @@
     "build": "deno run -A --env npm:astro build",
     "preview": "deno run -A --env npm:astro preview",
     "astro": "deno run -A --env npm:astro",
-    "test": "deno test -A",
-    "ci": "deno lint && deno fmt --check && deno check && deno test -A"
+    "test": "DB_PATH=:memory: deno test -A",
+    "ci": "deno lint && deno fmt --check && deno check --check-js --doc && deno run test"
   },
   "fmt": {
     "exclude": [

src/db/index.js

@@ -9,6 +9,11 @@ const { DatabaseSync } = await importDynamic("node:sqlite");
 const dbPath = process.env.DB_PATH || "goaly.db";
 export const db = new DatabaseSync(dbPath);
 
+// SQLite ships with foreign_keys disabled per-connection; without this the
+// ON DELETE CASCADE clauses in 001_initial.sql are silently ignored and
+// orphan rows can be inserted via bad joins.
+db.exec("PRAGMA foreign_keys = ON;");
+
 // Initialize migrations table
 db.exec(`
   CREATE TABLE IF NOT EXISTS migrations (

src/lib/advance.js

@@ -7,8 +7,12 @@ import { scheduleGoal } from "./scheduler.js";
  * new instances for each affected goal.
  *
  * Called by the daily cron and by the instance update API.
+ *
+ * @param {(user: any, goal: any) => Promise<void>} [scheduleFn] - Optional
+ *   scheduler injection point. Defaults to the real Google-Calendar-backed
+ *   scheduler; tests pass a stub to avoid network I/O.
  */
-export async function advanceGoalInstances() {
+export async function advanceGoalInstances(scheduleFn = scheduleGoal) {
   const nowIso = dayjs.utc().toISOString();
 
   // Find all pending instances whose start_time has passed
@@ -60,46 +64,67 @@ export async function advanceGoalInstances() {
       WHERE id IN (${placeholders})
     `).run(...data.instances);
 
-    // Update last_advance_at tracking column right after marking as missed
-    db.prepare(`
-      UPDATE goals SET last_advance_at = ? WHERE id = ?
-    `).run(nowIso, goalId);
-
-    // Fetch the full user and goal details for scheduling
-    const goalRow = db.prepare(`
-      SELECT g.*, u.*
+    // Fetch the full user and goal details for scheduling.
+    // Explicit aliases are required: goals.id and users.id would collide
+    // under SELECT g.*, u.* and the second wins, corrupting goal.id.
+    const row = db.prepare(`
+      SELECT
+        g.id              AS goal_id,
+        g.name            AS goal_name,
+        g.times_per_week,
+        g.duration_minutes,
+        g.time_preference,
+        g.color,
+        g.icon,
+        u.id              AS user_id,
+        u.email,
+        u.google_id,
+        u.access_token,
+        u.refresh_token,
+        u.gotify_url,
+        u.gotify_token,
+        u.timezone,
+        u.morning_start,
+        u.afternoon_start,
+        u.evening_start,
+        u.night_start
       FROM goals g
       JOIN users u ON g.user_id = u.id
       WHERE g.id = ?
     `).get(goalId);
 
-    if (goalRow) {
+    if (row) {
       const goal = {
-        id: goalRow.id,
-        name: goalRow.name,
-        times_per_week: goalRow.times_per_week,
-        duration_minutes: goalRow.duration_minutes,
-        time_preference: goalRow.time_preference,
-        color: goalRow.color,
-        icon: goalRow.icon,
+        id: row.goal_id,
+        name: row.goal_name,
+        times_per_week: row.times_per_week,
+        duration_minutes: row.duration_minutes,
+        time_preference: row.time_preference,
+        color: row.color,
+        icon: row.icon,
       };
       const user = {
-        id: goalRow.user_id,
-        email: goalRow.email,
-        google_id: goalRow.google_id,
-        access_token: goalRow.access_token,
-        refresh_token: goalRow.refresh_token,
-        gotify_url: goalRow.gotify_url,
-        gotify_token: goalRow.gotify_token,
-        timezone: goalRow.timezone,
-        morning_start: goalRow.morning_start,
-        afternoon_start: goalRow.afternoon_start,
-        evening_start: goalRow.evening_start,
-        night_start: goalRow.night_start,
+        id: row.user_id,
+        email: row.email,
+        google_id: row.google_id,
+        access_token: row.access_token,
+        refresh_token: row.refresh_token,
+        gotify_url: row.gotify_url,
+        gotify_token: row.gotify_token,
+        timezone: row.timezone,
+        morning_start: row.morning_start,
+        afternoon_start: row.afternoon_start,
+        evening_start: row.evening_start,
+        night_start: row.night_start,
       };
 
       try {
-        await scheduleGoal(user, goal);
+        await scheduleFn(user, goal);
+        // Only record advancement after scheduling succeeds — otherwise
+        // the tracking column lies and a stalled goal looks healthy.
+        db.prepare(
+          `UPDATE goals SET last_advance_at = ? WHERE id = ?`,
+        ).run(nowIso, goalId);
         advancedGoalIds.push(goalId);
       } catch (err) {
         console.error(

src/lib/advance.test.js

@@ -1,7 +1,27 @@
 import { strict as assert } from "node:assert";
-import { db } from "../db/index.js";
-import { advanceGoalInstances } from "./advance.js";
-import dayjs from "./dateUtils.js";
+import process from "node:process";
+
+// Safety guard: advanceGoalInstances() scans every user, marks their
+// pending instances as 'missed', and calls scheduleGoal against the real
+// Google Calendar API. Running these tests against goaly.db would corrupt
+// real user data. The `test` / `ci` tasks in deno.json set DB_PATH=:memory:
+// — this check refuses to run if someone invokes `deno test` directly
+// without overriding the default.
+if (!process.env.DB_PATH || process.env.DB_PATH === "goaly.db") {
+  throw new Error(
+    "advance.test.js refuses to run against the default DB_PATH. " +
+      "Use `deno task test` (which sets DB_PATH=:memory:) or export DB_PATH yourself.",
+  );
+}
+
+const { db } = await import("../db/index.js");
+const { advanceGoalInstances } = await import("./advance.js");
+const dayjs = (await import("./dateUtils.js")).default;
+
+// Stand-in for scheduleGoal so tests don't hit the real Google Calendar API
+// with fake refresh tokens. The unit under test here is the
+// mark-missed-and-tick-tracking logic, not the scheduler itself.
+const noopScheduler = async () => {};
 
 /**
  * @typedef {Object} TestUser
@@ -108,7 +128,7 @@ function createInstance(goal, startIso, endIso, status) {
 }
 
 Deno.test("advanceGoalInstances - marks past-due instances as missed", async () => {
-  const { user } = await setupUser();
+  const { user } = setupUser();
   const goal = createGoal(user, "Reading Time", 3, "afternoon");
 
   // Create 2 past-due pending instances
@@ -126,7 +146,7 @@ Deno.test("advanceGoalInstances - marks past-due instances as missed", async ()
     "Should have 2 pending instances before advancement",
   );
 
-  const result = await advanceGoalInstances();
+  const result = await advanceGoalInstances(noopScheduler);
 
   instances = db.prepare("SELECT * FROM goal_instances WHERE goal_id = ?").all(
     goal.id,
@@ -143,7 +163,7 @@ Deno.test("advanceGoalInstances - marks past-due instances as missed", async ()
 });
 
 Deno.test("advanceGoalInstances - skips future pending instances", async () => {
-  const { user } = await setupUser();
+  const { user } = setupUser();
   const goal = createGoal(user, "Future Goal", 3, "afternoon");
 
   const pastTime = dayjs.utc().subtract(1, "day").toISOString();
@@ -156,7 +176,7 @@ Deno.test("advanceGoalInstances - skips future pending instances", async () => {
     .toISOString();
   createInstance(goal, futureTime, futureEndTime);
 
-  const result = await advanceGoalInstances();
+  const result = await advanceGoalInstances(noopScheduler);
 
   const instances = db.prepare("SELECT * FROM goal_instances WHERE goal_id = ?")
     .all(goal.id);
@@ -181,16 +201,16 @@ Deno.test("advanceGoalInstances - skips future pending instances", async () => {
 });
 
 Deno.test("advanceGoalInstances - handles goals with no pending instances", async () => {
-  const { user } = await setupUser();
+  const { user } = setupUser();
   createGoal(user, "No Pending Goal", 3, "afternoon");
 
-  const result = await advanceGoalInstances();
+  const result = await advanceGoalInstances(noopScheduler);
   assert.equal(result.advanced, 0, "Should not have advanced any goals");
   assert.equal(result.missed, 0, "Should not have missed any instances");
 });
 
 Deno.test("advanceGoalInstances - marks all past-due instances as missed", async () => {
-  const { user } = await setupUser();
+  const { user } = setupUser();
   const goal = createGoal(user, "Many Past Goal", 5, "afternoon");
 
   const pastTime = dayjs.utc().subtract(2, "day").toISOString();
@@ -200,7 +220,7 @@ Deno.test("advanceGoalInstances - marks all past-due instances as missed", async
     createInstance(goal, pastTime, pastEndTime);
   }
 
-  const result = await advanceGoalInstances();
+  const result = await advanceGoalInstances(noopScheduler);
 
   const instances = db.prepare("SELECT * FROM goal_instances WHERE goal_id = ?")
     .all(goal.id);
@@ -212,7 +232,7 @@ Deno.test("advanceGoalInstances - marks all past-due instances as missed", async
 });
 
 Deno.test("advanceGoalInstances - multiple goals for same user", async () => {
-  const { user } = await setupUser();
+  const { user } = setupUser();
   const goal1 = createGoal(user, "Goal 1", 2, "morning");
   const goal2 = createGoal(user, "Goal 2", 2, "afternoon");
 
@@ -222,7 +242,7 @@ Deno.test("advanceGoalInstances - multiple goals for same user", async () => {
   createInstance(goal1, pastTime, pastEndTime);
   createInstance(goal2, pastTime, pastEndTime);
 
-  const result = await advanceGoalInstances();
+  const result = await advanceGoalInstances(noopScheduler);
 
   const instances1 = db.prepare(
     "SELECT * FROM goal_instances WHERE goal_id = ?",
@@ -243,7 +263,7 @@ Deno.test("advanceGoalInstances - multiple goals for same user", async () => {
 });
 
 Deno.test("advanceGoalInstances - does not mark non-pending instances as missed", async () => {
-  const { user } = await setupUser();
+  const { user } = setupUser();
   const goal = createGoal(user, "Non-Pending Goal", 3, "afternoon");
 
   const pastTime = dayjs.utc().subtract(1, "day").toISOString();
@@ -254,7 +274,7 @@ Deno.test("advanceGoalInstances - does not mark non-pending instances as missed"
   createInstance(goal, pastTime, pastEndTime, "completed");
   createInstance(goal, pastTime, pastEndTime, "skipped");
 
-  await advanceGoalInstances();
+  await advanceGoalInstances(noopScheduler);
 
   const instances = db.prepare("SELECT * FROM goal_instances WHERE goal_id = ?")
     .all(goal.id);
@@ -272,15 +292,15 @@ Deno.test("advanceGoalInstances - does not mark non-pending instances as missed"
 });
 
 Deno.test("advanceGoalInstances - updates last_advance_at tracking column", async () => {
-  const { user } = await setupUser();
+  const { user } = setupUser();
   const goal = createGoal(user, "Tracking Goal", 3, "afternoon");
 
   const pastTime = dayjs.utc().subtract(1, "day").toISOString();
   const pastEndTime = dayjs.utc().subtract(1, "day").add(30, "minute")
     .toISOString();
   createInstance(goal, pastTime, pastEndTime);
 
-  await advanceGoalInstances();
+  await advanceGoalInstances(noopScheduler);
 
   const updatedGoal = db.prepare(
     "SELECT last_advance_at FROM goals WHERE id = ?",

src/middleware.js

@@ -0,0 +1,9 @@
+import { startAdvanceCron } from "./server/advance.js";
+
+// Boot the daily advancement cron once when the server module is first
+// loaded. This replaces a sidecar process — keeping it in-process avoids
+// needing a shell wrapper in the (shell-less) distroless runtime image.
+startAdvanceCron();
+
+/** @type {import('astro').MiddlewareHandler} */
+export const onRequest = (_context, next) => next();