storkit: done 426_bug_mergemaster_pipeline_marks_story_done_without_verifying_code_landed_on_master

2026-03-28 12:37:43 +00:00
parent 953fce2ca6
commit a0326dae78
1 changed files with 0 additions and 0 deletions
@@ -0,0 +1,77 @@
+---
+name: "Mergemaster pipeline marks story done without verifying code landed on master"
+retry_count: 1
+---
+
+# Bug 426: Mergemaster pipeline marks story done without verifying code landed on master
+
+## Description
+
+The mergemaster pipeline can mark a story as done even when the feature code never makes it to master. The cherry-pick step in merge.rs may fail or be skipped, but the pipeline still advances the story to done via the filesystem watcher. There is no post-merge verification that the code actually exists on master before marking done.
+
+## How to Reproduce
+
+Observed on stories 422 and 403. For 422: mergemaster created merge-queue branch, resolved 2 conflicts in chat/commands/mod.rs and http/mcp/mod.rs, passed quality gates, created merge-queue commit cb2ef6b (4 files, 333 insertions including unblock.rs). But the done commit on master (05db012) only moves the story file — zero code changes. There is no 'storkit: merge 422' commit on master at all. The feature branch (db3157f) still has the code but it was never cherry-picked onto master.
+
+## Manual Merge Notes
+
+When manually cherry-picking 422 onto master, two conflicts arose:
+
+1. `server/src/chat/commands/mod.rs` — both 421 (timer) and 422 (unblock) added entries to the same BotCommand registry. Resolution: keep both.
+2. `server/src/http/mcp/mod.rs` — 420 (loc_file) and 422 (unblock) both bumped the tool count assertion from 49→50. Resolution: keep loc_file assertion, bump count to 51.
+
+Additionally, the cherry-pick could not proceed at all because master was on the `merge-queue/424` branch with 3 unresolved files (notifications.rs, ws.rs, watcher.rs). A concurrent in-progress merge left the working tree dirty, which likely caused the original cherry-pick to fail silently. This suggests a race condition: the filesystem watcher commits (story file moves) can leave master in a state where the cherry-pick step in merge.rs fails.
+
+## Full Audit of Done Stories (2026-03-28)
+
+Audited all 9 stories in `5_done/` to check whether their code actually landed on master:
+
+| Story | Merge Commit | Code on Master |
+|-------|-------------|----------------|
+| 417 — Split matrix/bot.rs | `665c036` (9 files, +1973/-1926) | YES |
+| 418 — Split pool/auto_assign.rs | `d375c4b` (7 files, +1901/-1813) | YES |
+| 419 — Matrix bot network error | `1193b7a` (1 file, +121/-3) | YES |
+| 420 — loc file command | `d6f8239` (5 files, +112/-32) | YES |
+| 421 — Timer command | `cf5424f` (7 files, +836) | YES |
+| 422 — Unblock command | `6c6bc35` (4 files, +336) — manual cherry-pick | YES |
+| 423 — Auto-schedule timer on rate limit | `b44f3a3` + `8ab2e19` (6 files, +375/-8) — manual cherry-pick | YES |
+| **424 — Rate limit traffic light** | **None** | **NO — moved back to backlog for redo** |
+| 425 — Chat notification on story block | `98b5475` (5 files, +184/-15) | YES |
+| **427 — Text normalization for line breaks** | **None** | **NO — phantom done, code never landed** |
+
+**4 out of 10 stories (422, 423, 424, 427) had broken merges.** 422 and 423 were fixed via manual cherry-pick. 424 was moved back to backlog for a fresh run. 427 also hit the same bug — marked done without code on master.
+
+## Actual Result
+
+Story moved to done with no code on master. The merge-queue commit exists on a detached branch but was never applied to master. No merge commit appears in git log on master.
+
+## Expected Result
+
+Pipeline should verify that the cherry-pick produced a merge commit on master before advancing to done. If cherry-pick fails or is missing, the story should remain in merge stage with a merge_failure flag.
+
+## Suggested Fix
+
+The code path is: `merge.rs::run_squash_merge` → `pipeline/merge.rs::start_merge_agent_work` → `lifecycle.rs::move_story_to_archived`.
+
+`run_squash_merge` (merge.rs:354) cherry-picks the merge-queue commit onto `project_root` and checks `cp.status.success()`. If it returns `success: true`, `start_merge_agent_work` (pipeline/merge.rs:106) immediately calls `move_story_to_archived`, which moves the story file to `5_done/`. The watcher then commits "storkit: done".
+
+The gap: between the cherry-pick returning success and the story moving to done, nobody verifies the cherry-pick actually produced a code commit on master. Possible failure modes:
+
+1. `project_root` is not on master (e.g. checked out to a merge-queue branch from a concurrent merge)
+2. Cherry-pick exits 0 but produces an empty commit (no code diff)
+3. Cherry-pick succeeds on the wrong branch
+
+**Fix:** After the cherry-pick in `run_squash_merge` succeeds (line 384), before returning `success: true`:
+
+1. Verify `project_root` is on master: `git rev-parse --abbrev-ref HEAD` must equal the base branch
+2. Verify the HEAD commit on master contains the expected merge message (e.g. matches `storkit: merge <story_id>`) or has a non-empty diff
+3. If either check fails, abort the cherry-pick and return `success: false`
+
+This keeps the fix entirely within `run_squash_merge` — no changes needed to the pipeline advance or lifecycle code.
+
+## Acceptance Criteria
+
+- [ ] Pipeline must not move a story to done unless a merge commit containing the feature code exists on master
+- [ ] If cherry-pick fails or produces no code diff on master, the merge must be reported as failed
+- [ ] Add a post-merge verification step that checks git log on master for the expected merge commit before advancing to done
+- [ ] When verification fails, emit a merge_failure and leave the story in the merge stage for retry