spec-kitty / spec-kitty/spec-kitty
Coord-topology implement-review loop friction: move-task recovery cascade, sync-daemon hang, and opaque gate errors
- 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
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