Harden kanban create: strip workspace scheme prefix before storing workspace_path - #47
Conversation
…rkspace_path A worktree:<path> / dir:<path> scheme prefix could leak into the stored workspace_path instead of being split into workspace_kind + a bare absolute path. Root-caused on t_a7bfc729, whose workspace_path was the literal 'worktree:/Users/testuser/projects/CPE-research/research-agent'. At spawn time resolve_workspace judged the value non-absolute, the spawn failed, and after 2 consecutive spawn_failed the dispatcher circuit breaker blocked the task. The CLI _parse_workspace_flag already splits the scheme correctly, but callers that bypass it (kanban model tool, dashboard, hand-written creates) pass workspace_kind and workspace_path separately and can jam the whole worktree:<path> string into workspace_path. Fix is centralized at the DB layer: - New _strip_workspace_scheme() + _WORKSPACE_SCHEME_RE: strips a leading scratch:/worktree:/dir: scheme. Scheme promotes kind only when kind is unset/default scratch; an explicit non-default kind is preserved. - create_task store-time guard: normalize before workspace_kind validation so the prefix never reaches the DB regardless of create surface. - resolve_workspace spawn-time self-heal: normalize at entry so an already persisted malformed value recovers instead of tripping the circuit breaker. - 4 regression tests incl. the exact failure case. Patch note: ~/.hermes/plans/hermes-patches/kanban-workspace-scheme-prefix.md Task: t_e3939f44
🔎 Lint report:
|
| Rule | Count |
|---|---|
PLW1514 |
1 |
First entries
gateway/run.py:5594: [PLW1514] `open` in text mode without explicit `encoding` argument
✅ Fixed issues (1):
| Rule | Count |
|---|---|
PLW1514 |
1 |
First entries
../../../../../tmp/lint-base/gateway/run.py:5594: [PLW1514] `open` in text mode without explicit `encoding` argument
Unchanged: 0 pre-existing issues carried over.
ty (type checker)
Total: 11585 on HEAD, 11572 on base (🆕 +13)
🆕 New issues (2):
| Rule | Count |
|---|---|
unresolved-attribute |
2 |
First entries
tests/hermes_cli/test_kanban_db.py:280: [unresolved-attribute] unresolved-attribute: Attribute `workspace_kind` is not defined on `None` in union `Task | None`
tests/hermes_cli/test_kanban_db.py:281: [unresolved-attribute] unresolved-attribute: Attribute `workspace_path` is not defined on `None` in union `Task | None`
✅ Fixed issues: none
Unchanged: 6093 pre-existing issues carried over.
Diagnostics are surfaced as warnings — this check never fails the build.
There was a problem hiding this comment.
Code Review
This pull request introduces a self-healing mechanism to strip scheme prefixes (such as worktree:, dir:, or scratch:) from workspace_path and correctly assign the workspace_kind during task creation and resolution. The review feedback highlights a critical bug where the spawn-time self-heal in resolve_workspace only updates a local reference and fails to persist the corrected values to the database. Additionally, the reviewer suggests simplifying the regex extraction logic and adding a unit test to verify the persistence of the self-healed values.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 36085f8bb8
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Gemini HIGH on PR #47: resolve_workspace's spawn-time self-heal called replace(task, ...) which only mutated a local copy. A task persisted with workspace_kind='scratch' + workspace_path='worktree:/abs' (the t_a7bfc729 failure) thus kept 'scratch' in the DB, so _dispatch_once_locked's `if claimed.workspace_kind == 'worktree'` stayed False -> set_branch_name was never called and _maybe_emit_scratch_tip emitted the wrong tip. - _dispatch_once_locked now heals + PERSISTS (UPDATE) right after claim, so the spawned task object and the DB row both carry the promoted kind/path. - _strip_workspace_scheme uses m.group(1) instead of re-splitting (Gemini MEDIUM). - create_task assigns the healed kind to a fresh local to clear a ty invalid-assignment (str|None -> str). - New regression test test_dispatch_self_heals_persisted_scheme_prefix: fails before (spawned/DB stay 'scratch'), passes after. - assert-not-None on test_create_bare_path_unchanged to clear ty diff.
A persisted row with workspace_kind='worktree' AND a 'worktree:' prefix in workspace_path must strip the prefix and keep the kind, so the claim-time heal routes it through worktree materialization with a bare absolute path instead of tripping the absolute-path check. The claim-time heal added in the prior commit already covers this (it runs before the worktree dispatch branch); this locks the behavior in.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 88fb339770
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
Babysit update — review fixes landed (HEAD 88fb339), all 5 threads resolved. Remaining CI red is pre-existing on base
The PR's own change is green and merge-ready. The base branch is red. Holding for a maintainer call: merge despite known-base-red, or reconcile |
The ready-queue claim path in _dispatch_once_locked strips a leaked <scheme>:<path> prefix off a just-claimed task and persists the correction before branching on workspace_kind; the review-queue claim path (after claim_review_task) did not, so a legacy review-status task with a prefixed workspace_path bypassed the heal, failed the worktree absolute-path check, and could trip the spawn-failure circuit breaker. Extract the heal-and-persist block into _heal_claimed_workspace() and call it in both claim paths. Add a fails-before/passes-after regression test for the review path.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 351164f08b
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
…ldren (Codex P2) resolve_workspace(conn=...) now persists the promoted (kind, path) so the manual 'hermes kanban claim' path no longer leaves a stale workspace_kind. decompose_triage_task strips the <scheme>:<path> prefix on child overrides before its direct INSERT, matching create_task's guard. Patch note: ~/.hermes/plans/hermes-patches/kanban-claim-decompose-scheme-heal.md
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d39baab54d
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| child_ws_kind, child_ws_path = _strip_workspace_scheme( | ||
| child_ws_kind, child_ws_path | ||
| ) |
There was a problem hiding this comment.
Honor child workspace schemes before inherited kinds
Fresh evidence beyond the earlier decompose-normalization comment is the non-scratch root case: when a triage root is dir/worktree and a child override supplies only workspace_path='worktree:/repo' (or dir:/repo), child_ws_kind has already inherited the root kind, so _strip_workspace_scheme treats that inherited non-default kind as explicit and refuses to promote the scheme. The child is stored as the root kind plus the bare path (for example dir + /repo) instead of the requested scheme, so dispatch can run in the repo directory rather than creating the linked worktree. Distinguish an explicit child workspace_kind from the inherited fallback before normalizing.
Useful? React with 👍 / 👎.
An opener is not always titleable — an image with no caption, a compaction handoff, a bare slash command — and those sessions stayed unnamed for life, because the guard that stops re-titling a named session also stopped the nameless one from ever asking again. Let a later turn name a session that still has no title. The derived title also ran the collision dedupe inline on the turn. It is a slice of the user's own words, so it collides constantly — people open sessions with "hi" — and resolving "hi #47" is a widening scan on the critical path for a name the model replaces a second later. Decline it there and let the background stage, which can afford the scan, pick it up.
Summary
Harden kanban task creation so a
worktree:/dir:/scratch:scheme prefix can never leak into the storedworkspace_path. Root-cause fix for the dispatcher circuit-breaker blocks that hit live-board tasks (t_a7bfc729, and the same class on t_c3bca12e / t_56abd192) today.Root cause
t_a7bfc729 stored the literal
worktree:/Users/testuser/projects/CPE-research/research-agentinworkspace_path. At spawn timeresolve_workspace's absolute-path check judged it non-absolute, the spawn failed, and after 2 consecutivespawn_failedthe dispatcher circuit breaker blocked the task.The CLI
_parse_workspace_flagalready splits the scheme correctly. The bug is callers that bypass it (the kanban model tool, the dashboard, hand-written creates) passingworkspace_kindandworkspace_pathseparately and jamming the wholeworktree:<path>string intoworkspace_path. Fix is centralized at the DB layer so every create surface is covered.Changes (
hermes_cli/kanban_db.py)_strip_workspace_scheme(kind, path)+_WORKSPACE_SCHEME_RE: strips a leadingscratch:/worktree:/dir:scheme. When a scheme is present andkindis unset/defaultscratch, the scheme's kind wins; an explicit non-default kind is preserved (only the prefix is stripped).create_taskstore-time guard: normalize beforeworkspace_kindvalidation so the prefix never reaches the DB regardless of create surface.resolve_workspacespawn-time self-heal: normalize at entry so an already-persisted malformed row recovers instead of tripping the circuit breaker.Tests (
tests/hermes_cli/test_kanban_db.py)4 regression tests added:
test_create_strips_worktree_scheme_from_workspace_path(the exact failure case:workspace_path='worktree:/abs'-> kind=worktree + bare absolute path)test_create_strips_dir_scheme_from_workspace_pathtest_create_explicit_kind_not_overridden_by_stray_prefixtest_create_bare_path_unchanged(healthy bare path passes through untouched)Validation
pytest tests/hermes_cli/test_kanban_db.py -k "scheme or workspace or bare_path"-> 23 passedpytest test_kanban_db.py test_kanban_cli.py test_kanban_core_functionality.py-> 440 passed, 1 skippedworktree:<real-repo>now materializes the worktree instead of raising.Residual risk
Low. Pure additive normalizer; bare paths pass through untouched. No schema change, no migration of existing rows (spawn-time self-heal recovers legacy malformed rows on next claim).
Patch note:
~/.hermes/plans/hermes-patches/kanban-workspace-scheme-prefix.mdTask: t_e3939f44