feat(kanban): direct-command workers via profile-level worker.command

[HirokiKobayashi-R]

What

A named profile can declare a fixed argv as its kanban worker:

# ~/.hermes/profiles/engine/config.yaml
worker:
  command:
    - /usr/local/bin/my-pipeline
    - --from-kanban

Cards assigned to that profile are executed by that command instead of the Hermes agent, with the board machinery unchanged — dispatcher, worktree workspaces, per-task logs, crash detection and max_runtime all behave exactly as for agent workers. Deterministic pipelines (an orchestrator binary, a build-and-ship script) get a first-class seat on the board without wrapping themselves in a prompt, and without being forced to speak kanban either: the command's exit code is its completion report (rc=0 → complete, anything else → block with the code/signal named, no retry — unblock re-runs). A command that already moved its card through a canonical channel keeps its own word; the translation is then a no-op, pinned by expected_run_id.

How

The dispatcher spawns a thin supervisor (hermes_cli.kanban_command_worker, launched with -P so the task workspace on cwd can never shadow the module) that runs the declared argv as its direct child, waits, and reports through the canonical transitions while still alive. That single decision is what makes the contract hold everywhere: the exit code is observed under every dispatcher constitution (resident gateway, kanban daemon, throwaway cron ticks), the card leaves running before the supervisor exits so no reclaim pass can race the report, and nothing about the worker is ever reconstructed at reap time — detect_crashed_workers is untouched.

Safety properties, each with a test:

• The declaration is host-side configuration only. Nothing on a card (title, body, comments, attachments) can alter what is executed.
• It must live in a named profile's config (.../profiles/<name>), judged structurally from the home path — a root-config declaration is rejected (the default assignee resolves to the root home), and the guard stays correct when the dispatcher itself runs inside a profile home.
• The profile config is parsed directly, not via load_config(): a YAML typo raises instead of silently running the agent via the loader's default/last-known-good fallback.
• argv[0] must be absolute or a bare PATH name — a relative path would resolve against the per-task workspace, whose content the task's own branch controls.
• On SIGTERM (enforce_max_runtime's first shot) the supervisor forwards to the child's whole process group (own session — grandchildren die too) and, after a grace clamped under the dispatcher's term-to-kill window, SIGKILLs the group and still reports. The handler only forwards; the grace lives in the main wait loop (a Popen.wait inside the handler deadlocks against the main thread's own wait).

Review

Developed under four rounds of adversarial review (nine must-fix findings, all resolved and pinned by tests), including: the silent-fallback config loader, an EX_TEMPFAIL requeue loop, the kanban-namespace scope inconsistency, a root-guard inversion under profile-home dispatchers, unobservable exit statuses under throwaway dispatchers, a reclaim-before-translation race, reap-time worker-kind guessing, workspace shadowing of the supervisor module via -m's cwd injection, and a handler-deadlocked termination grace.

Tests

