spec-kitty / spec-kitty/spec-kitty

Coord-topology implement-review loop friction: move-task recovery cascade, sync-daemon hang, and opaque gate errors

Open
#2,555 2 comments 0 reactions 0 assignees View on GitHub
priority:P2 reliability usability workflow
Dominant language
Python
Stars
1.6k
Forks
165
Avg merge
13h 53m
Merged PRs (30d)
336

Description

**Parent:** #2017 (workflow guards lacking depth or blocking legitimate in-flight actions — umbrella)
**Epic:** #2160 (Coord topology: unify artifact authority for task/status surfaces during the implement/review loop)

## Summary

Running mission `relocation-hardened-dead-code-scanners-01KX958P` (#2546, coord-topology, 5 WPs) surfaced five distinct friction points in the implement-review loop that together make per-WP orchestration overhead multiply across a mission rather than stay flat. None of these are individually catastrophic, but stacked they make the loop barely navigable without deep internals knowledge — a 0-code-diff "preserve" WP took **six** `move-task` attempts to land. Recording each occurrence below for triage; not prescribing which land in which existing child vs. a new one.

## 1. `move-task` on coord-topology lanes requires a brittle multi-step recovery

Moving a WP to `for_review` cascades through blockers:

a. Untracked planning artifacts (`analysis-report.md`, status batches) on the **primary** repo block the move ("Using planning repo's kitty-specs; commit these first").
b. The lane's pre-commit guard then **blocks** committing `kitty-specs/` on the lane branch ("Protected path: ... implementation branches must not modify kitty-specs/").
c. Committed WP-prompt/status divergence on the lane has to be reverted via:
```
git restore --source kitty/mission- --staged --worktree -- kitty-specs/
```
followed by a (guard-blocked) commit, after which `move-task` finally proceeds from the **planning repo**.

This is the same catch-22 family as #2017's Class A3/A4 occurrences (kitty-specs-on-lane guard vs. the gate that demands kitty-specs be present/clean), but the concrete recovery sequence for this coord-topology mission is new and worth recording as a fresh witness: a WP with **zero code diff** still needed six `move-task` attempts to reach `for_review`.

## 2. `move-task` hangs on the sync-daemon fan-out

`move-task` intermittently hangs (exit 124 against a 120s timeout) on the sync-daemon fan-out, even with `SPEC_KITTY_SYNC_MINIMAL_IMPORT=1` set. Workaround: background the call (`&`) or set `SPEC_KITTY_SYNC_DISABLE=1`; the lane transition itself has already been written by the time the hang occurs, so the write is not lost — but there is no built-in timeout or clear failure surfaced to the caller, so a foregrounded run just sits.

## 3. Bulk-edit false-positive blocks `agent action implement` claim on a non-bulk WP

A spec containing ordinary verbs like "rename/refactor/update/change" scored 5/4 on the bulk-edit heuristic and refused the claim until `--acknowledge-not-bulk-edit` was passed — for a WP that was a plain mechanism refactor (`change_mode: normal`, not a bulk occurrence-driven edit). This is a different mechanism from #2229 (diff-compliance gate over-blocking at review-claim time by file-dominant-category) — this fires earlier, at `agent action implement` claim time, off spec prose alone.

## 4. Dispatched implementer/reviewer subagents stall on the ~9-minute arch suite

Subagents dispatched to implement/review a WP background the architectural test suite and then yield ("waiting for the monitor notification") without committing, marking status, or calling `move-task` — leaving the orchestrator to take over the finish sequence (validate committed HEAD → mark-status → clean lane `kitty-specs/` → `move-task`). This was consistent across every WP in this mission, i.e. not an isolated flake — the background-and-wait pattern the subagent adopts for a long-running gate leaves the loop dangling every time.

## 5. Non-canonical issue-matrix column silently blocks ALL WP approvals

A stray `Notes` column in `issue-matrix.md` (outside the canonical closed set: `issue`/`verdict`/`evidence_ref` + optional `title`/`scope`/`wp`/`fr`/`nfr`/`sc`/`repo`) made **every** `move-task --to approved` fail with a cascading "Missing rows" error. The error did not name the real cause (column drift); it only surfaced by reproducing the validator directly (`src/specify_cli/cli/commands/review/_issue_matrix.py::validate_issue_matrix`). This is adjacent to #1738 (issue-matrix completeness scans only `spec.md`) but distinct: here the matrix was complete, just schema-drifted, and the failure mode was a misleading downstream symptom rather than a missing-row detection gap.

## Impact

Per-WP orchestration overhead from #1/#2/#4 compounds across every WP in a coord-topology mission; #3 and #5 are one-time-per-mission but opaque enough to cost significant investigation time (#5 required reading validator source to diagnose). Combined, these made a 5-WP mission's implement-review loop require substantially more manual intervention than the loop's steady-state design implies.

## Suggested directions

- [ ] Add a timeout and a clear failure message to the sync-daemon fan-out invoked from `move-task`, instead of a silent hang past the caller's timeout.
- [ ] Give `move-task` a single self-healing recovery path for lane `kitty-specs/` divergence (today's fix is a 3-step manual `git restore` + guard-blocked commit dance) — same root class as #2017's A3/A4.
- [ ] Make gate/validator errors name the offending column or path directly (e.g. `validate_issue_matrix`'s "Missing rows" error should name the non-canonical column, not just report rows as missing) — same spirit as the #1738/#1742 issue-matrix completeness work, applied to schema-drift diagnostics.
- [ ] Investigate/tighten the bulk-edit claim-time heuristic (#3) so ordinary refactor verbs in spec prose don't trip a bulk-edit acknowledgment requirement for non-bulk mechanism changes.
- [ ] Document (or change) the expected subagent behavior when a dispatched implement/review subagent hits a long-running background gate (#4) — either the subagent should poll-and-finish, or the orchestrating skill should account for the hand-back explicitly instead of relying on ad hoc "orchestrator finishes the job" recovery.

## References

- Mission where observed: #2546 (`relocation-hardened-dead-code-scanners-01KX958P`)
- Parent friction umbrella: #2017 (see Class A3/A4 for the closest existing kin to friction point #1)
- Epic: #2160 (coord-topology artifact-authority unification)
- Adjacent but distinct: #2229 (diff-compliance over-block at review-claim, different mechanism from #3), #1738/#1742 (issue-matrix completeness/read-path, different facet from #5), #1257 (closed — a different bulk-edit claim-time false positive)

Contributor guide

Open the contributing guide

Research direction

Treat this as triage for five separate failure modes rather than one patch. Start with the move-task flow and src/specify_cli/cli/commands/review/_issue_matrix.py::validate_issue_matrix, then reproduce the sync-daemon timeout and non-canonical-column failure described here. Done means each confirmed problem has a focused scope, reproduction, and appropriate test or diagnostic expectation.

Written by the indexing model from the issue text.

Assessment

Tech stack
git, python
Domain
cli, testing, tooling
Issue type
Bug
Difficulty
5/5
Estimated time
Over a week
Activity status
Quiet
Clarity
Needs clarification
Newbie friendliness
28/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.