Skip to content

fix(kanban): harden orchestration and add report-only workspace lifecycle - #85269

Open
ahmadashfq wants to merge 11 commits into
NousResearch:mainfrom
ahmadashfq:feat/workspace-registry-report-v2
Open

fix(kanban): harden orchestration and add report-only workspace lifecycle#85269
ahmadashfq wants to merge 11 commits into
NousResearch:mainfrom
ahmadashfq:feat/workspace-registry-report-v2

Conversation

@ahmadashfq

Copy link
Copy Markdown
Contributor

Summary

This PR closes the Hermes Kanban failure chain where recovery/escalation triage could be mistaken for fresh intake, existing graphs could be duplicated, and non-code children could inherit code worktrees. It also introduces a deliberately report-only workspace lifecycle registry and closeout manifest surface.

Cut 1: safe orchestration boundaries

  • persist and enforce fresh-intake vs recovery provenance in the kernel
  • fail closed for legacy/unclassified and escalated triage
  • reject duplicate open descendant graphs transactionally
  • allocate workspaces by declared capability; only repo-write children receive isolated worktrees
  • require real installed profiles at graph commit
  • serialize graph commits against profile delete/rename using a shared lifecycle lock
  • refuse delete/rename while profiles own nonterminal Kanban assignments

Report-only Cut 2

  • add an evidence-bound SQLite workspace registry, leases, observations, classification, and closeout manifests
  • make receipt validation, freshness checks, and insertion one write transaction
  • expose only inventory, classify, dry-run import, and manifest commands
  • keep apply/removal authority disabled; manifests always report apply_available=false

Safety boundaries

  • no automatic worktree or branch deletion
  • no apply/removal command
  • registry is not yet wired into live dispatcher allocation
  • legacy/unclassified triage and escalated triage cannot be decomposed in place; operators must retire and recreate a reviewed fresh-intake root
  • unknown or stale evidence fails closed to retention/review

Verification

Exact commit: 4188f08

  • focused local suite: 132 passed, 2 skipped
  • independent Hermes QA: 122 passed, 2 skipped
  • independent Challenger Lab archive run: 124 passed, 2 skipped
  • full CLI + gateway suite: 10,701 passed, 4 failed, 108 skipped
  • the 4 failures are unrelated baseline/environment failures previously reproduced on origin/main:
    • shutdown forensics subprocess spawn on this macOS host
    • Linux abstract systemd socket on macOS
    • ambient Qwen OAuth provider resolution
    • Darwin special mode-bit expectation
  • git diff --check clean
  • commit-bound approvals: Hermes Reviewer, Hermes QA, Sentinel, Challenger Lab

Operational rollout

Keep automatic decomposition and child auto-promotion disabled in production until this PR is merged, deployed, and the disposable-board/canary gates pass. This PR does not change live configuration.

DavidMetcalfe and others added 11 commits August 13, 2026 15:17
…esearch#79738)

