From d216f3c26767a69c4893544ca1d9c97929664a08 Mon Sep 17 00:00:00 2001 From: dave Date: Sat, 28 Mar 2026 19:47:36 +0000 Subject: [PATCH] storkit: done 440_refactor_consolidate_is_permission_approval_into_chat_util --- ...advance_stories_to_done_without_merging.md | 28 ------------------- ...e_is_permission_approval_into_chat_util.md | 24 ++++++++++++++++ 2 files changed, 24 insertions(+), 28 deletions(-) delete mode 100644 .storkit/work/1_backlog/445_bug_rate_limited_mergemaster_exits_advance_stories_to_done_without_merging.md create mode 100644 .storkit/work/5_done/440_refactor_consolidate_is_permission_approval_into_chat_util.md diff --git a/.storkit/work/1_backlog/445_bug_rate_limited_mergemaster_exits_advance_stories_to_done_without_merging.md b/.storkit/work/1_backlog/445_bug_rate_limited_mergemaster_exits_advance_stories_to_done_without_merging.md deleted file mode 100644 index 8dd1f7db..00000000 --- a/.storkit/work/1_backlog/445_bug_rate_limited_mergemaster_exits_advance_stories_to_done_without_merging.md +++ /dev/null @@ -1,28 +0,0 @@ ---- -name: "Rate-limited mergemaster exits advance stories to done without merging" ---- - -# Bug 445: Rate-limited mergemaster exits advance stories to done without merging - -## Description - -When the mergemaster agent is immediately rate-limited (zero turns, zero tool calls), it exits and run_server_owned_completion runs acceptance gates on the existing worktree. Since the coder already committed working code, the gates pass, and the pipeline advances the story to done — even though the mergemaster never executed run_squash_merge and the code was never cherry-picked onto master. - -## How to Reproduce - -Observed on stories 439 and 442. All mergemaster log entries show: init → rate_limit_event → error result. Zero turns, zero MCP tool calls, duration under 350ms. Yet both stories ended up in done with no merge commit on master. - -## Actual Result - -Stories advance to done with no code on master. The mergemaster never ran but the pipeline treated its exit as a successful completion. - -## Expected Result - -If the mergemaster exits without completing its work (no merge commit produced), the story should stay in the merge stage for retry, not advance to done. - -## Acceptance Criteria - -- [ ] run_server_owned_completion must not run for mergemaster agents — mergemaster has its own completion path via start_merge_agent_work -- [ ] If the mergemaster process exits without producing a SquashMergeResult, the story stays in merge stage -- [ ] Rate-limited mergemaster exits are treated as transient failures, not gate-passing completions -- [ ] Story remains eligible for retry when mergemaster fails due to rate limiting diff --git a/.storkit/work/5_done/440_refactor_consolidate_is_permission_approval_into_chat_util.md b/.storkit/work/5_done/440_refactor_consolidate_is_permission_approval_into_chat_util.md new file mode 100644 index 00000000..35494d0f --- /dev/null +++ b/.storkit/work/5_done/440_refactor_consolidate_is_permission_approval_into_chat_util.md @@ -0,0 +1,24 @@ +--- +name: "Consolidate is_permission_approval into chat::util" +--- + +# Refactor 440: Consolidate is_permission_approval into chat::util + +## Current State + +- TBD + +## Desired State + +Three copies of `is_permission_approval` exist across Slack (`chat/transport/slack/commands.rs`), WhatsApp (`chat/transport/whatsapp/commands.rs`), and Matrix (`chat/transport/matrix/bot/messages.rs`). The Slack and WhatsApp versions are identical; the Matrix version is a superset that also strips @mentions. Consolidate into a single `pub` function in `chat::util` using the Matrix superset behavior, then delete the 3 private copies. + +## Acceptance Criteria + +- [ ] Single pub fn is_permission_approval exists in chat::util +- [ ] All 3 private copies are removed +- [ ] Matrix @mention-stripping behavior is preserved in the shared version +- [ ] All call sites use the shared version + +## Out of Scope + +- TBD