Task handoff records and human-approval state (v0.2.0)

CHANGELOG.md

@@ -6,6 +6,73 @@ All notable changes to this project are documented here. The format follows
 
 ## [Unreleased]
 
+## [0.2.0] - Unreleased
+
+Task handoff records and a human-approval state, based on community feedback. A supervisor should keep a
+blocked task as blocked, with the failed check, the owner and the exact next action, so a later agent
+can resume it without guessing.
+
+### Added
+- **Handoff record** on every finished task (`task.handoff`, persisted in `task.json`): `state` (`done`,
+  `needs_fix`, `needs_review`, `needs_approval`, `needs_input`, `blocked`, `retryable`, `closed`),
+  `failed_checks` (tests with command, exit code, failing test names and an excerpt; integrity; blocked
+  tool calls; timeout, stall or interrupt; worker error; missing `## RESULT`; no changes), `owner`,
+  `next_action`, `resume` (suggested tool + args, session reusable, worktree, branch), `context`,
+  `notes`, `history`, `updated_at` / `updated_by`. The daemon derives it deterministically from the
+  verdict and the data it already has. `task_result` returns it in full, and `task_status` and
+  `list_tasks` return a summary. Tasks finished before 0.2.0 get a derived record on the fly.
+- **Parked status `needs_approval`**: the task waits for a human decision when the worker reports
+  `status: needs_approval` / `needs_input`, or `blocked` with a request or after blocked tool calls.
+  No worker runs. `task_status` reports `terminal: true, parked: true`. Retention keeps the worktree
+  unless `retention.parked_days` is set. With it set, expired parked tasks lose their worktree (the
+  diff is archived) and are closed. `cleanup_task` on a parked task also closes it.
+- MCP tools `update_handoff` (owner, next_action, state, note; state `needs_approval` / `needs_input`
+  parks a task and any other state unparks it) and `approve_task` (`approve` resumes through the
+  continue path with the approval noted, and `reject` closes the task or redirects it with
+  instructions).
+- `list_tasks` filters `status: "needs_attention"` (parked tasks plus finished tasks whose handoff is
+  not done or closed) and `status: "parked"`, and a new `owner` filter.
+- CLI: `workhorse handoff <id>` (show or update), `workhorse approve <id>`, `workhorse reject <id>`,
+  `workhorse attention`, `workhorse tasks --status/--owner`, `workhorse cleanup-old --parked-days N`.
+- Worker contract: RESULT statuses `needs_approval` and `needs_input`, plus an optional `needs:` line
+  for the one exact request.
+- `test_results.failing_tests` (names parsed from the full test log) when tests fail.
+- `health_report`: `parked_tasks`, `oldest_parked_days`, `needs_attention`.
+- Audit events `parked`, `handoff_updated`, `approval`, `closed`. `finished` now carries
+  `handoff_state` and `owner`.
+- Docs: [docs/handoff.md](docs/handoff.md). The delegation skill draft now tells the supervisor to follow
+  `handoff.next_action`, relay approvals and hand off with `update_handoff`.
+
+### Changed
+- `continue_task` also accepts parked (`needs_approval`) tasks. Each `previous_results` entry records
+  the handoff it replaced, and the approval that answered it.
+- `cancel_task` on a parked task closes it (status `cancelled`, handoff `closed`).
+- `list_tasks status: "terminal"` also includes parked tasks, since no worker is running.
+- `update_handoff state: "closed"` on a parked task closes it like `cancel_task` / reject without
+  instructions (status `cancelled`, handoff `closed`) instead of unparking it to `completed`.
+- Approvals, handoff updates and closes record `source` (`channel` mcp/cli, `auth`) next to the
+  free-text `by`, so attribution is not only a caller-chosen name.
+
+### Fixed
+- A task stopped by a graceful daemon shutdown (`workhorse stop`, SIGTERM) was marked `interrupted`
+  but never finalized, so it had no result and no handoff. It is now finalized on the next start.
+- The daemon's shutdown removed the socket path unconditionally after finalizing tasks, which could
+  delete the socket of a daemon started in the meantime. It now removes only its own socket (same
+  inode), writes `run/daemon.pid`, and a new daemon waits for a previous one that is still stopping
+  before recovery. `workhorse stop` / `restart` wait for the processes to exit.
+- `cleanup_task` and retention mark a task before their first async step. While its worktree is being
+  removed, `continue_task`, `approve_task`, `update_handoff` and a second cleanup are refused instead of
+  queueing a run into a worktree that is being deleted.
+- Audit log: the `run_started` event's run kind (`initial`, `continue`, `retry`, `fallback`) overwrote
+  the record's `kind` (`task`). It is now `run_kind`. Reserved audit fields (`ts`, `kind`, and
+  `task_id`, `event`, `status` on task events) can no longer be overwritten by event data. A clashing
+  key is kept as `data_<key>`.
+
+### Compatibility
+- Existing statuses, verdicts and tool arguments are unchanged. A parked task's verdict is the existing
+  `blocked`. Tasks that used to end as `completed` + `blocked` are now `needs_approval` + `blocked` when
+  the worker made a concrete request or hit blocked tool calls. Without either, they stay `completed`.
+
 ## [0.1.0] - 2026-09-26
 
 First public release.
@@ -34,4 +101,5 @@ First public release.
 - Draft Grok Bot skills in `grok-template/` (getting started, delegation).
 
 [Unreleased]: https://github.com/mrchatam/Grok-workhorse/compare/v0.1.0...HEAD
+[0.2.0]: https://github.com/mrchatam/Grok-workhorse/compare/v0.1.0...HEAD
 [0.1.0]: https://github.com/mrchatam/Grok-workhorse/releases/tag/v0.1.0

README.md

@@ -12,7 +12,7 @@ you (or your supervisor) review the branch and decide.
 
 - Repo: https://github.com/mrchatam/Grok-workhorse
 - License: MIT (vendored skills: MIT, see [NOTICE](NOTICE))
-- Status: v0.1.0, Linux only
+- Status: v0.2.0 in development (latest release v0.1.0), Linux only
 
 ## Features
 
@@ -23,6 +23,7 @@ you (or your supervisor) review the branch and decide.
 - **Defense in depth**: a repo allowlist, a test-command allowlist (anchored regexes), an allowlist of clone hosts, a guard plugin/hook for git mutation, network clients and secret access, a read-only git object store, and root-owned config.
 - **Structured results**: verdict (`success`, `tests_failed`, `no_changes`, `blocked`, `integrity_violation`, ...), diffstat, daemon-run test results, the worker's self-report, concerns, token usage and timings. Raw logs are paged on demand.
 - **Follow-ups and reviews**: `continue_task` resumes the same session and worktree. `mode: "review"` runs a read-only reviewer on another task's diff.
+- **Handoff records and human approval**: every finished task says who acts next (`owner`), the one exact `next_action`, which checks failed, and how to resume. A worker that needs a human decision parks the task as `needs_approval` (worktree kept) until someone answers with `approve_task`. Supervisors record their own handoffs with `update_handoff`. See [docs/handoff.md](docs/handoff.md).
 - **Operations**: stall detection (15 min by default), wall-clock timeouts, cancel, recovery after a daemon restart, retention sweeps, an append-only JSONL audit log, and `workhorse health` for daily checks.
 - **Credentials from the environment first** (daemon env, or the MCP connector env passed through the shim), with an optional secret-store fallback. Keys never appear in argv, logs or results.
 
@@ -44,7 +45,7 @@ flowchart LR
   R --> M
 ```
 
-More detail: [docs/architecture.md](docs/architecture.md).
+More detail: [docs/architecture.md](docs/architecture.md). Handoff records and the approval flow: [docs/handoff.md](docs/handoff.md).
 
 ## Security model (short version)
 
@@ -166,8 +167,8 @@ tests only. See [docs/adapters.md](docs/adapters.md) for the adapter interface a
 - **getting-started**: walks a new user through choosing a provider and model, adding repos, storing
   the key securely, running the installer, registering the stdio connector and running the first task.
 - **delegation**: how a supervisor should delegate. It reads `list_models` and `list_repos`, writes
-  self-contained task descriptions, polls, reviews the result and diff, uses `continue_task` for fixes,
-  and merges or cleans up.
+  self-contained task descriptions, polls, reviews the result and diff, follows `handoff.next_action`,
+  uses `continue_task` for fixes, relays approvals, and merges or cleans up.
 
 Copy them into your Grok Bot skills if you want them. Nothing in this repo installs them automatically.
 
@@ -178,6 +179,9 @@ workhorse start|stop|restart|status     supervisor + daemon (runs as the service
 workhorse health [--json]               full check (exit 1 on FAIL), good for a daily cron
 workhorse backends                      adapters: status, installed, capabilities
 workhorse tasks | logs | audit          recent tasks, daemon log, audit log
+workhorse attention                     tasks that need someone (parked or handoff not done)
+workhorse handoff <id> [--owner O --next "..." --note "..." --state S]   show or update a handoff
+workhorse approve <id> | reject <id>    answer a parked (needs_approval) task
 workhorse cleanup-old [--days N]        retention sweep now
 workhorse repos | add-repo | remove-repo | validate | check-provider | hello
 ```

adapters/kilo/config/AGENTS.md

@@ -6,14 +6,15 @@ You are an unattended coding worker. A supervisor agent will read your final mes
 2. **Git.** Never commit, push, merge, rebase, reset, stash, switch/checkout branches, create branches/tags, or change git config/remotes. Leave every change uncommitted; the supervisor handles integration. `git status`, `git diff`, `git log` and `git checkout -- <file>` / `git restore <file>` are fine. Create and edit files with the write/edit tools, not shell heredocs or `echo >`: a shell command whose text mentions `git commit`/`git push` (even inside a heredoc body) is denied by policy, while file content written with the write/edit tools is not checked.
 3. **Conventions.** Read the repo's README / AGENTS.md and the code around your change first. Follow existing style and structure. Keep the change minimal and focused on the task. Do not add dependencies unless the task explicitly requires it.
 4. **Verify.** Run the test command given in the task (otherwise discover the project's tests) before finishing, and fix failures you caused. Commands run in a sandbox without network access, so do not try to install packages or reach the internet. Any toolchain caches the operator provides (for example a read-only Go module cache or npm cache) are preconfigured through environment variables: do not set or override HOME, XDG_* or tool cache variables, and never create caches or tool directories (`go/`, `.config/`, `node_modules/.cache` and similar) inside the worktree unless the task asks for it. If a build tool still fails for environment reasons (missing toolchain, dependency not in the offline cache), stop retrying and report the exact error under concerns with status blocked or partial.
-5. **No questions.** If something is ambiguous, choose the most reasonable interpretation, proceed, and record the assumption under concerns.
+5. **No questions.** If something is ambiguous, choose the most reasonable interpretation, proceed, and record the assumption under concerns. Only when you truly cannot continue without a human (an approval, e.g. to add a dependency or run an action the policy/sandbox blocked, or an answer only the owner has), stop and finish with status needs_approval (or needs_input) and put the one exact request under needs.
 6. **Stay on task.** Do not refactor unrelated code, rewrite tests to make them pass unless the task says the tests are wrong, or delete tests.
 7. **Keep going until done.** Every reply before your final one must contain a tool call. Never end a reply with only a statement of what you will do next ("Now let me update X:"); that ends the run with the work unfinished. Do the step instead.
 8. **Finish** with this block as the very last thing in your final message (plain text, no code fence):
 
 ## RESULT
-status: done | partial | blocked
+status: done | partial | blocked | needs_approval | needs_input
 summary: <1-3 sentences on what you changed and why>
 files_changed: <comma-separated relative paths, or none>
 tests: <command you ran> -> <pass/fail with counts>
 concerns: <none, or short semicolon-separated list: assumptions, risks, follow-ups>
+needs: <none, or (for blocked / needs_approval / needs_input) the one exact action, approval or answer you need, e.g. approve adding dependency foo to package.json>

bin/workhorse

@@ -13,9 +13,19 @@ Service:
   status                          quick daemon status (exit 1 if not running)
   health [--json]                 full check for a daily cron/agent run (exit 1 on any FAIL)
   logs [N] | audit [N]            tail daemon.log / audit.jsonl
-  tasks [N]                       recent tasks
-  cleanup-old [--days N] [--task-days N] [--dry-run]
-                                  remove finished worktrees older than N days (default: daemon.json retention)
+  tasks [N] [--status S] [--owner O]
+                                  recent tasks (S: a status, active, terminal, parked, needs_attention)
+  attention [N]                   tasks that need someone: parked (needs_approval) or handoff not done/closed
+  cleanup-old [--days N] [--task-days N] [--parked-days N] [--dry-run]
+                                  remove finished worktrees older than N days (default: daemon.json retention;
+                                  parked tasks are kept unless --parked-days / retention.parked_days is set)
+Handoff and approval:
+  handoff <task_id> [--state S] [--owner O] [--next "<action>"] [--note "<text>"] [--by NAME]
+                                  show the task's handoff record, or update it when options are given
+  approve <task_id> [--instructions "<text>"] [--note "<text>"] [--timeout MIN] [--profile P] [--by NAME]
+                                  approve a parked/blocked task and resume it in the same session + worktree
+  reject <task_id> [--instructions "<text>"] [--note "<text>"] [--by NAME]
+                                  deny it: close the task, or resume with --instructions (do something else)
 Config (the config dir is root-owned once locked: use sudo for these):
   repos                           show the allowlist
   add-repo <git-url|path> [--name N] [--test "<cmd>"] [--allow-test "<regex>"]... [--base BRANCH]
@@ -67,8 +77,16 @@ async function stop() {
   fs.mkdirSync(dirs.run, { recursive: true, mode: 0o700 })
   fs.writeFileSync(path.join(dirs.run, "stop"), "")
   if (sup) try { process.kill(sup, "SIGTERM") } catch {}
-  for (let i = 0; i < 60 && fs.existsSync(SOCKET_PATH); i++) await wait(500)
-  out("stopped")
+  // Wait for the processes to exit, not just for the socket to go away: a daemon removes its socket
+  // first and then finalizes running tasks, and a successor must not start before it is gone.
+  const { isDaemonPid, daemonPidFromFile } = await import("../lib/daemon.mjs")
+  const dpid = daemonPidFromFile()
+  const alive = (pid) => {
+    try { process.kill(pid, 0); return true } catch (e) { return e.code === "EPERM" }
+  }
+  let i = 0
+  for (; i < 240 && ((sup && alive(sup)) || (dpid && isDaemonPid(dpid))); i++) await wait(250)
+  out(i >= 240 ? `still stopping after 60 s (supervisor ${sup || "-"}, daemon ${dpid || "-"})` : "stopped")
 }
 
 try {
@@ -123,13 +141,39 @@ try {
       out(execFileSync("tail", ["-n", rest[0] || "20", path.join(dirs.logs, "audit.jsonl")]).toString())
       break
     case "tasks":
-      out(await rpc("list_tasks", { limit: Number(rest[0]) || 20 }))
+    case "attention": {
+      const f = flags(rest, { status: "str", owner: "str" })
+      const status = cmd === "attention" ? "needs_attention" : f.status
+      out(await rpc("list_tasks", { limit: Number(f._[0]) || 20, ...(status ? { status } : {}), ...(f.owner ? { owner: f.owner } : {}) }))
+      break
+    }
+    case "handoff": {
+      const f = flags(rest, { state: "str", owner: "str", next: "str", note: "str", by: "str" })
+      const id = f._[0]
+      if (!id) throw new Error("usage: workhorse handoff <task_id> [--state S] [--owner O] [--next \"<action>\"] [--note \"<text>\"] [--by NAME]")
+      const upd = { ...(f.state ? { state: f.state } : {}), ...(f.owner ? { owner: f.owner } : {}), ...(f.next ? { next_action: f.next } : {}), ...(f.note ? { note: f.note } : {}) }
+      const caller = { caller: { client: "workhorse" } }
+      out(Object.keys(upd).length ? await rpc("update_handoff", { task_id: id, ...upd, ...(f.by ? { by: f.by } : {}) }, caller) : await rpc("get_handoff", { task_id: id }, caller))
       break
+    }
+    case "approve":
+    case "reject": {
+      const f = flags(rest, { instructions: "str", note: "str", timeout: "str", profile: "str", by: "str" })
+      const id = f._[0]
+      if (!id) throw new Error(`usage: workhorse ${cmd} <task_id> [--instructions "<text>"] [--note "<text>"]`)
+      out(await rpc("approve_task", {
+        task_id: id, decision: cmd,
+        ...(f.instructions ? { instructions: f.instructions } : {}), ...(f.note ? { note: f.note } : {}),
+        ...(f.timeout ? { timeout_minutes: Number(f.timeout) } : {}), ...(f.profile ? { profile: f.profile } : {}), ...(f.by ? { by: f.by } : {}),
+      }, { caller: { client: "workhorse" } }))
+      break
+    }
     case "cleanup-old": {
-      const f = flags(rest, { days: "str", "task-days": "str", "dry-run": "bool" })
+      const f = flags(rest, { days: "str", "task-days": "str", "parked-days": "str", "dry-run": "bool" })
       out(await rpc("cleanup_old", {
         ...(f.days !== undefined ? { worktree_days: Number(f.days) } : {}),
         ...(f["task-days"] !== undefined ? { task_days: Number(f["task-days"]) } : {}),
+        ...(f["parked-days"] !== undefined ? { parked_days: Number(f["parked-days"]) } : {}),
         dry_run: !!f["dry-run"],
       }, { caller: { client: "workhorse" }, timeoutMs: 600000 }))
       break