Skip to content

Fix J detach/re-parent on an already-nested sub-coordinator (#814 follow-up)#816

Merged
anutron merged 1 commit into
masterfrom
argus/hera-detach-nested-bridge
Jun 26, 2026
Merged

Fix J detach/re-parent on an already-nested sub-coordinator (#814 follow-up)#816
anutron merged 1 commit into
masterfrom
argus/hera-detach-nested-bridge

Conversation

@anutron

@anutron anutron commented Jun 26, 2026

Copy link
Copy Markdown
Collaborator

Problem

PR #814 added the — Detach (make top-level) — sentinel to the J coordinator picker so an operator can un-nest a sub-coordinator. But it could not be triggered on the exact case it was built for: an already-nested sub-coordinator.

When coordinator C is nested under parent P (via ReparentCoordinator), the rail renders C as a worker "bridge" row in P — the bridging worker role IS C's coordinator (same multi-binding task) and no separate child orchestrator header is drawn (internal/tui/hera/rail.go bridge placement). So selecting the nested sub-coord yields a Selection whose Role.Kind == HeraKindWorker. heraCoordReparentTarget only returned ok for a coordinator-kind role row or a coordinator's own orchestrator header — it returned false for a worker row, so heraOpenAdopt fell through to the statusbar error "J: select a freelancer or a coordinator to adopt".

The asymmetry: you could nest a top-level coordinator (selected as its orchestrator header) but could not un-nest it (now a headerless worker-bridge row).

Fix

heraCoordReparentTarget now qualifies a third shape: a non-archived worker-kind role whose Selection.BridgeChildOrchID != 0 — the same field the Ctrl+D cascade already reads, stamped by Rail.Selection when the cursor rests on a bridge row. That worker row IS the bridged child's coordinator, so it resolves the child orchestrator id and routes the existing heraAdoptCoordinator path:

  • Detach (the must-fix): the detach sentinel → DetachCoordinator(childOrchID).
  • Re-parent (symmetry): picking a different parent → ReparentCoordinator against the child orch; cycles still rejected authoritatively (ReparentCoordinator rejects nesting under self/descendant).

Guards preserved:

  • Only a bridge row qualifies (BridgeChildOrchID != 0) — a plain (non-bridging) worker is never misclassified and still surfaces the "select a freelancer or coordinator" feedback.
  • Inert in remote mode (heraAdoptOps == nil).
  • The coord-task hint uses the structural bridge task (roleReclaimTaskBridgeTaskID else live TaskID).

No op-layer changeDetachCoordinator/ReparentCoordinator already resolve everything from the child orchestrator id alone. No new keybinding; help modal / README unchanged.

Tests

  • TestHeraCoordReparentTarget — bridge-worker row qualifies (returns the child orch id, bridge role name, bridge task); prefers BridgeTaskID; plain worker (==0) and archived bridge row do not qualify.
  • TestSmoke_HeraDetachNestedBridgeThroughPicker — select the bridge-row shape, J, Enter on the detach sentinel → parent link gone (child top-level again), child's own coord binding intact.
  • TestSmoke_HeraReparentNestedBridgeThroughPicker — re-parent the nested coord under a different parent through the picker.
  • Existing coordinator-header / coordinator-role detach + re-parent tests unchanged (no regression).

OpenSpec

fix-detach-nested-bridge (archived in-PR): the hera-view J requirement now defines a coordinator selection to include a worker-bridge sub-coordinator row; delta merged into the base spec, folder moved to openspec/changes/archive/2026-06-25-fix-detach-nested-bridge/. openspec validate --all --strict passes (47/47).

make pre-pr

  • build ✓ · vet ✓ · fmt-check ✓ · lint-pr ✓ (0 issues) · test-cover-gate ✓ (89.5% ≥ 88 floor, -race green)
  • vuln reports 2 stdlib-only CVEs (net/textproto, crypto/x509 in go1.26.3) — advisory, matching CI's continue-on-error for the vuln step; no first-party code affected.

🤖 Generated with Claude Code

Co-Authored-By: Claude noreply@anthropic.com

…low-up)

#814 added "Detach (make top-level)" to the J coordinator picker, but it could
not be triggered on the exact case it was built for: an already-nested
sub-coordinator. A nested sub-coordinator renders as a headerless worker
"bridge" row in its parent (Role.Kind == worker), so heraCoordReparentTarget
returned false for it and heraOpenAdopt fell through to the "select a
freelancer or a coordinator" statusbar error. You could NEST a top-level coord
(selected as its orch header) but not UN-NEST it.

Fix: heraCoordReparentTarget now qualifies a THIRD shape — a non-archived
worker row whose Selection.BridgeChildOrchID is non-zero (the SAME field the
Ctrl+D cascade already reads). The rail stamps the bridged CHILD orchestrator
id there; that worker row IS the child's coordinator, so it routes the existing
heraAdoptCoordinator path (detach sentinel + DetachCoordinator, and
ReparentCoordinator for symmetry) against the CHILD orch. Only a bridge row
qualifies (BridgeChildOrchID != 0) — a plain worker is never misclassified, and
cycles are still rejected by ReparentCoordinator. Inert in remote mode.

No op-layer change (DetachCoordinator/ReparentCoordinator already resolve
everything from the child orch id). No new keybinding; help modal / README
unchanged.

OpenSpec: fix-detach-nested-bridge (archived in-PR; hera-view delta merged into
the base spec). Tests: TestHeraCoordReparentTarget bridge subtests +
TestSmoke_HeraDetachNestedBridgeThroughPicker /
TestSmoke_HeraReparentNestedBridgeThroughPicker. Gotcha added to hera-view.md.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>
@github-actions

Copy link
Copy Markdown

Merging this branch will decrease overall coverage

Impacted Packages Coverage Δ 🤖
github.com/drn/argus/internal/tui 83.98% (-0.03%) 👎

Coverage by file

Changed files (no unit tests)

Changed File Coverage Δ Total Covered Missed 🤖
github.com/drn/argus/internal/tui/heraactions.go 77.73% (+0.10%) 440 (+2) 342 (+2) 98 👍

Please note that the "Total", "Covered", and "Missed" counts above refer to code statements instead of lines of code. The value in brackets refers to the test coverage of that file in the old version of the code.

Changed unit test files

  • github.com/drn/argus/internal/tui/heraactions_test.go

@anutron anutron merged commit cc86cbc into master Jun 26, 2026
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant