Skip to content

fix(kanban): scratch children no longer inherit parent workspace path - #67574

Closed
webtecnica wants to merge 1 commit into
NousResearch:mainfrom
webtecnica:fix/67567-kanban-scratch-workspace
Closed

webtecnica wants to merge 1 commit into
NousResearch:mainfrom
webtecnica:fix/67567-kanban-scratch-workspace

Conversation

@webtecnica

Copy link
Copy Markdown
Contributor

Problem

When a dispatcher-spawned Kanban worker creates child tasks via kanban_create without explicit workspace arguments, scratch children inherit the parent's literal workspace_path. This means the child points at the parent's already-materialized <board>/workspaces/<parent-id> directory instead of getting its own isolated workspace.

Production evidence: A remediation child editing inside a rejected-review directory invalidated review evidence, and allowed concurrent tasks to share one checkout/index.

Root cause

In tools/kanban_tools.py::_handle_create, lines 1130–1132, when _inherit_workspace is true, both workspace_kind and workspace_path are unconditionally copied from the parent task — without distinguishing the scratch case from shared dir/worktree paths.

Fix

Only inherit workspace_path when the parent's workspace_kind is NOT "scratch". Scratch children still inherit workspace_kind="scratch" but keep workspace_path=None, so the dispatcher materializes a fresh directory at <board>/workspaces/<child-id>.

Dir / worktree children continue to inherit the parent's path as before, keeping follow-up work inside the same project checkout.

Expected behavior after fix

  • Worker-created scratch children get workspace_path=None → resolved to their own <board>/workspaces/<child-id>
  • Dir / worktree children still inherit the parent's shared path
  • Explicit workspace args (workspace_kind, workspace_path) in kanban_create still override inheritance
  • CLI/dashboard callers (no HERMES_KANBAN_TASK) unchanged — still default to scratch
  • All 114 existing kanban tests pass

Closes #67567.

When a kanban worker creates a child task via kanban_create without
explicit workspace args, the child previously inherited the parent's
literal workspace_path.  For scratch tasks this meant the child pointed
at the parent's already-materialized directory instead of getting its
own isolated workspace at <board>/workspaces/<child-id>.

Now scratch children still inherit workspace_kind='scratch' but leave
workspace_path=None so the dispatcher materializes a fresh directory.
Dir / worktree children continue to inherit the parent's path, keeping
follow-up work in the same project checkout.

Closes NousResearch#67567.

@teknium1 teknium1 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the focused fix. The current-main implementation still unconditionally copies a worker parent's workspace_path in tools/kanban_tools.py:1132; for scratch tasks, resolve_workspace reuses a stored path but otherwise creates <workspaces_root>/<task-id> (hermes_cli/kanban_db.py:5866-5881). The conditional in this PR therefore addresses the reported isolation failure while retaining persistent dir/worktree inheritance.

Problems

  • No regression test covers a materialized scratch parent creating a child without workspace arguments. Existing coverage tests only the persistent-dir inheritance path at tests/tools/test_kanban_tools.py:1009-1036.

Suggested changes

  • Add a focused test asserting that such a child stays scratch with workspace_path is None; keep the existing dir inheritance test as the persistent-workspace contract.

Automated hermes-sweeper review.

Comment thread tools/kanban_tools.py
# the parent's. Dir / worktree children keep their
# inherited path so follow-up work lands in the same
# project checkout.
if _self_task.workspace_kind != "scratch":

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please add a regression test for this branch with a claimed scratch parent whose materialized workspace_path is set. Assert the omitted-workspace child remains scratch with workspace_path is None; the current test suite covers only the dir inheritance case.

@teknium1 teknium1 added sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform labels Jul 19, 2026
@PRATHAMESH75

Copy link
Copy Markdown
Contributor

Reviewed against #67567. This correctly fixes the reported scratch case: guarding workspace_path inheritance behind _self_task.workspace_kind != "scratch" means a worker-created scratch child now keeps workspace_path=None and resolves to its own <board>/workspaces/<child-id> — the exact isolation break in the production evidence. Minimal and readable.

Two parts of the issue's Expected behavior remain uncovered, though, because the else branch still copies the parent's literal path for non-scratch parents:

  1. "Sharing a persistent directory remains supported only when the creator explicitly passes workspace_kind=dir plus workspace_path." — Here a dir: worker that omits workspace args still implicitly inherits the parent's dir + literal path, so implicit sharing persists.
  2. "Project-linked children may inherit the project identity, but must materialize their own task worktree/path." — For a worktree/project-linked parent, the child still inherits the parent's literal workspace_path, so an independent-review child would land in the parent's worktree/checkout rather than a fresh .worktrees/<child-id>.

The sibling PR #67570 addresses these by inheriting only project_id (which create_task resolves into a fresh per-task worktree) and requiring explicit args for any directory sharing, plus a test matrix covering the nested / project-linked / explicit-dir paths. Given #67570 closes the full scope of the issue, maintainers may prefer it — but that's their call. No correctness problem with this diff for the scratch case it targets.

@alt-glitch alt-glitch added type/bug Something isn't working P3 Low — cosmetic, nice to have comp/cron Cron scheduler and job management needs-decision Awaiting maintainer decision before any implementation labels Jul 19, 2026
@alt-glitch

Copy link
Copy Markdown

This was generated by AI during triage.

Related to #67570: this patch preserves explicit dir/worktree inheritance while #67570 changes the broader default child-workspace contract. Maintainer choice is needed.

@teknium1

Copy link
Copy Markdown
Collaborator

Closing as superseded by #70143 (merged, SHA 781968b), which salvaged #67570. That change removed the implicit literal-path inheritance in _handle_create entirely — the block this PR patched no longer exists. Omitted workspace args now always produce a fresh isolated scratch child; explicit dir sharing and project-linked worktree materialization are preserved. Thanks for the fix — the scratch case you targeted is covered.

@teknium1 teknium1 closed this Jul 23, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

comp/cron Cron scheduler and job management needs-decision Awaiting maintainer decision before any implementation P3 Low — cosmetic, nice to have sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

kanban: worker-created scratch children inherit parent workspace path

4 participants