fix(kanban): strip worker Kanban env from terminal-spawned subprocesses

[DavidMetcalfe]

fix(kanban): strip worker Kanban env from terminal-spawned subprocesses

Summary

A dispatcher-owned Kanban worker legitimately carries HERMES_KANBAN_* in its own env, but a nested hermes CLI it launches through the terminal tool inherits that identity and is accepted as the parent run owner — it can call kanban_complete / kanban_block / kanban_heartbeat against the parent's card while the real worker is still running. The merged delegate_task isolation (#56647/#69837) only covers delegated children; a nested full CLI spawned through the terminal was never scrubbed.

Production incident (reported in #81508): a read-only nested reviewer (hermes -z --model … -t terminal launched from a worker) inherited the parent's task/run env, appended a task comment, and completed the parent card. The owning worker's later kanban_block(reason="review-required: …") was rejected because the card was already terminal.

Root cause

Several subprocess env builders only stripped HERMES_KANBAN_* when the spawner was a delegate_task child (is_delegated_child_process_context()):

• tools/environments/local.py::_make_run_env — terminal foreground commands
• tools/environments/local.py::_sanitize_subprocess_env — background/PTY spawns via process_registry.spawn_local, watchers, cua-driver
• tools/code_execution_tool.py::_scrub_child_env — execute_code sandbox

A plain worker-spawned nested process is not a delegated child, so none of these scrubbed — the nested CLI inherited the full dispatcher identity, and the env-based ownership gates (tools/kanban_tools.py::_worker_run_id, _enforce_worker_task_ownership, model_tools.py kanban toolset auto-injection, turn_finalizer.py, session_context.py) all accepted it as the parent worker.

Fix

Strip HERMES_KANBAN_* unconditionally at the terminal-tool subprocess boundary, without setting the HERMES_DELEGATED_CHILD_CONTEXT lineage marker (a plain nested spawn is not a delegate_task child):

• New strip_kanban_env() in agent/delegation_context.py (strip, no marker), with scrub_kanban_env() refactored on top of it (strip + marker for delegated children).
• _make_run_env, _sanitize_subprocess_env, and _scrub_child_env now scrub unconditionally, closing the execute_code passthrough escape hatch for plain worker-spawned children.
• Fail closed: if agent.delegation_context cannot be imported, a HERMES_KANBAN_* prefix sweep still strips the identity rather than returning the env unscrubbed.

Intentionally unchanged

• hermes_subprocess_env keeps its delegated-child-only scrub: it feeds the codex app server / copilot ACP runtimes, which are the worker's own execution surface and must retain HERMES_KANBAN_TASK to write completion/block back to the board (agent/transports/codex_app_server.py:102, agent/transports/hermes_tools_mcp_server.py).
• The dispatcher's own worker spawn (kanban_db._default_spawn) builds its env explicitly and is unaffected.

Validation

• New regression tests: TestKanbanNestedSpawnScrub in tests/tools/test_local_env_blocklist.py (foreground / background / execute_code boundaries, passthrough cannot re-grant, no false delegated marker); test_non_delegated_worker_keeps_kanban_env_for_runtime in tests/tools/test_hermes_subprocess_env.py (codex runtime constraint); _legacy_posix_scrubber oracle updated in tests/tools/test_code_execution_windows_env.py per that file's own convention for deliberate POSIX scrub changes.
• pytest focused suites: 259 passed, 3 skipped (kanban tools, delegation isolation, cron kanban env isolation, local env, process registry, terminal tool, code execution).
• Full tests/tools + kanban DB/CLI suites: 5732 passed; 45 failures are pre-existing environment-dependent failures identical on unchanged main (daytona/docker, pulseaudio, wake-word models) — zero new failures.
• Real-subprocess smoke test: a child spawned with worker env via both _make_run_env and _sanitize_subprocess_env sees zero HERMES_KANBAN_* vars and no false HERMES_DELEGATED_CHILD_CONTEXT.

Notes

This PR provides the environment-level isolation fix for the nested-CLI ownership leak. The complementary database-level fix — four-field writer CAS (status + claim_lock + worker_pid + current_run_id) fencing terminal transitions to the full attempt identity — is planned in #79543 / #81324 and hardens this boundary further, including the sibling stale-worker race in #71175. The existing open PR #70898 takes a broader ContextVar-based ownership approach; this PR intentionally keeps a smaller footprint and does not collide with the #79543/#81324 plan's declared ownership of kanban_db.py.

Closes #70809. Closes #81508 (duplicate).

[SharadKumar]

Downstream dependency — what does this need to land?

This PR is the narrow fix for the nested-subprocess ownership track (#70809), and it currently gates nterprise-ai/claudius#870, a P0 in our repository. From the outside it looks ready: open, mergeable, required CI green.

Is there anything blocking it that a contributor could help with — review, additional regression coverage, or a rebase? Happy to supply either.

Production impact while it is open, from a fleet running Hermes Kanban daily. The inherited-environment path has reproduced repeatedly: a nested hermes -z review session launched from inside a Kanban worker inherited HERMES_KANBAN_TASK, received the Kanban lifecycle capability despite being a read-only terminal toolset conversation, and called kanban_complete on its parent task — moving the card to done while the owning builder was still mid-handoff. The owner's subsequent kanban_block then failed because the card was already terminal.

That is exactly the class this PR closes by stripping HERMES_KANBAN_* at the subprocess boundary.

The separate retry/old-worker overlap track (#71175, PR #71189) remains open too, and together they have produced duplicate concurrent implementations of the same task — two live workers, two PRs for one issue, one of which had to be closed as superseded while preserving its branch as evidence. We have not built any downstream workaround for either, deliberately: the fencing belongs in the runtime, and a local shim would only mask it.

Full attempt fencing (status + claim_lock + worker_pid + current_run_id) is the durable answer, but this PR is the smaller, shippable half.

[DavidMetcalfe]

@SharadKumar — I appreciate the summary and the offer to help.

Current status, answering directly:

• Rebase: not needed today — the PR is open, mergeable, and required CI is green, with no conflicts. If main moves and a conflict arises, I'll rebase immediately.
• Review: the PR is awaiting maintainer review; no review requests are pending and no reviews have been submitted yet, so nothing is blocked on a code change. If your team can review or test the diff against your fleet reproduction (nested hermes -z session inheriting HERMES_KANBAN_TASK), any technical feedback or confirmation on this thread is very welcome.
• Coverage: regression tests cover the three spawn boundaries — foreground (_make_run_env), background/PTY (_sanitize_subprocess_env), and execute_code (_scrub_child_env) — plus a real-subprocess smoke test and the codex-runtime keep case (test_non_delegated_worker_keeps_kanban_env_for_runtime).

This PR deliberately leaves kanban_db.py untouched, so it won't collide with the DB-level fencing (#79543/#81324) or #70898.

I can rebase if needed and add any extra coverage you identify.

[enzo-adami]

I found a gap between the documented fail-closed prefix contract and the successful-import path on this HEAD (ba9d497b3b). strip_kanban_env() removes only the seven names in KANBAN_ENV_KEYS; current main also dispatches HERMES_KANBAN_BRANCH, HERMES_KANBAN_GOAL_MODE, and HERMES_KANBAN_GOAL_MAX_TURNS, and #86609 adds HERMES_KANBAN_WORKER_SCOPE.

Deterministic probe through the real _make_run_env() on this PR:

HERMES_KANBAN_TASK=None
HERMES_KANBAN_RUN_ID=None
HERMES_KANBAN_BRANCH=project/t_parent
HERMES_KANBAN_GOAL_MODE=1
HERMES_KANBAN_GOAL_MAX_TURNS=25
HERMES_KANBAN_WORKER_SCOPE=lifecycle-only

In other words, the import-failure fallback strips every HERMES_KANBAN_* key, but the normal path is weaker and leaks current/future dispatcher-owned state. The existing tests only iterate a hand-written _WORKER_ENV, so they don't catch parity drift.

Suggested fix: make strip_kanban_env() use the same prefix sweep as the fallback (or derive the canonical set from the dispatcher's pinned worker keys), while preserving HERMES_DELEGATED_CHILD_CONTEXT behavior separately. Add a regression that seeds unknown/future HERMES_KANBAN_SENTINEL plus the branch/goal keys and asserts every prefixed key is absent from both _make_run_env() and execute-code children. This also makes the PR compose safely with #86609's lifecycle scope marker.

[enzo-adami]

I turned the reproduced normal-path gap into a focused draft extension against your exact branch: DavidMetcalfe#1. It makes the nested terminal/execute-code boundary prefix-wide, while leaving your Codex/ACP runtime exception unchanged. Regression matrix now includes BRANCH, GOAL_*, WORKER_SCOPE, and an unknown future capability; 87 targeted tests pass, Ruff/Windows/diff-check pass.

[DavidMetcalfe]

@enzo-adami — good catch. Verified and applied.

Confirmed: strip_kanban_env() only removed the 7 KANBAN_ENV_KEYS, while the dispatcher also pins HERMES_KANBAN_BRANCH / HERMES_KANBAN_GOAL_MODE / HERMES_KANBAN_GOAL_MAX_TURNS in the worker env (hermes_cli/kanban_db.py:9147-9158) — and issue #86609 adds HERMES_KANBAN_WORKER_SCOPE. My import-failure fallback prefix-swept everything, so the normal path was strictly weaker than the fail-closed path. That's a real parity gap for goal-mode and branch-pinned workers, and the failure mode only grows as new HERMES_KANBAN_* keys land.

Applied as cherry-pick of your commit 34c5d23f3a (authorship preserved) → now at PR head 5f1b678bee:

• strip_kanban_env() is now a full HERMES_KANBAN_* prefix sweep, matching the fail-closed fallback exactly (one contract, no drift). KANBAN_ENV_KEYS stays public as the documented historical contract for existing callers/tests.
• Regression matrix extended with BRANCH, GOAL_MODE, GOAL_MAX_TURNS, WORKER_SCOPE, and an unknown-future HERMES_KANBAN_FUTURE_CAPABILITY drift oracle in TestKanbanNestedSpawnScrub — so every existing boundary test (foreground / background / execute_code / real subprocess) now asserts all prefixed keys are absent.
• Codex/ACP runtime exception unchanged (hermes_subprocess_env still keeps the env for worker write-back).

Verification:

• Targeted suites: 230 passed, 3 skipped. Existing tests that read KANBAN_ENV_KEYS directly (e.g. the drift guard in test_cron_kanban_env_isolation.py) remain fully compatible and unaffected.
• Mutation check: reverting strip_kanban_env to the 7-key version fails exactly the 4 boundary tests (test_worker_terminal_foreground_spawn_strips_kanban_env, test_worker_terminal_background_spawn_strips_kanban_env, test_worker_terminal_spawn_real_subprocess_sees_no_kanban_env, test_worker_execute_code_spawn_strips_kanban_env) — the drift oracle is load-bearing, not decorative.
• Real-subprocess probe through _make_run_env(): zero HERMES_KANBAN_* keys reach the child (incl. BRANCH/GOAL_*/WORKER_SCOPE/future key), no false HERMES_DELEGATED_CHILD_CONTEXT.