Skip to content

fix(kanban): propagate deferred scratch-workspace sweep to grandparents - #41829

Open
r266-tech wants to merge 1 commit into
NousResearch:mainfrom
r266-tech:fix-kanban-recursive-scratch-cleanup
Open

fix(kanban): propagate deferred scratch-workspace sweep to grandparents#41829
r266-tech wants to merge 1 commit into
NousResearch:mainfrom
r266-tech:fix-kanban-recursive-scratch-cleanup

Conversation

@r266-tech

Copy link
Copy Markdown
Contributor

Follow-up to #41352 (deferred parent scratch-workspace cleanup).

_try_cleanup_parent_workspaces() sweeps a deferred parent scratch dir once all its children are terminal, but the sweep was non-recursive. In a depth-3+ scratch decomposition chain A->B->C, B's completion defers (C still active), and when leaf C completes it sweeps only its direct parent B and stops — A's scratch dir leaks on disk forever, with no global workspace GC to reclaim it (the only rmtree sites are own-scratch and direct-parent). It accumulates across runs.

Fix: after handling each eligible parent, recurse _try_cleanup_parent_workspaces(conn, parent_id, _visited) so the sweep cascades up the whole chain. The task DAG is acyclic (_would_cycle), and a threaded _visited set bounds each ancestor to one evaluation per completion cascade so diamond-shaped DAGs aren't re-traversed per path. The optional _visited keeps the existing 2-arg callers unchanged.

Adds depth-3 chain and diamond-DAG regression tests (existing tests only covered 2-level chains).

If this already landed via a separate patch, feel free to close.

@alt-glitch alt-glitch added type/bug Something isn't working P3 Low — cosmetic, nice to have comp/cli CLI entry point, hermes_cli/, setup wizard labels Jun 8, 2026
@teknium1

Copy link
Copy Markdown
Contributor

Thanks for the focused follow-up. The premise remains valid on current main: hermes_cli/kanban_db.py:4545 evaluates only direct parents, while _cleanup_workspace() defers deletion for active children at hermes_cli/kanban_db.py:4499-4515. The existing regression coverage at tests/hermes_cli/test_kanban_db.py:2600-2678 covers only the one-level case.

The proposed recursive traversal is bounded by _visited, follows the existing terminal-child predicate, and the depth-3 plus diamond tests directly exercise the missing cases. GitHub reports the PR as mergeable against current main; salvage should be mechanical despite unrelated movement in these files.

Automated hermes-sweeper review.

@teknium1 teknium1 added sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades sweeper:blast-contained Sweeper blast radius: contained — one narrow path / opt-in / few users labels Jul 14, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

comp/cli CLI entry point, hermes_cli/, setup wizard P3 Low — cosmetic, nice to have sweeper:blast-contained Sweeper blast radius: contained — one narrow path / opt-in / few users 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.

3 participants