Re-cut test_merge_stray_dirt_escalates_with_clear_message to pin the hazard-first refusal lead-in

Open Beginner friendly
#683 0 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

Assessment

Difficulty
2/5
Estimated time
1-3 hours
Newbie friendliness
87/100
Issue type
Bug
Clarity
Clearly specified
Activity status
Active
Tech stack
python
Domain
cli, tooling

Research direction

Start in WorktreeFlow.merge_local and inspect the escalation text raised from clean_incoming_collisions. Then read tests/test_engine_worktree.py::test_merge_stray_dirt_escalates_with_clear_message and its six assertions. Done means the lead-in covers both causes, preserves the two safe resolutions and negative properties, and the test assertions are deliberately updated.

Written by the indexing model from the issue text.

Description

area:engine bug P3

Found while wiring protected for #618 (phase 4). Message-only; out of scope there.

What is wrong

WorktreeFlow.merge_local's escalation text for a GitError out of clean_incoming_collisions reads:

merge of <branch> into <target> blocked: the target checkout has uncommitted changes to tracked files outside this branch — a merge or squash would fold them into this story's commit. Commit, stash or revert them, then `bmad-loop resume <run>`. One cause is a per_worktree engine Editor writing into the main checkout; another is ordinary local work. <inner GitError>

Since #618 that raise has two causes, and the lead-in describes only the first:

  1. staged changes to tracked files outside the branch — the merge really can fold those in. The lead-in is accurate.
  2. dirt on a path the run itself commits after the merge (protected). Here the merge folds nothing; commit_paths does, in the run's own bookkeeping commit. "a merge or squash would fold them into this story's commit" names the wrong mechanism.

The inner GitError appended as {e} is accurate for both — phase 3 gave it two separately-worded clauses, one per remedy — so the operator does get the right diagnosis, just after a lead-in that contradicts it.

"tracked files" is also not quite right for cause 2 in principle: protected blocks whatever the index column says. It happens to be accurate today only because _carried_artifact_rels restricts itself to tracked paths (see the sibling issue on untracked artifacts).

Why it was not fixed in place

tests/test_engine_worktree.py::test_merge_stray_dirt_escalates_with_clear_message pins six properties of this string, and phase 3 had just settled that taxonomy. Rewording is a small change that should land with its assertions re-cut deliberately rather than as a side effect of the wiring commit.

Suggested shape

Keep the two SAFE resolutions and the negative properties #460 won (no Unity attribution, no "clean them"), and make the lead-in cover both causes without asserting either mechanism — leaving the inner clauses to say which one fired.

Dominant language
Python
Stars
137
Forks
60
Avg merge
2d 13h
Merged PRs (30d)
45

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

More from bmad-code-org/bmad-loop

All issues in bmad-code-org/bmad-loop

Similar issues

More Python issues

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.