chore(webapp): cut the watch service and route comments to their invariants

Author: kathiekiwi

apps/webapp/app/routes/api.v1.dashboard-agent.alerts.$channelId.ts

@@ -7,10 +7,7 @@ import { authenticateUatOrApiRequest } from "~/services/uatRoutePreamble.server"
 
 /**
  * `DELETE /api/v1/dashboard-agent/alerts/:channelId` — stop alerting this channel
- * when a watch fires. Same semantics as the email's unsubscribe link.
- *
- * The channel is looked up scoped to the chat's project, so a channel id from
- * another project 404s rather than being touched.
+ * when a watch fires. The channel is looked up scoped to the chat's project.
  */
 
 const ParamsSchema = z.object({ channelId: z.string().min(1) });
@@ -34,7 +31,7 @@ export async function action({ request, params }: ActionFunctionArgs) {
     return json({ error: "Not allowed", code: "forbidden_client" }, { status: 403 });
   }
   const userId = authentication.userActor.userId;
-  // The turn's environment scope — the authority for the chat's project below.
+  // The turn's environment scope is the authority for the chat's project below.
   const environmentId = authentication.userActor.environmentId;
   if (!environmentId) {
     return json(

apps/webapp/app/routes/api.v1.dashboard-agent.alerts.ts

@@ -18,16 +18,8 @@ import { authenticateUatOrApiRequest } from "~/services/uatRoutePreamble.server"
 import { CreateAlertChannelService } from "~/v3/services/alerts/createAlertChannel.server";
 
 /**
- * `GET /api/v1/dashboard-agent/alerts` — what alerts this chat's project sends
- * when a watch fires.
- * `POST /api/v1/dashboard-agent/alerts` — subscribe the user's email to them.
- *
- * Accepts only the dashboard agent's delegated user-actor token, like the watches
- * endpoint: POST creates something that later mails a person. The environment comes
- * from the token, not the body, via `resolveAgentAlertContext`.
- *
- * The feature-flag gate is enforced here and again at delivery, and its denial
- * carries a machine-readable `reason` so the agent can say why.
+ * `GET` lists this chat's project's watch alerts; `POST` subscribes the user's email. Only
+ * the agent's delegated user-actor token is accepted, and the environment comes from it.
  */
 
 const ListQuerySchema = z.object({
@@ -39,16 +31,13 @@ const ListQuerySchema = z.object({
 const CreateBodySchema = z.object({
   chatId: z.string().min(1),
   channel: z.literal("email"),
-  /** Omit it: it defaults to, and may only be, the authenticated user's account email. */
+  /** May only be the authenticated user's own account email. */
   email: z.string().email().optional(),
   environmentId: z.string().min(1).optional(),
   projectRef: z.string().min(1).optional(),
 });
 
-/**
- * Shared preamble: a dashboard-agent token plus the environment scope it was minted
- * with. That scope is the authority here, so a token without one is unusable.
- */
+/** A token without an environment scope is unusable here. */
 async function authenticate(
   request: Request
 ): Promise<{ userId: string; environmentId: string } | { error: Response }> {
@@ -68,7 +57,7 @@ async function authenticate(
   return { userId: actor.userId, environmentId: actor.environmentId };
 }
 
-/** Context failures: a mismatched claim is the caller's error, the rest are 404s. */
+/** A mismatched claim is the caller's error, the rest are 404s. */
 function contextStatus(code: AgentAlertContextError) {
   return code === "environment_mismatch" ? 400 : 404;
 }
@@ -165,9 +154,8 @@ export async function action({ request }: ActionFunctionArgs) {
     return json({ error: "Alerts are not available here", code: gate.reason }, { status: 403 });
   }
 
-  // The agent may only subscribe the signed-in user's own account email, so a model
-  // can't be talked into mailing a watch to someone else. Read off the primary, not
-  // the replica: this is the identity the subscription is pinned to.
+  // Only the signed-in user's own account email may be subscribed. Read off the
+  // primary: this is the identity the subscription is pinned to.
   const user = await prisma.user.findFirst({ where: { id: userId }, select: { email: true } });
   if (!user) {
     return json({ error: "User not found", code: "invalid_request" }, { status: 404 });
@@ -190,8 +178,7 @@ export async function action({ request }: ActionFunctionArgs) {
       name: `Watch alerts for ${email}`,
       alertTypes: [DASHBOARD_AGENT_WATCH_ALERT_TYPE],
       environmentTypes: [environment.type],
-      // Stable per (email, project): asking twice re-enables the existing
-      // subscription instead of stacking duplicate channels.
+      // Stable per (email, project), so asking twice re-enables one channel.
       deduplicationKey: `dashboard-agent-watch:${email}`,
       channel: { type: "EMAIL", email },
     });

apps/webapp/app/routes/api.v1.dashboard-agent.watches.$watchId.check.ts

@@ -16,30 +16,13 @@ import {
 } from "~/services/dashboardAgentWatchToken.server";
 
 /**
- * `POST /api/v1/dashboard-agent/watches/:watchId/check` — private per-watch check.
- * The watcher task calls it once per tick with the watch's token.
- *
- * Security model: the token only names a watch; the row is the authority on
- * lifecycle and on the immutable project/environment/user snapshot; the user is
- * re-authorized against that snapshot on every call, and a revoked user gets the
- * watch cancelled here before any environment data is read.
- *
- * The route never transitions the watch to fired/expired and never advances the
- * tick counter. The watcher task owns both, so exactly one component decides when
- * the user is told and nothing here can fork the tick chain.
- *
- * It also hands a watch over to its group's batch chain: it makes sure a chain is
- * running for the watch's (environment, cadence) group and answers
- * `batched: true` when one is, at which point the caller stops rescheduling its
- * own per-watch chain.
+ * Private per-watch check. The token only names a watch; the row is the authority on
+ * lifecycle and its snapshot, and this route transitions nothing and advances no tick.
  */
 
 const ParamsSchema = z.object({ watchId: z.string().min(1) });
 
-/**
- * Best-effort: the verdict is what this route owes its caller, so a chain that
- * couldn't be armed returns `false` and the next check tries again.
- */
+/** Best-effort: a chain that couldn't be armed returns `false` and is retried next check. */
 async function ensureBatchChain(watch: {
   id: string;
   environmentId: string;
@@ -91,8 +74,7 @@ export async function action({ request, params }: ActionFunctionArgs) {
     );
   }
 
-  // A valid token for a different watch is 403, not 401: the caller is
-  // authenticated, just not for this resource.
+  // A valid token for a different watch is 403, not 401.
   if (claims.watchId !== watchId) {
     return json({ error: "Not allowed for this watch", code: "watch_mismatch" }, { status: 403 });
   }
@@ -129,8 +111,7 @@ export async function action({ request, params }: ActionFunctionArgs) {
   const now = new Date();
   const expired = watch.expiresAt.getTime() <= now.getTime();
   if (expired) {
-    // Past the deadline only the final evaluation is allowed, and only inside the
-    // grace window the token is valid for.
+    // Past the deadline only the final evaluation is allowed, inside the token's grace.
     const graceEnds = watch.expiresAt.getTime() + WATCH_TOKEN_GRACE_MS;
     if (body.final !== true || now.getTime() > graceEnds) {
       return json(
@@ -140,11 +121,8 @@ export async function action({ request, params }: ActionFunctionArgs) {
     }
   }
 
-  // Everything below reads or writes a tenant's data, so failures are logged with
-  // whose tick failed, then rethrown unchanged.
   try {
-    // Re-authorize the initiating user before any environment data is read, so a
-    // revoked user's tick can't observe anything.
+    // Re-authorize the initiating user before any environment data is read.
     const authorization = await authorizeWatchEnvironment({
       userId: watch.userId,
       organizationId: watch.organizationId,
@@ -153,8 +131,7 @@ export async function action({ request, params }: ActionFunctionArgs) {
     });
 
     if (!authorization.ok) {
-      // The watch must not survive the access it was created with. Cancellation is
-      // never notified, so `deliveryStatus` stays `not_required`.
+      // A watch must not outlive the access it was created with. Never notified.
       await cancelWatch(dashboardAgentDb, { id: watchId, reason: "access_revoked" });
       return json(
         { error: "Access to this environment was revoked", code: "access_revoked" },
@@ -166,8 +143,7 @@ export async function action({ request, params }: ActionFunctionArgs) {
     const outcome = await checkWatch(
       watch.spec,
       watchCheckDeps(authorization.environment, now),
-      // `previous` is the last check's facts, off the row. A tick that couldn't read
-      // anything freezes a streak instead of resetting it.
+      // A tick that couldn't read anything freezes a streak instead of resetting it.
       { now, since, previous: previousCheckFacts(watch.lastResult) },
       (error) =>
         logger.error("Dashboard agent watch check failed", {
@@ -180,8 +156,8 @@ export async function action({ request, params }: ActionFunctionArgs) {
         })
     );
 
-    // Recorded even on the final evaluation: `lastResult` is what the notification
-    // reads. Guarded on `active`, so a concurrent fire/expire wins and this no-ops.
+    // Recorded even on the final evaluation. Guarded on `active`, so a concurrent
+    // fire/expire wins and this no-ops.
     await recordWatchCheck(dashboardAgentDb, {
       id: watchId,
       lastResult: {
@@ -194,8 +170,7 @@ export async function action({ request, params }: ActionFunctionArgs) {
 
     const batched = await ensureBatchChain(watch);
 
-    // `observed` travels with the verdict: the task writes it onto the row in the
-    // same statement as the resolution, so no delivery surface re-reads the source.
+    // `observed` travels with the verdict so no delivery surface re-reads the source.
     return json({
       result: outcome.result,
       facts: outcome.facts,

apps/webapp/app/routes/api.v1.dashboard-agent.watches.$watchId.fired.ts

@@ -11,17 +11,8 @@ import {
 import { logger } from "~/services/logger.server";
 
 /**
- * `POST /api/v1/dashboard-agent/watches/:watchId/fired` — the watcher task tells
- * us a watch fired, so the project's alert channels can be notified.
- *
- * Same security model as the check endpoint next door: the token only names a watch,
- * the row is the authority on whether it fired, and the watch's initiating user is
- * re-authorized against the row's immutable project/environment before anything is
- * sent, so an alert never outlives the access the watch was created with.
- *
- * The caller's body is ignored, so a replay can only re-announce what the row says,
- * and the alert job is keyed on the watch (`watch-alert:{watchId}`) so the fan-out
- * happens at most once.
+ * The watcher task reports a fired watch. The row is the authority on whether it fired, and
+ * the initiating user is re-authorized against its snapshot before any alert is sent.
  */
 
 const ParamsSchema = z.object({ watchId: z.string().min(1) });
@@ -68,8 +59,6 @@ export async function action({ request, params }: ActionFunctionArgs) {
     );
   }
 
-  // The row is the first thing that knows whose request this is, so the logging
-  // boundary starts here. Failures are logged, then rethrown unchanged.
   try {
     const authorization = await authorizeWatchEnvironment({
       userId: watch.userId,
@@ -79,7 +68,7 @@ export async function action({ request, params }: ActionFunctionArgs) {
     });
 
     if (!authorization.ok) {
-      // Not cancelled here (the watch is already terminal) — just silence.
+      // Not cancelled here: the watch is already terminal.
       logger.info("Dashboard agent watch fired, but access was revoked; no alert", { watchId });
       return json(
         { error: "Access to this environment was revoked", code: "access_revoked" },

apps/webapp/app/routes/api.v1.dashboard-agent.watches.$watchId.investigate.ts

@@ -14,18 +14,8 @@ import {
 import { logger } from "~/services/logger.server";
 
 /**
- * `POST /api/v1/dashboard-agent/watches/:watchId/investigate` — the watcher task
- * tells us it has delivered the wake for a watch whose creator pre-approved an
- * investigation, so the agent can be sent off to actually conduct it.
- *
- * Same security model as the fired callback next door: the token only names a watch,
- * the row is the authority on what happened, and the watch's initiating user is
- * re-authorized against the row's immutable project/environment before anything is
- * minted, so an investigation never outlives the access the watch was created with.
- *
- * The caller's body is ignored, so a replay can only re-ask for what the row already
- * says, and the agent dedupes on the action's stable id. The consent, the outcome,
- * the user and the environment all come off the row, so the model cannot steer them.
+ * The watcher task reports a delivered wake for a pre-approved investigation. The caller's
+ * body is ignored: consent, outcome, user and environment all come off the row.
  */
 
 const ParamsSchema = z.object({ watchId: z.string().min(1) });
@@ -64,8 +54,6 @@ export async function action({ request, params }: ActionFunctionArgs) {
     return json({ error: "Watch not found", code: "not_found" }, { status: 404 });
   }
 
-  // The row decides on all three counts: it has resolved, the user consented, and
-  // the outcome is one the consent covers.
   if (!isTerminalWatchStatus(watch.status)) {
     return json(
       { error: `This watch is ${watch.status}`, code: "not_resolved", status: watch.status },
@@ -74,7 +62,7 @@ export async function action({ request, params }: ActionFunctionArgs) {
   }
 
   if (!watchWantsInvestigation(watch)) {
-    // Good news, neutral news, or no consent — the wake was the whole delivery.
+    // No consent, or an outcome consent doesn't cover: the wake was the whole delivery.
     return json({ ok: true, investigating: false });
   }
 
@@ -95,10 +83,8 @@ export async function action({ request, params }: ActionFunctionArgs) {
     );
   }
 
-  // Best-effort, and never an error to the caller: the wake is already delivered and
-  // marked by the time we are called, so a failed kick must not make the watcher
-  // retry. A turn that never starts, or dies mid-investigation, is settled by the
-  // stale-investigation sweep.
+  // Never an error to the caller: the wake is already delivered and marked, so a failed
+  // kick must not make the watcher retry. The stale-investigation sweep settles it.
   try {
     await kickWatchInvestigation({ watch, environment: authorization.environment });
   } catch (error) {

apps/webapp/app/routes/api.v1.dashboard-agent.watches.batch-check.ts

@@ -8,11 +8,8 @@ import {
 } from "~/services/dashboardAgentWatchToken.server";
 
 /**
- * `POST /api/v1/dashboard-agent/watches/batch-check` — private batch check. One call
- * per (environment, cadence) group per cadence, in place of one call per watch.
- *
- * The token names the group and the body names the tick inside it.
- * `runWatchBatchCheck` does the rest, and documents the security model.
+ * Private batch check: one call per (environment, cadence) group per cadence. The token
+ * names the group, the body names the tick. `runWatchBatchCheck` documents the rest.
  */
 
 const BodySchema = z.object({
@@ -55,8 +52,7 @@ export async function action({ request }: ActionFunctionArgs) {
   if (!parsedBody.success) return json({ error: "Invalid request body" }, { status: 400 });
   const body = parsedBody.data;
 
-  // A valid token for a different group is 403, not 401: the caller is
-  // authenticated, just not for this group.
+  // A valid token for a different group is 403, not 401.
   if (
     claims.environmentId !== body.environmentId ||
     claims.cadenceMinutes !== body.cadenceMinutes