[T-192] Surface 可結案 and add 強制結案 to the task card

frontend/src/api/adapter.ts

@@ -653,6 +653,26 @@ export interface TaskView {
   updatedTs: number;
   /** Epoch when the task closed (done/terminated/duplicated); null while open. */
   closedTs: number | null;
+  /** Who forced this task closed and why (wire `forced_done_by` /
+   * `forced_done_reason`, T-182 — surfaced on the card by T-192). Non-empty
+   * ONLY on a task closed through `force_task_done`; a task that closed itself
+   * through `mark_task_done` carries "" on both, which is exactly how the card
+   * tells the two apart.
+   *
+   * 🔴 THE LIGHT LIST DOES NOT CARRY THEM. `TaskListItemDTO` declares neither
+   * field (`spec/openapi.json`), so `toTaskListItem` sets neither and they read
+   * `undefined` on every collapsed row that has not been hydrated. That is NOT
+   * the same as "" — "" is the server saying "this close was not forced", while
+   * `undefined` is "this projection does not answer the question", and the card
+   * must not print 強制結案 information from a projection that cannot deny it.
+   * Optional for that reason, and the card renders the row only on a truthy
+   * `forcedDoneBy` (so `undefined` renders nothing, never a blank 強制結案 row).
+   *
+   * The reason may be "" on a task that IS force-closed: the owner's ruling
+   * rc-a92a6252c3bd made it optional server-side. `forcedDoneBy` is the carrier
+   * of "this was forced"; the reason never was. */
+  forcedDoneBy?: string;
+  forcedDoneReason?: string;
   progressDone: number;
   progressTotal: number;
   steps: TaskStepView[];
@@ -2359,6 +2379,38 @@ export interface Api {
    * caller refetches (the SSE delta also fans).
    */
   terminateTask(id: string): Promise<void>;
+  /**
+   * Close a task as done (`POST /api/tasks/{id}/mark-done` / MCP
+   * `mark_task_done`) — the action `ready_for_done` waits for. NO BODY.
+   *
+   * PRECONDITION: the task must be in `ready_for_done`; anything else is a 409
+   * that NAMES the status the task is actually in (ApiError, thrown). The
+   * cockpit only offers the button from that status, so the 409 is the race
+   * (an SSE delta moved the task under the open menu), not the normal path —
+   * and the card surfaces the named status rather than a bare "failed".
+   *
+   * 🔴 AUTHZ IS THE EXECUTOR'S, NOT THE OWNER'S. The server admits the task's
+   * OWN executor here and 403s everyone else, the owner included. This method
+   * exists on the cockpit's port anyway because the port is the wire's shape,
+   * not the caller's permissions — see `forceTaskDone` for the owner's door.
+   */
+  markTaskDone(id: string): Promise<void>;
+  /**
+   * Force a task closed (`POST /api/tasks/{id}/force-done` / MCP
+   * `force_task_done`) — the exit for a task nobody is going to close: an
+   * executor that is gone, or a plan whose remaining steps will never be
+   * reported.
+   *
+   * WHO: the owner and the admin assistant only; every other principal is a
+   * 403, the task's own executor included (an executor that could force its own
+   * task would simply have `markTaskDone` with no precondition).
+   *
+   * `reason` is OPTIONAL (owner ruling rc-a92a6252c3bd): pass "" and the task
+   * still closes, with `forcedDoneReason` reading back "". Whatever IS passed
+   * is trimmed and stored, and `forcedDoneBy` is stamped either way. Any
+   * non-terminal status is accepted; an already-terminal task is a 409 (throws).
+   */
+  forceTaskDone(id: string, reason: string): Promise<void>;
   /**
    * Mark a task duplicated (`POST /api/tasks/{id}/duplicate`), pointing at the
    * ORIGINAL it duplicates — so whoever spots the duplicate closes it instead of

frontend/src/api/generated/schema.ts

@@ -3050,10 +3050,10 @@ export interface paths {
         get?: never;
         put?: never;
         /**
-         * Close this task as done OVER its own precondition — the exit for a task that is never going to be closed by the agent holding it. Two shapes reach it: one sitting in ``ready_for_done`` whose executor is gone, and one still mid-plan whose remaining steps will never be reported. WHO: the OWNER and the ADMIN ASSISTANT only. Every other principal is a 403, the task's own executor INCLUDED — an executor that can force its own task simply has ``mark_task_done`` without a precondition, and the precondition is the whole point. ``reason`` is REQUIRED and refused blank (422): a forced close is the one close nobody can reconstruct afterwards from the steps, because the steps do not agree that the work is finished. The forcing principal and the reason are recorded on the task and come back on every read as ``forced_done_by`` / ``forced_done_reason``, so a done task always says whether it got there by itself. ANY NON-TERMINAL STATUS IS ACCEPTED. Already terminal is a 409 — this forces the precondition, not the terminal wall. Answers with a bounded receipt (``artifact_count``, ``closed_ts``, ``deps``, ``description_sha256``, ``description_size_chars``, ``duplicate_of``, ``executor_id``, ``executor_kind``, ``lock``, ``progress_done``, ``progress_total``, ``status``, ``task_id``, ``title``), not the task — call ``get_task`` when you need the rest. THE BOUND OUTSOURCE WORKER IS DISMISSED HERE, and here is the only place it happens: the row releases AND the session is reclaimed at once, on all four closes alike. It used to wait for a separate close-out report, which is gone — the close-out now happens while the task is still open, so once a close lands there is nothing the worker is still allowed to do on it.
+         * Close this task as done OVER its own precondition — the exit for a task that is never going to be closed by the agent holding it. Two shapes reach it: one sitting in ``ready_for_done`` whose executor is gone, and one still mid-plan whose remaining steps will never be reported. WHO: the OWNER and the ADMIN ASSISTANT only. Every other principal is a 403, the task's own executor INCLUDED — an executor that can force its own task simply has ``mark_task_done`` without a precondition, and the precondition is the whole point. ``reason`` is OPTIONAL — omitted, blank or whitespace-only all close the task (owner ruling rc-a92a6252c3bd). A forced close is the one close nobody can reconstruct afterwards from the steps, because the steps do not agree that the work is finished, so a reason is worth giving: it is asked for, not demanded. The forcing principal and the reason are recorded on the task and come back on every read as ``forced_done_by`` / ``forced_done_reason``, so a done task always says whether it got there by itself. ANY NON-TERMINAL STATUS IS ACCEPTED. Already terminal is a 409 — this forces the precondition, not the terminal wall. Answers with a bounded receipt (``artifact_count``, ``closed_ts``, ``deps``, ``description_sha256``, ``description_size_chars``, ``duplicate_of``, ``executor_id``, ``executor_kind``, ``lock``, ``progress_done``, ``progress_total``, ``status``, ``task_id``, ``title``), not the task — call ``get_task`` when you need the rest. THE BOUND OUTSOURCE WORKER IS DISMISSED HERE, and here is the only place it happens: the row releases AND the session is reclaimed at once, on all four closes alike. It used to wait for a separate close-out report, which is gone — the close-out now happens while the task is still open, so once a close lands there is nothing the worker is still allowed to do on it.
          * @description - Closes any non-terminal task as done, ignoring the `ready_for_done` precondition.
          *     - Callers: the owner and the admin assistant only — the executor is a 403 here.
-         *     - `reason` is required (422 when blank) and is recorded with the forcing principal.
+         *     - `reason` is OPTIONAL (owner ruling rc-a92a6252c3bd); when given it is recorded with the forcing principal.
          *     - Reads carry `forced_done_by` / `forced_done_reason` afterwards.
          *     - Already terminal is a 409.
          *     - Stamps closed_ts and dismisses any bound outsource worker (row released, session reclaimed).
@@ -8983,14 +8983,14 @@ export interface components {
         };
         /**
          * TaskForceDoneDTO
-         * @description Force this task done (MCP ``force_task_done``), over the ``ready_for_done`` precondition ``mark_task_done`` enforces. OWNER AND ADMIN ASSISTANT ONLY — the executor is a 403 here, because an executor that can force its own task just has ``mark_task_done`` with no precondition. ``reason`` is REQUIRED and a blank one is refused (422): every other close leaves the steps agreeing that the work is finished, and this one does not, so the reason is the only record of why the task ended. It is stored with the forcing principal and read back as ``forced_done_by`` / ``forced_done_reason``. Any non-terminal status is accepted; an already-terminal task is a 409.
+         * @description Force this task done (MCP ``force_task_done``), over the ``ready_for_done`` precondition ``mark_task_done`` enforces. OWNER AND ADMIN ASSISTANT ONLY — the executor is a 403 here, because an executor that can force its own task just has ``mark_task_done`` with no precondition. ``reason`` is OPTIONAL (owner ruling rc-a92a6252c3bd) — omitted, blank or whitespace-only all close the task and leave ``forced_done_reason`` ''. Every other close leaves the steps agreeing that the work is finished and this one does not, so a reason is the only record of why the task ended: when one IS given it is trimmed and stored with the forcing principal, read back as ``forced_done_by`` / ``forced_done_reason``. ``forced_done_by`` is stamped either way. Any non-terminal status is accepted; an already-terminal task is a 409.
          */
         TaskForceDoneDTO: {
             /**
              * Reason
-             * @description Why this task is being closed without its steps saying so. Required, and a blank or whitespace-only reason is refused.
+             * @description Why this task is being closed without its steps saying so. OPTIONAL: omit it, or send a blank/whitespace-only string, and the task still closes with ``forced_done_reason`` ''. When given it is trimmed and stored.
              */
-            reason: string;
+            reason?: string;
         };
         /**
          * TaskMarkDuplicatedDTO

frontend/src/api/http.task-close.test.ts

@@ -0,0 +1,130 @@
+// httpApi · the two task-close writes, at the WIRE (T-192).
+//
+// 🔴 WHY THIS FILE EXISTS. `forceTaskDone` decides, in product code with a
+// paragraph of comment defending it, whether the owner's typed reason rides the
+// request at all:
+//
+//     const trimmed = reason.trim();
+//     body: trimmed === "" ? {} : { reason: trimmed }
+//
+// and until this file NOTHING anywhere in the repo called `httpApi.forceTaskDone`
+// or `httpApi.markTaskDone`. An independent review replaced that body with a
+// constant `{}` — every reason the owner types silently dropped on the floor,
+// the force still succeeding, the dialog still closing — and the frontend suite
+// stayed green, `src/api` included. A comment explaining a decision is not a
+// guard on it.
+//
+// The three cells, which are the three the product code actually branches on:
+//   1. a reason given      → body carries `reason`, TRIMMED;
+//   2. blank / whitespace  → body carries NO `reason` KEY (not `reason: ""`);
+//   3. `markTaskDone`      → POST to its own path with NO body at all.
+//
+// ⚠️ CELL 2 IS ABOUT THE KEY, NOT THE VALUE. `{reason: ""}` and `{}` both
+// "close the task without a reason" server-side today, so an assertion on the
+// stored outcome cannot tell them apart — but they say different things on the
+// wire ("my reason is the empty string" vs "I gave none"), and it is the second
+// that owner ruling rc-a92a6252c3bd made expressible. So the assertion is
+// `Object.keys(body)`, and `toEqual({})` alone would NOT do it: `toEqual`
+// treats an explicit `undefined` as absent, which is precisely the shape a
+// half-broken omission would produce.
+//
+// Shape copied from http.mutations.test.ts (openapi-fetch drives a real
+// `Request` through global fetch, so the stub returns real `Response` objects,
+// fresh per call — a body is one-shot).
+
+import { describe, it, expect, vi, beforeEach, afterEach } from "vitest";
+import { httpApi } from "./http";
+
+/** TaskWriteReceiptDTO is what both routes answer; neither caller reads it
+ * (useTasks awaits and refetches), so the minimum that parses is enough. */
+const WIRE_RECEIPT = { task_id: "task-1", status: "done", title: "t" };
+
+function jsonResponse(body: unknown, status = 200): Response {
+  return new Response(JSON.stringify(body), {
+    status,
+    headers: { "Content-Type": "application/json" },
+  });
+}
+
+const fetchMock = vi.fn(async () => jsonResponse(WIRE_RECEIPT));
+
+async function lastRequest(): Promise<{
+  url: string;
+  method: string;
+  body: string | undefined;
+}> {
+  const calls = fetchMock.mock.calls as unknown as [Request][];
+  expect(calls.length).toBeGreaterThan(0);
+  const req = calls[calls.length - 1][0];
+  const u = new URL(req.url);
+  const text = await req.clone().text();
+  return { url: u.pathname + u.search, method: req.method, body: text || undefined };
+}
+
+beforeEach(() => {
+  fetchMock.mockReset();
+  fetchMock.mockImplementation(async () => jsonResponse(WIRE_RECEIPT));
+  vi.stubGlobal("fetch", fetchMock);
+});
+
+afterEach(() => {
+  vi.unstubAllGlobals();
+});
+
+describe("httpApi.forceTaskDone · the reason reaches the wire, or is honestly absent", () => {
+  it("a reason GIVEN rides the body under `reason`", async () => {
+    await httpApi.forceTaskDone("task-1", "負責人已離場,活早就交付了");
+    const { url, method, body } = await lastRequest();
+    expect(url).toBe("/api/tasks/task-1/force-done");
+    expect(method).toBe("POST");
+    expect(JSON.parse(String(body))).toEqual({
+      reason: "負責人已離場,活早就交付了",
+    });
+  });
+
+  it("a reason is TRIMMED before it is sent, not sent with the textarea's padding", async () => {
+    await httpApi.forceTaskDone("task-1", "   worker 已經不在了  \n");
+    const { body } = await lastRequest();
+    expect(JSON.parse(String(body))).toEqual({ reason: "worker 已經不在了" });
+  });
+
+  it("an EMPTY reason omits the key — no `reason` on the body at all", async () => {
+    await httpApi.forceTaskDone("task-1", "");
+    const { url, method, body } = await lastRequest();
+    expect(url).toBe("/api/tasks/task-1/force-done");
+    expect(method).toBe("POST");
+    // The key itself must be gone. `toEqual({})` would also accept
+    // `{reason: undefined}`; `Object.keys` will not.
+    const parsed = JSON.parse(String(body));
+    expect(Object.keys(parsed)).toEqual([]);
+    expect("reason" in parsed).toBe(false);
+  });
+
+  it("a WHITESPACE-ONLY reason is the empty case, not a reason made of spaces", async () => {
+    await httpApi.forceTaskDone("task-1", "   \n\t  ");
+    const { body } = await lastRequest();
+    const parsed = JSON.parse(String(body));
+    expect(Object.keys(parsed)).toEqual([]);
+  });
+
+  it("the task id is the one that lands in the PATH, not in the body", async () => {
+    await httpApi.forceTaskDone("task-zzz", "理由");
+    const { url, body } = await lastRequest();
+    expect(url).toBe("/api/tasks/task-zzz/force-done");
+    expect(Object.keys(JSON.parse(String(body)))).toEqual(["reason"]);
+  });
+});
+
+describe("httpApi.markTaskDone · path-only, no body", () => {
+  // Kept under test even though the cockpit no longer offers an ordinary-close
+  // button (T-192 scope: that route 403s every principal this SPA can be). The
+  // port binding is still the frontend's one statement of what the route looks
+  // like, and it was equally unguarded.
+  it("POSTs to the mark-done path and sends no body", async () => {
+    await httpApi.markTaskDone("task-1");
+    const { url, method, body } = await lastRequest();
+    expect(url).toBe("/api/tasks/task-1/mark-done");
+    expect(method).toBe("POST");
+    expect(body).toBeUndefined();
+  });
+});

frontend/src/api/http.ts

@@ -1776,6 +1776,33 @@ export const httpApi: Api = {
     });
   },
 