20 new tests: spawn/argv-freezing, fail-loud resolution (invalid shapes, YAML parse failure, root rejection in both constitutions, relative argv[0]), the supervisor as a real subprocess (rc0→done, rc3→blocked, missing executable → its own failure with no transition, respecting the command's own transition), a mock-free dispatch_once integration test to done, workspace-shadowing A/B, and SIGTERM against a SIGTERM-ignoring child finishing reported and child-dead inside the kill window. tests/hermes_cli -k kanban: 310 passed; the pre-existing failure set is byte-identical to main. The agent spawn path is byte-identical to main throughout.

https://claude.ai/code/session_018ao2GE6xFgExNgzQBsZib3

[andrexibiza]

Reviewed exact head 4abe503937888b529197a2f86a3118d453937433 against base/current main 13ce0c5c675e843af70d19c9e5144249cd51c8d1. I inspected the full four-file change, the existing Kanban claim/run/reaper/timeout contracts, _default_spawn's environment and cwd boundary, the new supervisor's process semantics, the new regression suite, exact-head workflow state, and the adjacent Kanban execution/security work (#31363, #82936/#83565/#55600, and #89934/#84333).

The basic shape is good: resolving worker.command from host-side profile config rather than card data is the right authority boundary; failing loudly on malformed YAML is materially safer than falling through to the agent; -P correctly closes Python module shadowing from the task workspace; expected_run_id is the right stale-run fence; and making the supervisor report while it is still alive avoids inventing worker-kind state during reap. I also like that agent-only task knobs are explicitly ignored rather than half-applied to a non-agent worker.

I found four merge blockers at the execution/lifecycle boundary.

1. max_runtime is no longer the existing Kanban timeout contract

The PR says max_runtime behaves exactly as for agent workers, but the new SIGTERM test actually pins a different state machine.

On current main, enforce_max_runtime() owns timeout semantics: it SIGTERMs the recorded worker PID, waits ~5s, SIGKILLs if necessary, then—while the task is still running—records a timed_out run/event, restores the source phase for retry, and increments the unified consecutive_failures counter/circuit breaker.

The command supervisor instead catches that SIGTERM, forwards it to the child, kills the child after its own grace, then translates the child's negative return code through block_task():

if returncode < 0:
    reason = f"worker command terminated by signal {-returncode}"
...
kb.block_task(... expected_run_id=run_id)

The new test explicitly requires the card to be blocked after SIGTERM. That transition occurs inside the dispatcher's timeout grace. When enforce_max_runtime() resumes, its running CAS has already lost, so it cannot record timed_out, cannot requeue, and cannot advance the existing timeout failure counter. A deterministic command failure being non-retriable is a defensible policy; a dispatcher-enforced timeout silently becoming a human block is not the existing policy and contradicts the PR's compatibility claim.

Please leave timeout authority with the dispatcher. A supervisor terminated by dispatcher/service signal should tear down descendants and exit without asserting an ordinary command outcome; enforce_max_runtime() should then remain the writer of timed_out/retry/circuit-breaker state. Add an end-to-end witness that starts a real command worker, expires max_runtime, and proves the run event/status, retry phase, and failure counter are byte-for-byte equivalent to an agent worker timeout.

2. A bare argv[0] is not actually bound to host-side code while cwd is the task workspace

_resolve_worker_command() rejects ./run.sh but explicitly allows a bare executable name. _default_spawn() then starts the supervisor with cwd=workspace, and the supervisor runs:

subprocess.Popen(argv, start_new_session=True)

with the bare name still unresolved.

That means the executable is selected after entering the untrusted workspace. On POSIX, an empty/relative PATH component (:, ., bin, etc.) resolves against that workspace. On Windows, executable lookup can consult the current directory directly. A worktree-controlled my-pipeline/my-pipeline.exe can therefore satisfy a profile declaration of worker.command: [my-pipeline, ...] under legal host environments. The card did not rewrite the YAML, but its branch can still change which bytes the host declaration executes—the exact property the relative-path guard is trying to forbid.

Please resolve a bare name to an absolute executable path at dispatch/config-resolution time against a trusted PATH contract, reject ambiguous/relative/empty PATH components, and freeze that absolute path into HERMES_KANBAN_WORKER_COMMAND; alternatively require absolute argv[0]. Add a hostile-workspace witness for PATH=.:... / an empty PATH element and the Windows current-directory case.

3. The process-tree contract is POSIX-only, but this is an unguarded cross-platform code path

The new supervisor relies on:

subprocess.Popen(argv, start_new_session=True)
os.killpg(pgid, signum)

Current Hermes already centralizes why this is unsafe on Windows in hermes_cli/_subprocess_compat.py: start_new_session=True is ignored there; Windows needs explicit creation flags/process-group handling. os.killpg is not a Windows API at all. The helper windows_detach_popen_kwargs() exists specifically to replace the start_new_session=True pattern.

This matters especially here because max_runtime targets the supervisor PID. On Windows, the dispatcher's os.kill(pid, SIGTERM) termination path can end the supervisor without its POSIX signal-forwarding contract ever running, leaving the direct child/grandchildren alive after the board has reclaimed/retried the card. Even aside from that, _signal_group() only catches OSError; an absent os.killpg raises AttributeError.

Please route child creation/termination through the repository's Windows-aware process primitives (or add a purpose-built tree owner/terminator) and add a native Windows witness proving supervisor termination kills descendants and leaves the same Kanban outcome as POSIX. Exact-head hosted CI is currently action_required, so there is no upstream Windows matrix evidence to close this by observation.

4. This feature inherits the known Kanban worker credential-leak boundary and gives it a new arbitrary-command consumer

Current main still begins _default_spawn() with env = dict(os.environ). This branch keeps that environment, adds HERMES_KANBAN_WORKER_COMMAND, and the supervisor's child inherits it unchanged. Open issue #82936 documents this exact Kanban spawn seam leaking the dispatcher/default profile's secrets into secondary-profile workers, and tracker #83565 binds the class. #55600 by @necoweb3 is the direct sibling fix—replace raw parent-env inheritance with the sanitized subprocess environment—but it remains open.

For an agent worker that is already a security defect; for this PR it becomes a first-class profile-configured arbitrary binary receiving the same cross-profile credentials. That expands the known sink before the boundary is closed.

Please compose the Kanban env scrub into this branch (preserving #55600/@necoweb3 attribution if that implementation is reused) or make the merge dependency explicit and restack after the canonical env-boundary fix. The acceptance test should seed a parent/default secret and prove a command worker assigned to another profile cannot observe it, while the explicit worker/Kanban env contract still arrives.

Interlocks / ownership / non-duplicates

• #82936 + #83565 + #55600 / @necoweb3 are the same child-environment trust boundary this new consumer sits on. This is a merge-order dependency, not duplicate feature work.
• #89934 (refresh of #84333) is adjacent but not a duplicate. It introduces a governed builder capability with its own allowlist/evidence/authority contract; this PR introduces a generic profile-owned worker executable. If both land, keep those policy surfaces distinct rather than letting worker.command accidentally become an alternate path around builder governance.
• #31363 remains the other side of the lifecycle shape: a human Block on a running task is DB-only and does not terminate the worker. expected_run_id prevents the supervisor from overwriting that canonical transition later, which is good, but a build-and-ship command can still continue external side effects after the UI says Blocked. This PR does not need to solve #31363, but the direct-command docs should state that limitation until the running-worker interruption contract lands.

CI / proof state

The author reports 310 Kanban tests green locally with 20 new tests. At the exact reviewed head, upstream CI, Docker, and Nix all ended action_required, so no hosted exact-head job matrix executed. That is an approval-gate state, not evidence of a code failure, but it means the cross-platform/process findings above are not displaced by CI.

Re-review gate: preserve dispatcher-owned timeout semantics; bind bare commands to trusted absolute executables; make child-tree termination truly cross-platform; close/compose the Kanban credential-env boundary; then run the exact-head Kanban + native Windows/process matrix.