Skip to content

fix(kanban): scratch decompose children must not inherit a claimed root's workspace_path - #103325

Open
chelsealong wants to merge 2 commits into
NousResearch:mainfrom
chelsealong:fix-kanban-scratch-decompose-workspace-inherit-103303
Open

chelsealong wants to merge 2 commits into
NousResearch:mainfrom
chelsealong:fix-kanban-scratch-decompose-workspace-inherit-103303

Conversation

@chelsealong

Copy link
Copy Markdown

Problem

decompose_triage_task() (hermes_cli/kanban_db.py, _insert_decomposed_child) deliberately leaves a worktree-kind child's workspace_path unset so dispatch materializes a distinct <repo>/.worktrees/<child-id> per sibling. scratch children got no such treatment: they fell into the child_ws_kind == root_ws_kind branch and inherited the root's literal workspace_path verbatim.

That's harmless while the root has never been claimed (workspace_path is NULL, so children get NULL too and resolve their own workspaces/<child-id> at dispatch). But once the root has been claimed at least once, the dispatcher has already stamped a concrete path onto it via set_workspace_path(). Decomposing that root pins every scratch sibling to the same directory, so concurrently dispatched workers all write into one shared dir instead of isolated per-task scratch dirs — silent cross-contamination with no error anywhere in the board state, diagnostics, or run logs.

Fixes #103303.

Fix

Route scratch through the same "leave workspace_path unset" branch already used for worktree:

elif child_ws_kind in ("worktree", "scratch"):
    child_ws_path = None
elif child_ws_kind == root_ws_kind:
    child_ws_path = root_row["workspace_path"]

dir-kind inheritance (the intentional shared-checkout case) is untouched. Per-child explicit workspace_path overrides still win, exactly as before.

Test

Added test_decompose_scratch_children_get_own_workspace in tests/hermes_cli/test_kanban_decompose_db.py, modeled on the existing worktree-isolation regression test: creates a scratch-kind triage root, stamps a concrete workspace_path onto it (simulating a prior claim), decomposes it, and asserts both children come back with workspace_path IS NULL.

Confirmed the test fails without the fix (git checkout HEAD~1 -- hermes_cli/kanban_db.py equivalent — actually verified by stashing the fix hunk before it was committed):

FAILED tests/hermes_cli/test_kanban_decompose_db.py::test_decompose_scratch_children_get_own_workspace
AssertionError: assert '/home/x/.hermes/workspaces/root' is None

And passes with the fix applied:

$ python -m pytest tests/hermes_cli/test_kanban_decompose_db.py tests/hermes_cli/test_kanban_worktree_isolation.py tests/hermes_cli/test_kanban_decompose.py -v
...
8 passed in 1.32s

ruff check hermes_cli/kanban_db.py tests/hermes_cli/test_kanban_decompose_db.py — all checks passed.

A broader -k kanban run shows 15 pre-existing failures (write-guard/hook-isolation/module-import issues unrelated to this change) — verified identical on unmodified upstream/main before this fix was applied, so they are not caused or affected by this patch.

AI assistance disclosure

This PR was prepared with AI assistance (Claude), with the diff reviewed and verified against the issue's repro before submission.

…ot's workspace_path

decompose_triage_task() left worktree children's workspace_path unset so
each materializes its own per-child dir, but scratch children fell into
the "kinds match -> inherit root_ws_path" branch. A scratch root that had
already been claimed (workspace_path stamped by set_workspace_path) then
pinned every decomposed sibling to that same directory, so concurrently
dispatched workers wrote into one shared dir instead of isolated
workspaces/<child-id> dirs. Unclaimed roots (workspace_path NULL) were
unaffected, which made the bug intermittent and easy to miss.

Route scratch through the same "leave it unset" branch as worktree so
dispatch always materializes workspaces/<child-id> per child. dir-kind
inheritance (the intended shared-checkout case) is unchanged.

Fixes NousResearch#103303
@alt-glitch alt-glitch added type/bug Something isn't working P3 Low — cosmetic, nice to have comp/cron Cron scheduler and job management labels Sep 5, 2026
@Enough1122

Copy link
Copy Markdown

AI code review — automated review for reference; please use your judgment.

Summary

