Implementation Plan: Implement-Loop Commit & Move-Task Hardening
Branch: mission/2533-pr-bound-coord-claim-precondition | Date: 2026-07-15 | Spec: spec.md Input: kitty-specs/implement-loop-commit-hardening-01KXJ1ZX/spec.md Tracker: closes #2647, #2648, #2649, #2650 (sub-issues of #2160). Stacks on the merged #2533.
Summary
Close four #2533 follow-ups: fix move-task from a lane-worktree cwd (#2647), delete the silent protected-branch coord-divert and fail-close the narrow-triple placement_ref is None state (#2648, NOT bare None — that stays the legit strangler path), pay down the Sonar S3776/S107 debt in implement.py/tasks_move_task.py (#2649, folds #2604), and consolidate the three partition-decision sites onto the existing residue predicate (#2650). Brownfield paradigm active — structural moves are gated on captured understanding (characterization-first), and the consolidation is pinned to the #2533-safe direction (kind=None→PRIMARY, C-007) so it cannot regress the bug #2533 just closed.
Technical Context
Language/Version: Python 3.11+ Primary Dependencies: Typer, rich / CliConsole, ruamel.yaml, internal mission_runtime + coordination packages Storage: Git refs (status surface, coordination vs primary branches); no database Testing: pytest — characterization tests (pin implicit invariants before extraction), red-first bug repros through the real move-task / write-side entry points, structural test for the single partition authority; -n auto --dist loadfile Target Platform: Spec Kitty CLI (Linux/macOS dev) Project Type: single (Python CLI monorepo) Performance Goals: N/A (correctness + maintainability) Constraints: ruff C901 ≤ 15, params ≤ 13; mypy --strict zero new issues; no net-new public API (C-008); kind=None→PRIMARY (C-007); preserve #2576 dual-handler (C-001) and Lane-A→B symbol contracts (C-006) Scale/Scope: 7 WPs, 3 lanes (final, per tasks.md); ~4 source files (implement.py, implement_cores.py, commit_router.py, tasks_move_task.py) + tests
Charter Check
GATE: Must pass before Phase 0. Re-check after Phase 1.
Charter present; charter context --action plan = compact. Brownfield-onboarding paradigm ACTIVE — its discipline governs this mission:
gate precedes the FR-005 swap; FR-003/FR-004 pin implicit invariants before extraction (NFR-002 floor). ✅
(_resolve_bookkeeping_transaction_identifiers cascade+raise; _json_safe_output _file=None reset; _mt_uncheck_rollback_subtasks dual-handler) are pinned, not guessed. ✅ (C-001)
real entry points. ✅ (C-002)
consolidation direction is #2533-safe (C-007). ✅
fallback (the #2533 hole) is explicitly forbidden by C-007. ✅
- DM-D (document-then-refactor) / characterization-first — FR-006 characterization
- Chesterton's fence — the degod targets with load-bearing invariants
- ATDD / red-first (DIRECTIVE_041) — the two bugs (WP01/WP05) repro red-first through
- Single canonical authority — FR-005 collapses three partition sites to one; the
- No legacy resolver paths / no silent fallback — the
kind=None→caller-partition - DIR-013 — any pre-existing unrelated test failure → open an issue before treating as baseline.
No Charter Check violations → Complexity Tracking not required.
Project Structure
Documentation (this mission)
kitty-specs/implement-loop-commit-hardening-01KXJ1ZX/
├── plan.md, research.md, data-model.md, quickstart.md, contracts/
├── traces/ # tracer files (seeded; append during implement)
└── tasks.md # /spec-kitty.tasks output (NOT created here)
Source Code (repository root)
src/specify_cli/cli/commands/
├── implement.py # Lane A — _partition_files_for_commit (write site), _commit_planning_artifacts_transaction
│ # (protected-branch fallback + warning, FR-002), _json_safe_output / _resolve_bookkeeping_transaction_identifiers
│ # / _run_recover_mode (degod, FR-003)
├── implement_cores.py # Lane A — resolve_precondition_ref (read site + "HEAD"; FR-005)
└── agent/tasks_move_task.py # Lane B — status-surface resolution (FR-001), _mt_commit_wp_file /
# _do_move_task / _mt_uncheck_rollback_subtasks (degod, FR-004)
src/specify_cli/coordination/
└── commit_router.py # Lane A — _group_files_by_partition (write site; kind classifier) → consolidated (FR-005)
src/mission_runtime/
└── artifacts.py # READ-ONLY — partition authority (is_coordination_artifact_residue_path etc.)
Structure Decision: Single project. Three file-linearized lanes (final): Lane A owns implement.py + implement_cores.py, Lane B owns commit_router.py, Lane C owns tasks_move_task.py. mission_runtime read-only.
Complexity Tracking
Not applicable — no Charter Check violations.
Implementation Concern Map
> Concerns are NOT work packages. /spec-kitty.tasks translates these into WPs.
IC-01 — Delete the 767 divert; NARROW-triple fail-close (#2648, FR-002)
the artifact-commit half fail-closed (PlacementResolutionRequired, same message as the status half) on the narrow triple — placement_ref is None AND meta coord_branch truthy AND is_protected(planning_branch) — which is EXACTLY the 767 precondition and EXACTLY where the status half (_resolve_claim_commit_target, implement_cores.py:608) already fails closed. So both halves agree on that state.
None is the C-004 strangler signal (_resolve_placement_ref returns None on ActionContextError OR artifact_placement is None). The legacy/flat 755 arm and the coord-non-protected 790 arm MUST stay green — three write-side tests (test_implement_writeside.py:192/231/285) + the #2533 regression (test_implement.py:283) drive placement_ref=None expecting SUCCESS. An unconditional raise reds all of them.
narrow-triple arm (Option B — mirror _resolve_claim_commit_target's PlacementResolutionRequired + message). Do NOT rely on deleting 767 and letting the 790 else-path hit BookkeepingTransaction's generic typer.Exit(1) (Option A) — that raises the WRONG type and fails SC-002's "same precondition + message" requirement.
primary/coord default), C-004 (the fail-closed pin is enduring for IC-04).
arm body with the raise; place the guard after the if not files_to_commit: return short-circuit). New red-first test using the _seeded_coord_mission harness with planning_branch="main" (protected) + placement_ref=None: RED = silent divert to coord; GREEN = pytest.raises(PlacementResolutionRequired). Acceptance MUST also re-run the 3 write-side None tests + TestSoloPrBoundCoordMissionClaimPrecondition green.
IC-04** (the consolidation must keep both halves fail-closed on one authority).
flat/legacy mission ever reach this seam with None?), not guessed. Do NOT re-add a warn-and-degrade; do NOT broaden to bare None.
- Purpose: Remove the silent
767-789coord-divert (a #2533-introduced guard) and make - CRITICAL scoping (squad BLOCKER-1): do NOT fail-close on bare
placement_ref is None. - Implementation choice (resolve the plan's old "and/or"): raise explicitly at the
- Requirements: FR-002; NFR-001; guards C-002 (red-first), C-009 (narrow fail-closed, no
- Surfaces:
implement.py::_commit_planning_artifacts_transaction(replace the767 - Sequencing: none (first in Lane A). Its fail-closed behavior is a **dependency of
- Risks: the narrow triple is confirmed by WP03's characterization gate (does a real
IC-02 — implement.py degod, characterization-first (#2649, FR-003) — carves into WP02a + WP02b
_resolve_bookkeeping_transaction_identifiers (≈16), _run_recover_mode (≈24), preserving implicit invariants. NOTE: all three already pass ruff C901 (cyclomatic 4-10) — S3776 is the target metric (not ruff-measurable), so the local done-condition is the extraction + per-helper tests + behavior preservation; S3776 confirmation is advisory-post-merge.
(the C-006 5-tuple symbol) with a signature-freeze + a consumer-side import-contract test; WP02b = _json_safe_output + _run_recover_mode. This lands the C-006 edge at Lane-A unit 2 and keeps the heavy S3776 work off the critical coupling.
WP02a** — declared hard edge (C-006), not "preserve OR sequence".
_resolve_bookkeeping_transaction_identifiers: cascade order, ambiguous-handle RAISE, the legacy-<slug> mission-id fallback, and the mid8 precedence (meta-mid8 > resolve_mid8 > None); for _json_safe_output: console._file=None reset, console.quiet save/restore, the dual exception arms (typer.Exit re-raised verbatim vs bare Exception → typer.Exit(1)), and the exit_code-0 payload suppression + last-20-non-blank summary.
- Purpose: Reduce S3776 cognitive complexity on
_json_safe_output(≈33), - Carve-out (squad): WP02a =
_resolve_bookkeeping_transaction_identifiersalone - Requirements: FR-003; NFR-001, NFR-002, NFR-004; C-008.
- Surfaces:
implement.py(those functions + extracted private helpers + tests). - Sequencing: depends IC-01 (same file, serialized). **WP06 (Lane B degod) depends_on
- Characterization floor (expanded — renata): pin BEFORE extraction — for
IC-03 — Partition-consolidation characterization gate (#2650, FR-006)
set and pin the intended unified placement (kind=None→PRIMARY) before any swap.
commit_router::_group_files_by_partition, implement_cores.py::resolve_precondition_ref.
intended contract explicit so IC-04 is a documented move, not a guess.
- Purpose: Document-first — enumerate the three sites + the
kind=Nonedisagreement - Requirements: FR-006; NFR-003; C-007.
- Surfaces: new characterization tests over
implement.py::_partition_files_for_commit, - Sequencing: depends IC-02 (Lane A serialization).
- Risks: the suites currently encode BOTH fallback directions — the gate makes the
IC-04 — Partition-classifier consolidation + ref unification (#2650, FR-005) — carves into WP04a + WP04b
unify the read/write primary-ref expression; keep #2648 + #2533 regressions green.
mission_runtime.is_coordination_artifact_residue_path (two sites already call it; implement.py:600, implement_cores.py:268). FR-005 = swap commit_router:404's divergent kind_for_mission_file(file) or kind classifier onto it. Classifier-only: keep resolve_placement_only for commit_router's actual COORD ref (other_ref); do NOT add a cli-side partition_of wrapper (redundant + inverts cli → coordination layering, C-008); do NOT edit mission_runtime (read-only). No net-new symbol needed → satisfies SC-004 against the existing predicate.
test that the three sites resolve partition through the one predicate AND commit_router no longer consults is_primary_artifact_kind(kind_for_mission_file(...)) for the split. WP04b = the read ("HEAD") / write (planning_branch) primary-ref unification — cli-local (Lane A), does NOT touch commit_router; own structural test + its own regression surface (detached-HEAD / off-target). Two authorities, two tests.
implement.py, implement_cores.py. Structural tests per WP.
caller-fallback would misroute meta.json→COORD; C-007 forbids it. Structural test must assert the kind-classifier is dropped for the split, not merely that meta.json routes PRIMARY (else a future coord-kind caller re-diverges past the test — squad RISK-4).
- Purpose: Route the three sites' PRIMARY-vs-COORD partition through ONE authority and
- Scope correction (squad RISK-3, boundary): the authority ALREADY exists —
- Carve-out (squad): WP04a = the classifier swap (
commit_router:404) + structural - Requirements: FR-005; SC-004; guards C-007 (kind=None→PRIMARY), C-004 (#2648 green).
- Surfaces: WP04a:
commit_router.py(+ the residue predicate call). WP04b: - Sequencing: depends IC-03 (characterization) → IC-02 → IC-01.
- Risks: the #2533 regression — consolidating onto the kind-classifier's
IC-05 — move-task cwd-independent status surface (#2647, FR-001)
root regardless of cwd; works from a lane worktree AND from repo root.
_read_transactional_wp_lane at :308 ALREADY reads with repo_root=st.main_repo_root (mission root) — it is NOT the taint. The cwd taint enters upstream at locate_project_root() (:244) and locate_work_package(repo_root, …) (:306). The red-first test MUST FIRST locate which read returns the stale lane before extracting; if no fixture can make the currently-named seam go RED, the seam is misidentified — escalate before the fix.
so is_worktree_context(cwd) (:223-224) is genuinely true; RED asserts the concrete "Illegal transition" error string (not merely "succeeds after"); pair with an explicit repo-root-cwd no-regression assertion (FR-001).
- Purpose:
move-taskresolves the mission status surface from the canonical mission - Requirements: FR-001; NFR-001; C-002 (red-first).
- Surfaces:
tasks_move_task.pystatus-read path. Localization caveat (squad RISK-4): - Sequencing: none (first in Lane B). Independent of Lane A.
- Risks / test shape: drive a REAL
git worktreefixture withmonkeypatch.chdir(worktree)
IC-06 — tasks_move_task.py degod, characterization-first (#2649, FR-004) — folds #2604
param-object), _mt_uncheck_rollback_subtasks (S8572). Folds #2604 (same _mt_commit_wp_file).
ONE locally-hard gate is _do_move_task parameters ≤ 13 (from 21, via param-object) — this is the concrete acceptance. Do NOT assert success via ruff C901 (already green).
WP02a** (the runtime call at :1392 consumes the 5-tuple regardless of ordering — the guard is the dep edge + WP02a's consumer contract test, not "reconcile if changed"). Landing note C-005 — rebase after PR #2639.
keep the degrade-never-crash discipline; param-object so #2639's added arg stays ≤13.
- Purpose: Decompose
_mt_commit_wp_file(S3776≈19),_do_move_task(21 params → - Gate re-anchor (brownfield check): the S3776 reductions are advisory-post-merge; the
- Requirements: FR-004; NFR-001, NFR-002, NFR-004; C-001 (#2576 dual-handler), C-005 (#2639), C-008.
- Surfaces:
tasks_move_task.py(those functions + private helpers + tests). - Sequencing: depends IC-05 (same file). **Declared hard C-006 edge: WP06 depends_on
- Risks: do NOT merge the #2576 dual-handler / swap to
logging.exception(C-001);
Work-Package Shape (FINAL — as finalized by /spec-kitty.tasks; tasks.md is authoritative)
> The IC map above used provisional WP02a/WP04a a/b labels. /spec-kitty.tasks consolidated > them into a strict WP\d+ numbering (the finalizer collapses alphabetic suffixes) and, to > break a lane cycle, folded the FR-006 characterization gate into the cli-side ref-unification > WP. Final shape: 7 WPs / 3 file-linearized lanes (post-tasks squad-reviewed).
755/790 strangler arms).
signature-frozen + consumer-side import-contract test.
The gate is folded here (not a separate test-only lane) so it isn't a mid-chain island that would cycle the lane graph. Does NOT touch commit_router.
commit_router:404 onto the existing residue predicate. depends_on WP04.
(#2649 move-task degod + _do_move_task param-object; folds #2604).
tasks_move_task.py:1392 reads _resolve_bookkeeping_transaction_identifiers' return the moment WP02 reshapes it, regardless of lane parallelism). Lane graph: Lane B → Lane A and Lane C → Lane A, acyclic; Lane A independent.
P1 bug) has no WP-level dependencies and should be claimable immediately, in parallel with Lane A — do NOT let it be withheld behind Lane A. It shares Lane C's file with WP07 (which depends_on WP02), so the allocator marks the whole lane depends_on lane-a; that is a lane-granularity artifact, not a real dependency of WP06. Claim WP06 first (per-WP readiness gates on WP-level deps, which WP06 has none of). If the allocator can only gate per-lane, land WP06 as an early standalone before WP07 — the P1 fix must not wait on the tech-debt WPs. (Logged as a tooling gap in traces/tooling-friction.md.)
- Lane A —
implement.py+implement_cores.py(serial): - WP01 — #2648 delete
767+ narrow-triple fail-close (NOT bareNone; preserve the - WP02 — degod
_resolve_bookkeeping_transaction_identifiers(the C-006 5-tuple symbol), - WP03 — degod
_json_safe_output+_run_recover_mode(the heavy S3776). - WP04 — FR-006 characterization gate + FR-005 cli-side read/write ref-unification.
- Lane B —
coordination/commit_router.py: WP05 — FR-005 classifier-only swap of - Lane C —
agent/tasks_move_task.py(serial): WP06 (#2647 move-task cwd) → WP07 - C-006 is a declared hard dependency edge: WP07
depends_onWP02 (the runtime call at - Parallelism (corrected — the earlier "zero critical-path" claim was wrong): WP06 (#2647,
Scope Fences
the ref expression; the two named bug fixes; the six named degod functions.
to move-task/commit semantics beyond the two bugs; mission_runtime/* edits (read-only).
- IN: the four named issues only; consolidate the three named partition sites + unify
- OUT: broader #2160 placement-seam work beyond these three sites; any behavior change