+  async markTaskDone(id: string): Promise<void> {
+    // POST /api/tasks/{task_id}/mark-done -> TaskWriteReceiptDTO. NO BODY —
+    // the route declares none. The precondition (ready_for_done) and the authz
+    // (the task's OWN executor; the owner is a 403 here) are both the server's;
+    // a refusal arrives as an ApiError through the client middleware and the
+    // card renders the status the 409 names.
+    await client.POST("/api/tasks/{task_id}/mark-done", {
+      params: { path: { task_id: id } },
+    });
+  },
+
+  async forceTaskDone(id: string, reason: string): Promise<void> {
+    // POST /api/tasks/{task_id}/force-done {reason?} -> TaskWriteReceiptDTO.
+    //
+    // 🔴 THE KEY IS OMITTED WHEN THE REASON IS BLANK, rather than sent as "".
+    // Both are accepted today (the server trims and stores ""), and the wire
+    // now declares `reason` optional — but sending `reason: ""` states that the
+    // caller HAS a reason and it is the empty string, which is not what an
+    // empty textarea means. Omitting says "no reason given", which is the thing
+    // owner ruling rc-a92a6252c3bd made expressible.
+    const trimmed = reason.trim();
+    await client.POST("/api/tasks/{task_id}/force-done", {
+      params: { path: { task_id: id } },
+      body: trimmed === "" ? {} : { reason: trimmed },
+    });
+  },
+
   async markTaskDuplicate(id: string, duplicateOf: string): Promise<void> {
     // POST /api/tasks/{task_id}/mark-duplicated {duplicate_of} ->
     // TaskWriteReceiptDTO (T-182 renamed the route; the tool is now

frontend/src/api/index.ts

@@ -5,6 +5,7 @@
 // changes — that is the entire point of the seam.
 
 import type { Api } from "./adapter";
+import { hasToken } from "./auth";
 import { mockApi } from "./mock";
 import { httpApi } from "./http";
 
@@ -16,6 +17,43 @@ export const USE_MOCK = import.meta.env.VITE_USE_MOCK !== "false";
 
 export const api: Api = USE_MOCK ? mockApi : httpApi;
 
+/**
+ * May the principal this SPA is authenticated as force a task closed
+ * (`POST /api/tasks/{id}/force-done`)? T-192.
+ *
+ * 🔴 THIS DEFENDS NOTHING, and reading it as a permission check is the one way
+ * to get it wrong. The REAL gate is the server's route floor —
+ * `Gated(principalAdminAgent, …)` in `server/ocserverd/routes.go` — which admits
+ * the owner and the admin assistant and 403s every other principal, the task's
+ * OWN executor included. A caller who forces a `true` out of this function still
+ * gets that 403. What this decides is only whether the cockpit OFFERS the
+ * control, under one rule: never render a button that could only ever fail.
+ *
+ * The body is `USE_MOCK || hasToken()` — the SAME predicate `AuthGate` already
+ * uses to decide that a session is the owner's — and its honesty rests on a fact
+ * about this SPA rather than a guess about roles: THE COCKPIT HAS EXACTLY ONE
+ * PRINCIPAL. `/api/login` mints an OWNER token (`TokenDTO.owner_id`) and every
+ * gated call rides it; there is no member login, no impersonation, and
+ * `/api/auth/status` discloses nothing about who is asking (`password_set` /
+ * `mfa_required`, nothing else). So "this session holds an owner token" IS "this
+ * session is inside the set the route floor admits". The mock half is not a
+ * loophole: mock mode never renders the wall at all (`AuthGate`), so a token
+ * test there would say "not the owner" about the only principal that exists.
+ *
+ * The negative arm is therefore real, not decorative: a real-mode page with no
+ * owner token — a cleared or expired session, the unauthenticated share-link
+ * surfaces — is a caller the server refuses, and it is not shown the control.
+ *
+ * ⚠️ IF A MEMBER-SCOPED COCKPIT EVER EXISTS, THIS IS WRONG RATHER THAN MERELY
+ * INCOMPLETE: a plain member's token would make `hasToken()` true while the
+ * route floor still refuses them. Whoever adds that login must replace this body
+ * with a real principal read. Nothing in the type system would notice, which is
+ * why the warning is here and not in a ticket.
+ */
+export function viewerMayForceTaskDone(): boolean {
+  return USE_MOCK || hasToken();
+}
+
 export type {
   Api,
   SseConnectionState,