Fixes decomposed scratch children inheriting a claimed root's workspace_path: scratch children now get None (per-child dir at dispatch), matching the existing worktree rule. Prevents concurrent siblings writing into one directory.

Findings (Non-blocking)

  • hermes_cli/kanban_db.py:25 — condition widened to ("worktree", "scratch") while explicit per-child workspace_path overrides still win (line 22 checked first). Correct precedence: override > fresh path > kind-matched inherit.
  • Edge: a dir-kind child of a scratch root still inherits the root path via the kind-match branch (line 27). That is pre-existing behavior for dir and presumably intended (non-concurrent or user-pinned). Just confirming the dir inherit path is deliberate and not the same aliasing bug.

@chelsealong

Copy link
Copy Markdown
Author

Re the dir-inherit question: confirmed deliberate, and it doesn't actually alias the way the finding describes. The kind-match branch (hermes_cli/kanban_db.py:3614) only fires when child_ws_kind == root_ws_kind, so a dir-kind child of a scratch-kind root never hits it — that pairing falls through to the else at line 3616-3617 and gets None, same as worktree/scratch. The shared-path inherit only happens for a genuine dir-root → dir-child pair, which is the intentional shared-checkout case (non-concurrent/user-pinned) noted in the docstring at lines 3599-3606. No aliasing bug there, and it's unaffected by this fix.

@chelsealong

Copy link
Copy Markdown
Author

Merged upstream/main (the fix's function had moved from kanban_db.py to kanban_db_graph.py in a refactor since this PR was opened). Re-applied the scratch-workspace fix at its new location and updated the test's import to match; tests/hermes_cli/test_kanban_decompose_db.py, test_kanban_worktree_isolation.py, and test_kanban_decompose.py all pass (11/11), and ruff check is clean. Confirmed the bug still reproduces on current main (unfixed code fails with workspace_path == '/home/x/.hermes/workspaces/root' instead of None), so this is not superseded.

@Enough1122

Copy link
Copy Markdown

Re-verified at 6522b2ec6. Re-applying at kanban_db_graph.py is the right call, and this is the one where I wanted to check the surrounding logic rather than just the marker — because the fix changes a condition, and a condition is where a re-application can be syntactically present but behaviourally different.

The re-applied condition is the right one:

    child_ws_kind = child.get("workspace_kind") or root_ws_kind
    if child.get("workspace_path"):
        child_ws_path = child.get("workspace_path")
-   elif child_ws_kind == "worktree":
+   elif child_ws_kind in ("worktree", "scratch"):
        child_ws_path = None
    elif child_ws_kind == root_ws_kind:
        child_ws_path = root_row["workspace_path"]

The ordering is what makes it work. The new branch has to sit before the child_ws_kind == root_ws_kind branch, because a scratch child of a scratch root would otherwise match that second branch and inherit the root's path — which is the bug. Putting it first means scratch never reaches the inheritance branch at all. Had it been appended after, the fix would have been inert for exactly the case it targets, and the in addition would have looked correct in review.

This is consistent with what you established earlier: a dir child of a scratch root already falls through to None via the else, so the remaining aliasing case was same-kind, and only worktree was covered before. scratch is the second instance of that shape.

The docstring was updated rather than left stale — it now says "worktree or scratch" and documents both materialization paths (<repo>/.worktrees/<child-id> and workspaces/<child-id>). That matters here more than usual, because the next reader of this function is going to be whoever handles a third workspace kind, and the failure mode of adding one is silently inheriting a shared directory.

Post-merge diff is +7/-5 in the moved file — 5 deletions being the replaced docstring paragraph — and the test import was updated to match the new module, which is the part a rebase usually misses. Your 11/11 across test_kanban_decompose_db.py, test_kanban_worktree_isolation.py, and test_kanban_decompose.py covers both the new case and the existing worktree isolation, and ruff check clean.

Nothing outstanding from me. If a workspace_kind is ever added, the branch that needs updating is this one, and the docstring now makes that visible.

I am not a maintainer and my review is not maintainer approval.

This branch has not been deployed

No deployments
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 P3 Low — cosmetic, nice to have type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

kanban: decompose_triage_task lets scratch siblings inherit the root's workspace_path, so concurrent workers share one directory

3 participants