spec-kitty / spec-kitty/spec-kitty

E2 CONSOLIDATED write: wrong-local-branch residue + FR-006 recovery wording + guard-allowlist reverse-check rot

Open
#3,088 1 comment 0 reactions 0 assignees View on GitHub
git priority:P2 reliability
Dominant language
Python
Stars
1.6k
Forks
165
Avg merge
14h 22m
Merged PRs (30d)
343

Description

Follow-up from the landing-pass review squad on PR #3087 (post-consolidation write surface + deterministic authoring). These are MINOR/NOTE findings deliberately **not** folded into #3087 (they are new robustness edges, not incompleteness in the FRs #3087 closes). Parent epic: **#3044** (write-side placement seam; cf. #2160, #1676).

## 1. Wrong-local-branch E2 write leaves residue + generic error (MINOR — architect-alphonso)

For a PUBLISHED (E2) mission whose consolidated content **is** present at the Primary-Branch tip, but whose repository-root checkout HEAD sits on a leftover branch (e.g. a `review/…` branch), a PRIMARY-kind write:

- `resolve_placement_only` → `PUBLISHED` → `content_present_at_primary_tip` is True → returns `CommitTarget(ref=main)` (no `CONSOLIDATED_CONTENT_ABSENT` raise);
- `write_seam.write_artifact` probe succeeds → `_materialize_files` runs `stage()` (writes the file to disk);
- `_commit_post_consolidation_write` → `safe_commit(target=main, worktree_root=repo)` fails the **unconditional HEAD-match guard** (`git/commit_helpers.py`) → `status="error"` with the **generic** safe-commit message.

Two coherence gaps: (a) the operator gets a generic error, **not** the FR-006 branch-named recovery hint; (b) because resolution *succeeded*, `stage()` already ran, so the materialized artifact is left as **untracked residue** in the working tree. #3073's "no residue on a *refused* write" contract is genuinely satisfied (probe-before-stage), but this is the distinct "resolved-but-uncommittable-from-here" path.

Repro sketch: seed a genuine E2 mission (baseline_merge_commit + mission_number + published/pruned), leave HEAD on a non-primary branch, `safe-commit` a retrospective → observe `status=error`, generic message, and the retrospective file left untracked.

Candidate fixes (need design, hence follow-up): make the E2 probe HEAD-aware so it refuses **before** `stage()` with the branch-named recovery, **or** clean up materialized residue on a post-stage commit failure. Do **not** weaken the unconditional HEAD-match guard.

## 2. FR-006 recovery message incomplete for the stale-clone trigger (MINOR — architect-alphonso)

`_resolve_consolidated_e2_target`'s `CONSOLIDATED_CONTENT_ABSENT` message says "check out {primary} and retry", but its actual trigger is a clone whose Primary-Branch **ref** does not yet carry the content (stale clone) — the operator must `git fetch` first; "checkout and retry" alone won't resolve it. Reword to cover fetch-then-checkout (taking care not to mislead the genuinely-not-yet-published case).

## 3. Guard-capability allowlist reverse-check rot (NOTE — paula-patterns)

`tests/architectural/test_guard_capability_call_sites.py` restricts its *reverse* "stale allowlist entry" assertion to `{MERGE_BOOKKEEPING, UPGRADE_BOOKKEEPING}`. If a future refactor drops one of the two `POST_CONSOLIDATION_WRITE` assert sites, its allowlist entry silently rots (retained but no longer asserting). Both sites assert today — no live defect. Add `POST_CONSOLIDATION_WRITE` to that reverse-check set once it stabilizes.

## 4. Canonical repo-slug is upstream-only (NOTE — paula-patterns)

`issue_matrix._CANONICAL_REPO_SLUG` hardcodes `Priivacy-ai/spec-kitty`, so a **fork**-URL reference to the same logical issue is treated cross-repo. Deliberate (git-remote introspection would break tmp-repo fixture determinism) and documented inline. Flagging as known coupling for future multi-repo work. (The case-insensitivity false-negative was folded into #3087; this is the residual fork-org coupling only.)

---
🤖 Filed by the PR #3087 landing pass (review squad: reviewer-renata, architect-alphonso, paula-patterns).

Contributor guide

Open the contributing guide

Research direction

Start with _resolve_consolidated_e2_target and write_seam.write_artifact, then inspect _commit_post_consolidation_write and the unconditional HEAD guard in git/commit_helpers.py. Review the FR-006 message and tests/architectural/test_guard_capability_call_sites.py, including the issue_matrix slug note. Done means the chosen design prevents or cleans residue, gives accurate fetch/checkout recovery guidance, and keeps the allowlist reverse-check aligned.

Written by the indexing model from the issue text.

Assessment

Tech stack
git, python
Domain
testing, tooling
Issue type
Bug
Difficulty
4/5
Estimated time
3-5 days
Activity status
Quiet
Clarity
Mostly clear
Newbie friendliness
38/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.