Read run records, and make a Start leave a mark on the task

AGENTS.md

@@ -31,6 +31,12 @@ src/
                         log reader. Not pure — resolving symlinks is the point.
   launch-log.ts         Reader side of the launch logs: the inverse of
                         launchLogName, fenced-block extraction, run timestamps.
+  run-record.ts         Run records — the parser for what BOTH launchers write,
+                        and the merge that turns (record, log stat, queue match)
+                        into one list row. Extracted for the usual reason, and
+                        because the merge is where the decisions are: which
+                        source wins on times, what a missing record means, and
+                        what gets said when nothing is known.
   launch-policy.ts      Reads and validates the shared launch roster, and builds
                         the spawn argv. Same extraction reason.
   vocabulary.ts         The task-queue vocabulary (statuses, task types, workflow
@@ -82,6 +88,12 @@ importable. esbuild and `tsc` (`allowImportingTsExtensions`) both accept `.ts`.
 - **The headless-runs section reads launch logs and never writes them.** Both routes are `GET`, resolve through `resolveAllowedPath`, and never treat a route id as a path: `:id` is validated as `<agent>-<task8>` and the filename is then *rebuilt* via `launchLogName`, so the caller's string cannot reach the filesystem even before the realpath guard runs. The guard applies to the **list** route as well as the detail route — the list head-reads every file, so without it a symlink planted in the log directory puts the first line of its target into a row. That was verified against a real `~/.secrets/forge.env` symlink, which leaked with the guard removed and was refused with it present.
 - **The launch-log filename parser is the inverse of `launchLogName`, and is pinned to it by a round-trip test.** `launch-policy.ts` owns the name shape and two producers write it (this plugin and `task-dispatcher.py`). A hardcoded regex here would not error when it drifted — the section would simply list nothing. Unparseable names are **skipped**, never rendered with a guessed or empty agent.
 - **A run's status comes from the queue, never from the log.** A log proves a session ran; it does not prove its task closed. `steward-f42d3aeb` completed on 2026-08-23 against a task that sat at `approved` for four days. Render that disagreement — do not infer status from the log's prose.
+- **A reaped run's `exit_code` is never rendered as success.** `outcomeLabel` maps a null code on an ended run to `ended, exit code unknown`, and the panel colours it as neither pass nor fail. Null is not missing data: a dispatcher tick spawns a detached child and exits, so the child is reparented and its status is reaped by init — there is no `waitpid()` and no surviving `/proc` entry. This plugin is long-lived and *can* observe its own children exit, so `child.on('exit')` records a real code for its own launches; that asymmetry is the design, not an inconsistency to iron out.
+- **The run-record filename is the log filename with a `.json` suffix, and both are produced in `launch-policy.ts`.** `launchLogName` and `runRecordFileName` sit next to each other because the union in `listHeadlessRuns` keys on their shared stem — if the two ever disagree, one run renders as two rows, with the metadata on one and the output on the other. A round-trip test pins them. The `.log` name itself must not move: `task-dispatcher` writes it and the launch-log retention job matches on it.
+- **The list is the union of logs and records, and a stem with only one of them still renders.** A log with no record is one of the 29 runs that predate them. A record with no readable log is the security-audit launcher, whose output goes to `~/.pm2/logs` — a prefix that stays outside `PREVIEW_ALLOWED_PREFIXES` deliberately, because it covers every PM2 service log on this host. Keying the list on `.log` files alone would omit the commonest kind of headless session here; adding the prefix to make it readable would turn this endpoint into a reader of every service log. The row renders and the detail view says where the log is.
+- **`loadRunRecord()` in `run-record.ts` is the only reader of a run record.** It resolves through `resolveAllowedPath` and returns the approved path alongside the record, so a caller writing back uses the path the guard checked. Do not add a second inline read in `server.ts` — that is exactly what happened before, and because that reader's content was not surfaced anywhere, nothing was red and nothing was tested. The extraction exists so the guard has tests at all; `server.ts` calls `listen()` at import time and cannot have them.
+- **Values off the wire are validated, not type-asserted.** `JSON.parse(body) as {…}` is a compile-time claim and no runtime check. `mode` on the Start route escaped notice for as long as it only reached safe `===` comparisons; it became a real gap the moment it acquired a consumer that persists it into task history. When adding a consumer of a request field, check what already validates it — the answer may be "the shape of the code that used to read it."
+- **A Start records itself in the task's history without changing its status.** The control-API call re-asserts the status the task is already in, which the handler accepts with `allow_override` and a note. Advancing `approved` → `in-progress` here looks tidier and breaks every plugin-started session: the agent's own first action is `update_task(in-progress)`, permitted only *from* `approved`. Recording must also never fail the Start — the session is already running by then.
 - **`birthtime` is trusted only when it precedes `mtime`.** The historical logs were copy-migrated into the launch-log directory, and a copy resets birthtime while `cp -p` preserves mtime, so for every migrated run birthtime is *later* than mtime. Trusting it reports each as starting "just now" and running for a negative duration. An untrustworthy birthtime yields a `null` duration, rendered as unknown rather than as zero.
 - **Unterminated fenced blocks are dropped from `commands`.** Those strings are handed to the operator behind a copy button, i.e. built to be pasted into a shell. A fence with no closing delimiter has no known end, and half of a destructive command is worse than none. The full text is still rendered in the log pane.
 - **The WebSocket upgrade gates on the peer address first, and on `Origin` only if one is present.** The server binds `127.0.0.1` on an ephemeral port, so a non-loopback peer is refused outright; a loopback peer with a *present but wrong* `Origin` is still refused. A loopback peer with **no** `Origin` is accepted, because that is CloudCLI's own plugin WS proxy — it uses the `ws` client library, which sends no `Origin` unless one is passed, and its browser leg is already authenticated by CloudCLI's `verifyClient` before the proxy is invoked.

CHANGELOG.md

@@ -2,6 +2,100 @@
 
 All notable changes to this project will be documented in this file.
 
+## [0.9.0] - 2026-08-29
+
+Reads the run records both launchers now write, and makes a Start leave a mark on the task
+it started.
+
+Tracker: vikunja#559. Build plan: agent-workflow-interop-2026-08, Phase 3.
+
+### Added
+
+- **A run record beside every launch this plugin starts.**
+  `~/.claude/comms/artifacts/task-launches/<agent>-<task8>.json`, the same shape
+  `task-dispatcher` v1.3.0 writes. A **sibling** of the log, never a replacement: the
+  `.log` name is what this plugin's own reader parses and what the launch-log retention
+  job matches on.
+- **A real exit code for runs this plugin starts.** It is a long-lived process, so the
+  child handle outlives the spawn and `'exit'` still fires after `unref()`. A
+  dispatcher-launched run can only ever be reaped as `pid-gone` with a null code; this one
+  records what the process actually returned, and a signalled child records the signal
+  rather than a coerced number.
+- **`FORGE_RUN_ID` / `FORGE_TASK_ID` in the launched session's environment**, so a trace
+  can be joined back to the task that paid for it.
+- **An outcome column**, with three states kept distinct: `no run record`, `running`,
+  and the run's actual outcome — `exit 0`, `exit 137`, `ended, exit code unknown`, or
+  `slot released — still running`. `ended, exit code unknown` is the honest rendering of a
+  dispatcher launch and is deliberately not collapsed into success.
+- **The list is now the union of logs and records.** A record with no co-located log
+  renders too — that is the security-audit launcher, whose output goes to `~/.pm2/logs`.
+  That prefix stays outside the preview allowlist because it covers every PM2 service log
+  on this host, so the detail view says where the log is rather than the row being dropped.
+  Dropping it would have omitted the commonest kind of headless session here.
+- **Open runs sort above finished ones.** A descending compare on `ended` put them at the
+  bottom, under three months of finished runs.
+
+### Fixed
+
+- **A Start made no queue mutation at all.** A plugin-started task stayed at `approved`
+  until its agent got as far as claiming it, and a session that died before that left
+  nothing behind anywhere — which is why one completed steward run was invisible for four
+  days (vikunja#534).
+
+  A Start now appends a history entry through the control API. **The status is
+  deliberately unchanged** — the call re-asserts the status the task is already in.
+  Advancing `approved` → `in-progress` is the obvious-looking alternative and it breaks
+  every plugin-started session: the agent's own first action is
+  `update_task(in-progress)`, which task-queue-mcp permits only *from* `approved`.
+- **Duration for a still-open run is unknown, not a growing number.** It would otherwise
+  be derived from the log's mtime and tick upward on every poll for a session that in fact
+  died an hour ago and has not been reaped.
+
+### Changed
+
+- **`src/run-record.ts` added** — the parser for what both launchers write, plus the merge
+  that turns (record, log stat, queue match) into one list row. Extracted from `server.ts`
+  for the reason every other module here was: `server.ts` listens at import time, so
+  nothing inside it can be unit-tested, and the merge is where the decisions are.
+- `runRecordFileName` sits beside `launchLogName` in `launch-policy.ts`, pinned to it by a
+  round-trip test. If the two stems disagree, one run renders as two rows.
+
+### Security
+
+Both from the audit (`agent-workflow-interop-2026-08-phase34`, 0 Critical/High/Medium,
+2 Info). Both fixed rather than accepted.
+
+- **`mode` on `POST /tasks/:id/start` is validated, not type-asserted.** `JSON.parse(body)
+  as {mode: StartMode}` is a compile-time claim and no runtime check. Harmless while the
+  value only reached `=== 'review'` comparisons that fell through safely — but this
+  release gave it a second consumer that writes it into a task's **persisted history
+  note**, and a value that lands in a durable record deserves a validator. `toStartMode()`
+  now sits beside the type in `launch-policy.ts`, with the closed set exported so the two
+  cannot drift.
+
+  An absent mode still defaults to `review`; a present but unrecognised one is a 400.
+  Defaulting there would silently downgrade an operator who asked for `auto`, turning a
+  typo into a session that quietly does nothing. An unparseable body is refused for the
+  same reason.
+
+- **`loadRunRecord()` is now the only way this plugin reads a run record**, extracted into
+  `run-record.ts` so the path guard has tests. `closeRunRecord`'s guard was added during
+  the pre-audit and had no regression test of its own — `server.ts` calls `listen()` at
+  import time, so nothing in it can be unit-tested, which is the same property that let
+  the guard be omitted to begin with. The audit's point was that a fix with no test is one
+  refactor from being undone.
+
+  It returns the **guard-approved** path alongside the record, so a caller writing the
+  record back uses the path realpath approved rather than re-deriving one that was never
+  checked. Seven tests, including a real escaping symlink and a non-escaping one.
+
+### Unchanged, deliberately
+
+- **Status still comes from the queue and only from the queue.** A record saying a run
+  exited 0 does not promote its task's status. A finished run whose task is still
+  `approved` is the disagreement this panel exists to show.
+- **The `<agent>-<task8>.log` filename.** Two other consumers key on it.
+
 ## [0.8.0] - 2026-08-29
 
 Closes the plugin's task-queue vocabulary drift, then gates it so it cannot silently

README.md

@@ -87,17 +87,56 @@ of final text, and exits. Every such launch already wrote its full stdout to
 26 of these had accumulated with nothing able to show them, and one completed steward run
 stayed invisible for four days.
 
-Each row shows agent, short task id, status, started, duration, and the first line of
-output. Click a row to open the full log.
+Each row shows agent, short task id, status, started, duration, outcome, and the first line
+of output. Click a row to open the full log.
 
 **Status comes from the task queue, not from the log.** A log proves a session ran; it does
 not prove the task closed. The two disagreeing — a finished run whose task is still
-`approved` — is the feature working, not a bug.
-
-**Duration can be unknown**, rendered as an em dash. It's derived from the log file's
-timestamps, and birthtime is only trusted when it precedes mtime. Every log migrated into
-the shared directory on 2026-08-27 was copied rather than moved, and a copy resets birthtime
-while preserving mtime — so those runs show an unknown duration rather than a wrong one.
+`approved` — is the feature working, not a bug. This holds for the run record too: a record
+saying the run exited 0 never promotes a task's status.
+
+### Run records
+
+Both launchers now write `<agent>-<task8>.json` beside the log. It is a **sibling**, never a
+replacement — the `.log` name is what this plugin's own reader parses and what the
+launch-log retention job matches on.
+
+The list is the **union** of the two artefacts, keyed on the shared `<agent>-<task8>` stem:
+
+- A **log with no record** is one of the 29 runs that predate them. Times still come from
+  the file's mtime, and the outcome column reads `no run record`.
+- A **record with no readable log** is the security-audit launcher, which writes its output
+  to `~/.pm2/logs/security-audit-<build>.log`. That prefix stays outside the preview
+  allowlist deliberately — it covers every PM2 service log on this host, and adding it would
+  make this endpoint a reader of all of them. The row renders anyway and the detail view
+  says where the log is, because dropping it would omit the commonest kind of headless
+  session here.
+
+**The outcome column has three honest states and does not collapse them.**
+
+| Rendered | Means |
+|---|---|
+| `no run record` | Predates run records; nothing is known about how it ended |
+| `running` | A record with no `ended` |
+| `exit 0`, `exit 137` | A real observed exit code — only this plugin's own launches get one |
+| `ended, exit code unknown` | The run ended and the code is unrecoverable |
+| `slot released — still running` | Past the dispatcher's max runtime; its concurrency slot was freed and the process was left alone |
+
+`ended, exit code unknown` is not a gap. A dispatcher tick spawns a detached child and
+exits, so the child is reparented and its status is reaped by init — there is no `waitpid()`
+and no surviving `/proc` entry. This plugin is a long-lived process and *can* observe its own
+children exit, so runs it starts carry a real code. Rendering the unknown case as success
+would be a counter reporting success for something nobody observed succeed.
+
+**Duration can be unknown**, rendered as an em dash. With a record it is `ended - started`,
+and it is unknown while the run is still open — deliberately not "now minus started", which
+would tick upward forever for a session that died an hour ago and has not been reaped.
+Without a record it comes from the log file's timestamps, where birthtime is only trusted
+when it precedes mtime: every log migrated into the shared directory on 2026-08-27 was
+copied rather than moved, and a copy resets birthtime while preserving mtime.
+
+Open runs sort above finished ones. A plain descending compare on `ended` puts them at the
+bottom, under three months of finished runs.
 
 Below the log text, a **Commands** block lists every fenced code block scraped from the
 output, each with a copy button. The extraction is deliberately dumb — no inference about
@@ -161,7 +200,7 @@ The backend exposes a small HTTP API consumed by the UI via `api.rpc()`.
 | `GET` | `/health` | Liveness check; returns `{status, uptime, version}` |
 | `GET` | `/tasks` | List tasks; query params `agent`, `status`, `type` |
 | `GET` | `/tasks/:id` | Task detail plus context-ref previews |
-| `POST` | `/tasks/:id/start` | Launch a session; body `{mode: "review"\|"auto"}` (a local spawn, not a queue mutation) |
+| `POST` | `/tasks/:id/start` | Launch a session; body `{mode: "review"\|"auto"}`. Spawns locally, writes a run record, and records the launch in the task's history |
 | `POST` | `/tasks/:id/approve` | Approve — proxied |
 | `POST` | `/tasks/:id/cancel` | Cancel (terminal); body `{note?}` — proxied |
 | `POST` | `/tasks/:id/status` | Operator status change; body `{status, note?, allow_override?}` — proxied |
@@ -188,6 +227,11 @@ The upgrade handler gates on the **peer address** first: the server binds `127.0
 | `review` | `plan` | Read the task, present a summary, wait for approval |
 | `auto` | `default` | Read the task, claim it (`in-progress`), execute |
 
+`mode` is validated against that set at the parse site. An **omitted** mode defaults to
+`review` — the safe leg. A **present but unrecognised** mode is a 400, not a silent
+default: defaulting would downgrade an operator who asked for `auto`, turning a typo into
+a session that quietly does nothing.
+
 ### The launch policy file
 
 A task's `target_agent` is resolved through a **data file**, not a map in the source:
@@ -236,10 +280,29 @@ rather than launching.
 > `--permission-mode plan` is not reachable. The UI says so on launch rather than implying a
 > tool gate.
 
-### Launch logs
+### Launch logs and run records
 
 Each launch appends to `~/.claude/comms/artifacts/task-launches/<agent>-<task8>.log`, the
-same shape and directory the reference dispatcher writes, so both are listable together.
+same shape and directory the reference dispatcher writes, so both are listable together, and
+writes `<agent>-<task8>.json` beside it. The session also receives `FORGE_RUN_ID` and
+`FORGE_TASK_ID` in its environment, which is what makes a trace joinable back to its task.
+
+### A Start leaves a mark on the task
+
+Before v0.9.0 a Start made **no queue mutation at all**, so a plugin-started task stayed at
+`approved` until its agent got as far as claiming it — and a session that died before that
+left nothing behind anywhere. That is why one completed steward run was invisible for four
+days.
+
+A Start now appends a history entry through the control API. **The status is deliberately
+unchanged**: the call re-asserts the status the task is already in. Advancing
+`approved` → `in-progress` here is the obvious-looking alternative and it breaks every
+plugin-started session — the agent's own first action is `update_task(in-progress)`, which
+task-queue-mcp permits only *from* `approved`. Doing it for the agent means its claim is
+rejected as an invalid transition.
+
+A failure to record is logged and does not fail the Start: the session is already running by
+then, and reporting the launch as failed would be the bigger lie.
 
 ## Development
 

manifest.json

@@ -1,7 +1,7 @@
 {
   "name": "task-queue",
   "displayName": "Task Queue",
-  "version": "0.8.0",
+  "version": "0.9.0",
   "description": "Task queue dashboard — view, filter, and launch agent tasks.",
   "author": "TadMSTR",
   "icon": "icon.svg",

package.json

@@ -1,6 +1,6 @@
 {
   "name": "cloudcli-plugin-task-queue",
-  "version": "0.8.0",
+  "version": "0.9.0",
   "private": true,
   "type": "module",
   "scripts": {