The unblock-loop breaker routes a repeatedly same-cause-blocked task to
'triage' (block_loop_detected) as a human-in-the-loop gate. The gateway
auto-decomposer could not distinguish those cards from fresh triage, so
on its next tick it re-specified them (rewriting title/body), promoted
them to ready, and left them there under the active_pr respawn guard —
non-terminal for external observers (NousResearch#79738, NousResearch#79728).

- list_triage_ids(): exclude block-loop-escalated cards (block_loop_detected
  event with no newer triage_escalation_recovered)
- decompose_task(): refuse escalated cards for the auto-decomposer; an
  explicit manual call is the operator's decision — acknowledge (audited)
  and proceed (NousResearch#79728)
- recover_escalated_triage_task(): operator recovery — clears block_kind,
  resets block_recurrences (fresh loop budget), appends audited event
- hermes kanban unblock --recover-escalated exposes the recovery path

Tests: 8 new covering the reporter's exact chain (kind=None review block),
typed escalation, operator recovery, fresh-triage regression, and manual
decompose of an escalated card.

Co-authored-by: Hafiz Ahmad Ashfaq <28647270+ahmadashfq@users.noreply.github.com>
Signed-off-by: Hafiz Ahmad Ashfaq <28647270+ahmadashfq@users.noreply.github.com>
…mpose exclusion

Covers the block_loop_detected / triage_escalation_recovered events, the
fresh-vs-escalated triage distinction, the --recover-escalated operator
action, and the manual-decompose acknowledgment path (NousResearch#79738).

Co-authored-by: Hafiz Ahmad Ashfaq <28647270+ahmadashfq@users.noreply.github.com>
Signed-off-by: Hafiz Ahmad Ashfaq <28647270+ahmadashfq@users.noreply.github.com>
A manual decompose on an escalated card acknowledged the escalation
(recover_escalated_triage_task) before the decomposition ran. If the
attempt then failed (aux client, LLM error, malformed JSON, empty
spec), the card stayed in triage with the escalation cleared: the
auto-decompose feed re-included it and the next dispatcher tick
re-specified it — the exact automation loop this PR removes.

Recovery now happens only after specify_triage_task /
decompose_triage_task succeed (both fanout branches), so a failed
manual attempt leaves the card escalated and out of the auto-decompose
feed, and the triage_escalation_recovered event records an outcome,
not an attempt. The recovery predicate no longer requires
status='triage' because a successful decompose transitions the card
out of triage before the acknowledgment is written; the escalation
event predicate remains the gate.

Tests: new failure-path regression (failed manual decompose keeps the
card escalated, excluded from the feed, still refused by the
auto-decomposer, no recovery event) and a fanout=true success-path
variant.

Co-authored-by: Hafiz Ahmad Ashfaq <28647270+ahmadashfq@users.noreply.github.com>
Signed-off-by: Hafiz Ahmad Ashfaq <28647270+ahmadashfq@users.noreply.github.com>
Persist fresh-intake authority, keep legacy triage out of automatic decomposition, and atomically couple escalation recovery with graph/spec writes. Reject duplicate open graphs after the decomposer call and honor the manual promotion gate for single-task specifications.

Co-authored-by: Hafiz Ahmad Ashfaq <28647270+ahmadashfq@users.noreply.github.com>

Signed-off-by: Hafiz Ahmad Ashfaq <28647270+ahmadashfq@users.noreply.github.com>
Require the decomposer to declare whether each child needs repository writes. Only repo_write children receive isolated worktrees; missing or invalid policy fails closed to scratch, preventing non-code lanes from inheriting a root checkout.

Co-authored-by: Hafiz Ahmad Ashfaq <28647270+ahmadashfq@users.noreply.github.com>

Signed-off-by: Hafiz Ahmad Ashfaq <28647270+ahmadashfq@users.noreply.github.com>
Add a non-mutating workspace lifecycle authority with SQLite recovery, immutable receipts, exact 54-path baseline comparison, and report-only CLI controls. V1 does not grant removal authority or invoke worktree removal.

Co-authored-by: Hafiz Ahmad Ashfaq <28647270+ahmadashfq@users.noreply.github.com>
Signed-off-by: Hafiz Ahmad Ashfaq <28647270+ahmadashfq@users.noreply.github.com>
Implement host-owned registry leases, explicit safe reconciliation, immutable receipts, and backup recovery while retaining V1 removal prohibition.

Co-authored-by: Hafiz Ahmad Ashfaq <28647270+ahmadashfq@users.noreply.github.com>
Signed-off-by: Hafiz Ahmad Ashfaq <28647270+ahmadashfq@users.noreply.github.com>
Remove the incident-specific 54-worktree baseline, bind review packets to exact observed paths and evidence hashes, refuse unmanaged path collisions, and keep V1 apply/removal authority unavailable.

Co-authored-by: Hafiz Ahmad Ashfaq <28647270+ahmadashfq@users.noreply.github.com>
Signed-off-by: Hafiz Ahmad Ashfaq <28647270+ahmadashfq@users.noreply.github.com>
Make read-only Git inventory deterministic across Windows locale code pages while retaining undecodable bytes as replacement characters for fail-closed classification.

Co-authored-by: Hafiz Ahmad Ashfaq <28647270+ahmadashfq@users.noreply.github.com>
Signed-off-by: Hafiz Ahmad Ashfaq <28647270+ahmadashfq@users.noreply.github.com>
Co-authored-by: Hafiz Ahmad Ashfaq <28647270+ahmadashfq@users.noreply.github.com>
Signed-off-by: Hafiz Ahmad Ashfaq <28647270+ahmadashfq@users.noreply.github.com>
Co-authored-by: Hafiz Ahmad Ashfaq <28647270+ahmadashfq@users.noreply.github.com>
Signed-off-by: Hafiz Ahmad Ashfaq <28647270+ahmadashfq@users.noreply.github.com>
@alt-glitch alt-glitch added type/feature New feature or request P3 Low — cosmetic, nice to have comp/cron Cron scheduler and job management comp/cli CLI entry point, hermes_cli/, setup wizard comp/gateway Gateway runner, session dispatch, delivery labels Aug 13, 2026
@enzo-adami

Copy link
Copy Markdown
Contributor

I reproduced a missing-writer race in the profile lifecycle authority on this HEAD (4188f082c7). Delete/rename and decomposition/specification take profile_lifecycle_lock(), but ordinary graph writers do not: create_task() and assignment/reassignment paths neither acquire that lock nor revalidate profile existence.

Deterministic probe with an isolated DB: hold profile_lifecycle_lock() in one thread, then call the real create_task(..., assignee='victim') from another connection/thread. The writer completed in 10.2 ms while the lifecycle lock was still held, and the nonterminal assignment persisted:

writer_completed_while_lock_held=True
persisted_assignee=victim

This falsifies the deletion comment that a concurrent graph commit must either precede the assignment check or observe the identity missing. The actual interleaving can be:

  1. delete holds lock and observes no assignments;
  2. create_task inserts an open assignment without taking the lock;
  3. delete tombstones/removes the profile;
  4. the new card is stranded on the deleted assignee.

Suggested fix: every mutation that can introduce/change a nonterminal assignee (create_task, assign/reassign, and any direct SQL equivalents) must take the same lifecycle lock before its SQLite write transaction and validate the target profile while holding it. Keep the global order profile lifecycle lock -> board write_txn, which is already used by decomposition, and add a two-connection regression that blocks the writer until delete/rename has completed (then rejects the missing profile) rather than merely testing the protected decomposition path.

@enzo-adami

Copy link
Copy Markdown
Contributor

I turned the reproduced create_task(assignee=...) vs profile-delete TOCTOU into a narrow draft extension on your exact 4188f082c7954154f5c275d17545c3780839a311 HEAD: ahmadashfq#4

The extension preserves legacy never-materialized assignees, serializes assigned task insertion with the lifecycle authority, persists a path-safe retirement fence before rename, and preserves lock order for nested swarm graph creation. Deterministic RED on the author HEAD: the writer completed during the post-check delete pause; GREEN rejects it after waiting, leaves zero stranded cards, and permits a recreated live profile. Focused cluster: 156 passed / 4 skipped; Ruff, changed-production Windows footgun scan, and diff-check pass. It does not merge or copy #86609/#86610.

@Enough1122

Copy link
Copy Markdown
Contributor

AI code review — automated review for reference, author can ignore or act on any point.

fix(kanban): harden orchestration and add report-only workspace lifecycle

The fail-closed direction is right: block-loop-escalated triage is excluded from auto-decomposition via durable markers, assignees are revalidated inside the graph commit under a cross-process lifecycle lock, and the workspace registry refuses every removal path. Observations:

  1. Scope/footprint: the PR bundles three largely independent concerns — block-loop escalation hardening, profile delete/rename lifecycle locking, and a ~1,300-line report-only workspace lifecycle registry (hermes_cli/workspace_lifecycle.py + subcommands/workspace.py). The registry is explicitly not wired into the live dispatcher ("report-only evidence authority"), and most of the Registry machinery (reserve/create_or_get/materialization_result/receipt/held_lease/reconcile_preparing) has no runtime caller outside tests. Consider splitting the workspace-lifecycle portion into its own PR so the security hardening can merge on its own merits and the unwired surface gets separate review.

  2. hermes_cli/kanban_decompose.py list_triage_ids now only returns auto-decomposable cards, and decompose_task(<legacy-id>) hard-fails with "provenance is unclassified" even for a manual CLI invocation. Deliberate fail-closed behavior, but it is a user-facing change for existing boards: every pre-upgrade triage card becomes undecomposable by any path (the documented remedy is retire + recreate). Confirm the manual hermes kanban decompose <id> path and the dashboard "⚗ Decompose" button on legacy cards both surface this failure clearly instead of appearing to do nothing.

  3. hermes_cli/kanban_db.py decompose_triage_task: requiring root_assignee and every child assignee to be non-None at the DB layer is a good fail-closed gate, but it changes the contract for non-LLM callers that previously relied on the default-assignee fallback. Confirm the only caller (decompose_task) always resolves a concrete orchestrator profile (tests do), and update the docstring to state the new requirement so direct callers aren't surprised by a ValueError inside the write txn.

  4. hermes_cli/profile_lifecycle.py: the Windows path pre-writes a space into the lock file when it is empty/missing so msvcrt.locking has a byte to lock. The "write if empty" check is a TOCTOU race (two processes can both observe empty and write — benign since content is ignored, but the reasoning is subtle). Consider writing the byte unconditionally before opening to simplify the race reasoning.

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 comp/cron Cron scheduler and job management comp/gateway Gateway runner, session dispatch, delivery P3 Low — cosmetic, nice to have type/feature New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants