fix(dashboard): close review findings in logWorkAction/polishNoteAction — Task 6

- Consolidate requireOwnerPb to ensure getAdminPb unreachable without auth check
- Both actions now never throw; all failures return { ok: false, message }
- Verify cardId belongs to projectId before creating commit
- link_source always "manual" (never "none") to prevent auto-attachment
- Gracefully handle recomputeMetrics failures (entry saved, only KPIs stale)
- Revalidate card page + unassigned tray in addition to dashboard
- Fix hours validation message to clarify 0 is not valid

Co-Authored-By: Claude Haiku 4.5 <noreply@anthropic.com>

src/app/actions/log-work.ts

@@ -15,21 +15,35 @@ const MAX_MESSAGE_LENGTH = 2000;
 const MAX_HOURS = 24;
 const ISO_DATE = /^\d{4}-\d{2}-\d{2}$/;
 
-/** See the security note at the top of src/app/actions/cards.ts. */
+/** See the security note at the top of src/app/actions/cards.ts for why a
+ *  role check like this matters — unlike cards.ts, this file writes through
+ *  the superuser connection (see requireOwnerPb below), so this check is not
+ *  backstopped by PocketBase's own rules the way a cards.ts write is. */
 async function requireOwner() {
   const session = await requireSession();
   if (session.role !== "owner") throw new Error("Not found");
   return session;
 }
 
+/**
+ * The owner's superuser connection — never obtainable without passing the
+ * owner check above first. getAdminPb() bypasses every collection rule and
+ * checks nothing itself ("Every call site MUST have already checked
+ * authorization itself" per its own doc comment); this is the only place in
+ * this file that imports it, so there is no code path here that can reach it
+ * without going through requireOwner() first.
+ */
+async function requireOwnerPb() {
+  await requireOwner();
+  return getAdminPb();
+}
+
 /**
  * Log a block of off-repo work as a commits row.
  *
- * Writes through the superuser connection, not the owner's own token —
- * `commits.createRule` is `null` (superuser-only) because every OTHER row in
- * this collection comes from the ingestion pipeline, which already writes
- * this way. The owner-only check above is what stands in for a collection
- * rule here, exactly as it does for every client-manager write in this app.
+ * Never throws — every failure, including "not signed in" and "wrong role",
+ * comes back as { ok: false, message }, so the calling form only ever has
+ * one shape to handle.
  */
 export async function logWorkAction(input: {
   projectId: string;
@@ -40,62 +54,110 @@ export async function logWorkAction(input: {
   message: string;
   cardId?: string;
 }): Promise<{ ok: boolean; message?: string }> {
-  await requireOwner();
+  let pb;
+  try {
+    pb = await requireOwnerPb();
+  } catch {
+    return { ok: false, message: "Not found" };
+  }
 
   const message = input.message.trim().slice(0, MAX_MESSAGE_LENGTH);
   if (!message) return { ok: false, message: "A message is required" };
 
   if (!Number.isFinite(input.hours) || input.hours <= 0 || input.hours > MAX_HOURS) {
-    return { ok: false, message: `Hours must be between 0 and ${MAX_HOURS}` };
+    return {
+      ok: false,
+      message: `Hours must be greater than 0 and at most ${MAX_HOURS}`,
+    };
   }
 
   if (!ISO_DATE.test(input.date)) {
     return { ok: false, message: "Invalid date" };
   }
 
-  const pb = await getAdminPb();
   const cardId = input.cardId ?? "";
 
-  await pb.collection("commits").create<CommitRecord>({
-    project: input.projectId,
-    // Guaranteed not to collide with a real 40-character git sha, and unique
-    // enough to satisfy the (project, sha) index every other commit relies on.
-    sha: `manual-${randomUUID()}`,
-    short_sha: "manual",
-    message,
-    subject: message,
-    type: "",
-    scope: "",
-    author_name: "",
-    author_email: "",
-    authored_at: `${input.date} 12:00:00.000Z`,
-    is_merge: false,
-    additions: 0,
-    deletions: 0,
-    card: cardId,
-    // Reuses the existing meaning of "manual" on link_source: a card link the
-    // owner chose by hand, not one the linking rules derived.
-    link_source: cardId ? "manual" : "none",
-    url: "",
-    summary: message,
-    summary_version: 0,
-    summary_attempts: 0,
-    source: "manual",
-    manual_minutes: Math.round(input.hours * 60),
-  });
+  if (cardId) {
+    // A card id from a different project would count toward THIS project's
+    // metrics while appearing in another project's card history — the same
+    // mismatch src/lib/data/card.ts guards against for the same reason.
+    const card = await pb
+      .collection("cards")
+      .getOne(cardId)
+      .catch(() => null);
+    if (!card || card.project !== input.projectId) {
+      return { ok: false, message: "That card does not belong to this project" };
+    }
+  }
 
-  // So the dashboard's KPIs reflect it on the very next load, not only after
-  // the owner separately clicks Recompute.
-  await recomputeMetrics(input.projectId);
+  let created: CommitRecord;
+  try {
+    created = await pb.collection("commits").create<CommitRecord>({
+      project: input.projectId,
+      // Guaranteed not to collide with a real 40-character git sha, and
+      // unique enough to satisfy the (project, sha) index every other
+      // commit relies on.
+      sha: `manual-${randomUUID()}`,
+      short_sha: "manual",
+      message,
+      subject: message,
+      type: "",
+      scope: "",
+      author_name: "",
+      author_email: "",
+      authored_at: `${input.date} 12:00:00.000Z`,
+      is_merge: false,
+      additions: 0,
+      deletions: 0,
+      card: cardId,
+      // Always "manual", whether or not a card is linked — this is a human
+      // deliberately deciding the card association (including deciding not
+      // to have one), and must be inert to the scope/reference auto-linking
+      // rules the way a real commit's link is inert once it is "manual".
+      // Leaving an unlinked entry as "none" would let incidental text in
+      // the owner's note (e.g. a stray "#12") silently attach it later.
+      link_source: "manual",
+      url: "",
+      summary: message,
+      summary_version: 0,
+      summary_attempts: 0,
+      source: "manual",
+      manual_minutes: Math.round(input.hours * 60),
+    });
+  } catch {
+    return { ok: false, message: "Could not save that entry" };
+  }
+
+  try {
+    // So the dashboard's KPIs reflect it on the very next load, not only
+    // after the owner separately clicks Recompute.
+    await recomputeMetrics(input.projectId);
+  } catch {
+    // The entry is already saved — only the cached KPIs are stale. Do NOT
+    // report this as a failed log: the synthetic sha is fresh on every call,
+    // so a retry after a false "failed" would create a duplicate entry
+    // rather than being caught by the (project, sha) unique index. The
+    // Recompute button exists for exactly this case.
+  }
 
   revalidatePath(`/p/${input.clientSlug}/${input.projectSlug}/dashboard`);
+  if (cardId) {
+    revalidatePath(`/p/${input.clientSlug}/${input.projectSlug}/c/${cardId}`);
+  } else {
+    revalidatePath(`/p/${input.clientSlug}/${input.projectSlug}/unassigned`);
+  }
+
   return { ok: true };
 }
 
 export async function polishNoteAction(input: {
   text: string;
 }): Promise<{ ok: boolean; text?: string; message?: string }> {
-  await requireOwner();
+  try {
+    await requireOwner();
+  } catch {
+    return { ok: false, message: "Not found" };
+  }
 
   const text = input.text.trim().slice(0, MAX_MESSAGE_LENGTH);
   if (!text) return { ok: false, message: "Nothing to polish yet" };