Skip to content

fix(kanban): isolate worker-created child workspaces - #67570

Closed
peacockesq wants to merge 2 commits into
NousResearch:mainfrom
peacockesq:fix/67567-kanban-scratch-child-isolation
Closed

peacockesq wants to merge 2 commits into
NousResearch:mainfrom
peacockesq:fix/67567-kanban-scratch-child-isolation

Conversation

@peacockesq

Copy link
Copy Markdown

Summary

  • Stop kanban_create from implicitly copying a running worker's literal workspace_kind / workspace_path into a child.
  • Keep omitted workspace args isolated (scratch, unmaterialized path) so dispatch resolves <board>/workspaces/<child-id>.
  • Preserve the original same-project intent safely: inherit only project_id, which resolves to a fresh per-task worktree and branch.
  • Preserve intentional sharing when callers explicitly pass workspace_kind=dir and workspace_path.
  • Surface the created task's resolved workspace kind/path/project in the tool response and created event.

Fixes #67567.

Root cause and RED proof

tools/kanban_tools.py::_handle_create treated omitted workspace fields as a request to copy the parent worker task's literal workspace fields. A materialized scratch review parent therefore produced a child row pointing at the same directory.

On unpatched main, the new focused matrix reproduced the defect:

3 failed, 2 passed
- scratch child inherited parent workspace_path
- default child inherited parent dir workspace
- nested scratch child inherited the root workspace_path

The scratch test also models a parent review-evidence file and a child write, proving the paths cannot alias after the fix.

Behavior after this change

Creation path Result
Worker tool, workspace omitted Fresh scratch task; workspace_path=NULL until dispatch
Nested worker tool children One deterministic scratch directory per task id
Explicit scratch Fresh isolated scratch task
Explicit dir + path Shared directory, unchanged and intentional
Project-linked parent, workspace omitted Same project; fresh child worktree path/branch
CLI/dashboard without worker context Existing fresh-scratch default unchanged
Existing rows No migration or rewrite

Verification

scripts/run_tests.sh -j 4 \
  tests/tools/test_kanban_tools.py \
  tests/hermes_cli/test_kanban_db.py \
  tests/hermes_cli/test_kanban_cli.py \
  tests/hermes_cli/test_kanban_decompose.py \
  tests/hermes_cli/test_kanban_decompose_db.py \
  tests/hermes_cli/test_kanban_goal_mode.py \
  tests/hermes_cli/test_kanban_project_link.py \
  tests/hermes_cli/test_kanban_reclaim_claim_lock_guard.py \
  tests/hermes_cli/test_kanban_dispatch_lock.py \
  tests/plugins/test_kanban_dashboard_plugin.py
# 552 passed, 0 failed

.venv/bin/ruff check .
# All checks passed!

.venv/bin/python scripts/check-windows-footguns.py --all
# No Windows footguns found (783 files scanned)

# CI-style scoped ruff + ty diff against upstream/main
# ruff: no new issues
# ty: no new issues (72 HEAD vs 75 base diagnostics in changed files)

git diff --check upstream/main...HEAD
# clean

git fsck --full --no-dangling
# clean

Exact implementation commit at PR creation: ce80a54436a56062c94defc86e4a6a515410237b (tree b3aaa09223d2cf494bb7ae59249ed68c59b22003).

Default kanban_create children now keep fresh scratch paths, while explicit dir sharing remains supported and project context resolves to a per-task worktree. Surface resolved workspace fields in create responses/events and cover scratch mutation, nesting, explicit sharing, and project inheritance.

Fixes NousResearch#67567
@teknium1

Copy link
Copy Markdown
Collaborator

Thanks for the focused isolation fix. The premise is verified against current main: tools/kanban_tools.py:1093-1136 copies a worker parent’s literal workspace_path whenever the child omits workspace fields. For a materialized scratch parent, hermes_cli/kanban_db.py:5866-5881 then resolves the child to that copied path rather than <board>/workspaces/<child-id>.

The change removes that literal workspace inheritance while retaining only project_id; hermes_cli/kanban_db.py:2482-2491 and :2618-2622 resolve project-linked children to their own per-task worktree path. Explicit directory sharing remains available through caller-supplied workspace fields. The added focused tests cover scratch isolation, nesting, explicit sharing, and project-linked children.

Automated hermes-sweeper review.

@teknium1 teknium1 added sweeper:risk-session-state Sweeper risk: may lose/corrupt/mis-associate session or context state 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 PR closes the whole issue, not just the reported scratch case.

Root cause / fix — correct. _handle_create no longer copies the running worker's literal workspace_kind/workspace_path; omitted workspace fields always resolve to a fresh scratch task (workspace_path unmaterialized), and the only thing inherited implicitly is project_id, which create_task turns into a fresh per-task worktree + branch. That matches every bullet in the issue's Expected behavior:

  • Omitted/default scratch → own <board>/workspaces/<child-id> ✔ (test_create_default_child_isolates_materialized_scratch_workspace, which also asserts the child can't write into the parent's review-evidence.txt)
  • Nested worker children each isolated ✔ (test_create_nested_default_scratch_children_each_get_own_workspace)
  • Persistent-dir sharing only via explicit workspace_kind=dir + workspace_path ✔ (test_create_default_child_does_not_implicitly_share_worker_dir / test_create_explicit_dir_workspace_shares_parent_path)
  • Project-linked child inherits project identity but materializes its own worktree/branch ✔ (test_create_default_child_inherits_project_without_reusing_worktree asserts workspace_path == repo/.worktrees/<child-id> and branch_name != parent.branch_name)

The created-event payload + tool response now surface workspace_kind/workspace_path/project_id (kanban_db.py + _ok(...)), which is a nice observability add for verifying isolation.

Overlap — consistent with the #61907 worktree-sibling lane (that PR left dir/scratch inheritance unchanged; this one removes the implicit literal-path inheritance the issue flagged) and orthogonal to #65953 (delegate_task).

One note vs the sibling PR #67574: that one special-cases only scratch and keeps implicitly inheriting the parent's literal path for dir/worktree parents — which the issue explicitly says should require explicit args (dir) or materialize a fresh worktree (project-linked). This PR handles those paths too, so it's the more complete fix. LGTM from my read.

@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
Contributor

This was generated by AI during triage.

Related to #67574. This patch has a broader default-child workspace and project/worktree contract, while #67574 preserves existing explicit inheritance behavior.

@teknium1

Copy link
Copy Markdown
Collaborator

Merged via #70143 — both of your commits were cherry-picked onto current main with your authorship preserved in git history (merge SHA 781968b). Thanks for the complete treatment: the isolation matrix tests, the explicit-sharing escape hatch, and the cross-profile project_source_task_id routing fallback all landed as-is.

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 sweeper:risk-session-state Sweeper risk: may lose/corrupt/mis-associate session or context state 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