ldraney/claude-custom
1
0
Fork 0
Code Issues 32 Pull requests Projects Releases Packages Wiki Activity Actions

Bug: merge hook reports failure when Forgejo API returns merged=true #216

New issue
Closed
opened 2026-03-28 18:50:08 +00:00 by forgejo_admin · 3 comments
forgejo_admin commented 2026-03-28 18:50:08 +00:00
Contributor
Copy link

Type

Bug

Lineage

  • Board: board-pal-e-agency
  • Story: story:pm-scope
  • Arch: arch:hooks
  • Discovered scope from session 2026-03-28 (8 merges, all false negatives)

Repo

forgejo_admin/claude-custom

User Story

As the PM, I need the post-merge hook to correctly detect successful merges so that /update-docs and board automation fire reliably. Currently the hook reports "Merge was not successful" even when the Forgejo API returns merged: true.

Context

During session 2026-03-28, all 8 PR merges via mcp__forgejo__merge_approved_pr returned merged: true from the API but the PostToolUse hook fired "Merge was not successful. Do NOT run /update-docs." This blocks the entire post-merge chain (board updates, doc updates). Betty Sue had to manually verify each merge via API and manually update board items.

Root cause: The _parse_merged_status function in hooks/forgejo-helper.sh (lines 333-380) has 5 parsing shapes, but none handle the actual PostToolUse JSON shape from Claude Code. The shape is double-stringified: tool_response (string) → parse → {"result": "..."} → parse result → {"merged": true}. This requires a two-level unwrap ("Shape 4+2") that no existing shape performs. Reproduction data captured at /tmp/hook-debug-merge.json.

File Targets

  • hooks/forgejo-helper.sh (lines 333-380, _parse_merged_status function) — add a new parsing shape for double-stringified tool_response. The hook scripts that call this function (post-mcp-merge-rebase.sh, remind-update-docs.sh, board-item-on-merge.sh) need no changes.
  • tests/test_parse_merged_status.sh — add test case for the double-stringified shape using the payload structure from /tmp/hook-debug-merge.json

Acceptance Criteria

  • _parse_merged_status correctly detects merged: true in the double-stringified shape (tool_response string → parse → result string → parse → merged boolean)
  • Existing 13 test cases continue to pass (no regression)
  • New test case covers the double-stringified shape and passes

Test Expectations

  • Add test case to tests/test_parse_merged_status.sh with double-stringified payload — returns "true"
  • All 13 existing tests still pass
  • Run bash tests/test_parse_merged_status.sh — all pass

Constraints

  • Fix only _parse_merged_status in forgejo-helper.sh — do not modify the hook scripts that call it
  • Don't break existing parsing shapes 1-5

Checklist

  • Add "Shape 6" (double-stringified) to _parse_merged_status in forgejo-helper.sh
  • Add test case for Shape 6 in tests/test_parse_merged_status.sh
  • Run full test suite to verify no regression
  • Create PR

Related

  • forgejo_admin/claude-custom#189 — predecessor bug that introduced _parse_merged_status with 5 shapes
  • forgejo_admin/claude-custom#161 — scope review pipeline (sibling hook work)
  • session_2026_03_28 — 8 false negatives observed
### Type Bug ### Lineage - Board: board-pal-e-agency - Story: story:pm-scope - Arch: arch:hooks - Discovered scope from session 2026-03-28 (8 merges, all false negatives) ### Repo `forgejo_admin/claude-custom` ### User Story As the PM, I need the post-merge hook to correctly detect successful merges so that /update-docs and board automation fire reliably. Currently the hook reports "Merge was not successful" even when the Forgejo API returns `merged: true`. ### Context During session 2026-03-28, all 8 PR merges via `mcp__forgejo__merge_approved_pr` returned `merged: true` from the API but the PostToolUse hook fired "Merge was not successful. Do NOT run /update-docs." This blocks the entire post-merge chain (board updates, doc updates). Betty Sue had to manually verify each merge via API and manually update board items. **Root cause:** The `_parse_merged_status` function in `hooks/forgejo-helper.sh` (lines 333-380) has 5 parsing shapes, but none handle the actual PostToolUse JSON shape from Claude Code. The shape is double-stringified: `tool_response` (string) → parse → `{"result": "..."}` → parse `result` → `{"merged": true}`. This requires a two-level unwrap ("Shape 4+2") that no existing shape performs. Reproduction data captured at `/tmp/hook-debug-merge.json`. ### File Targets - `hooks/forgejo-helper.sh` (lines 333-380, `_parse_merged_status` function) — add a new parsing shape for double-stringified tool_response. The hook scripts that call this function (`post-mcp-merge-rebase.sh`, `remind-update-docs.sh`, `board-item-on-merge.sh`) need no changes. - `tests/test_parse_merged_status.sh` — add test case for the double-stringified shape using the payload structure from `/tmp/hook-debug-merge.json` ### Acceptance Criteria - [ ] `_parse_merged_status` correctly detects `merged: true` in the double-stringified shape (tool_response string → parse → result string → parse → merged boolean) - [ ] Existing 13 test cases continue to pass (no regression) - [ ] New test case covers the double-stringified shape and passes ### Test Expectations - [ ] Add test case to `tests/test_parse_merged_status.sh` with double-stringified payload — returns "true" - [ ] All 13 existing tests still pass - [ ] Run `bash tests/test_parse_merged_status.sh` — all pass ### Constraints - Fix only `_parse_merged_status` in `forgejo-helper.sh` — do not modify the hook scripts that call it - Don't break existing parsing shapes 1-5 ### Checklist - [ ] Add "Shape 6" (double-stringified) to `_parse_merged_status` in `forgejo-helper.sh` - [ ] Add test case for Shape 6 in `tests/test_parse_merged_status.sh` - [ ] Run full test suite to verify no regression - [ ] Create PR ### Related - `forgejo_admin/claude-custom#189` — predecessor bug that introduced `_parse_merged_status` with 5 shapes - `forgejo_admin/claude-custom#161` — scope review pipeline (sibling hook work) - `session_2026_03_28` — 8 false negatives observed
forgejo_admin commented 2026-03-28 22:58:23 +00:00
Author
Contributor
Copy link

Scope Review: NEEDS_REFINEMENT

Review note: review-585-2026-03-28
Ticket has correct traceability and is well-scoped for a single agent, but file targets are wrong and the Type header contradicts the board label.

Issues found:

  • Type header says "Feature" but board label and title say Bug
  • Primary file target is hooks/post-mcp-merge-rebase.sh but the actual fix target is hooks/forgejo-helper.sh (lines 333-380, _parse_merged_status function)
  • Missing file target: tests/test_parse_merged_status.sh (existing test file needs new test case)
  • Root cause confirmed: double-stringified tool_response shape not handled (string -> parse -> {result: string} -> parse -> {merged: true})
  • AC1 is vague -- should specify the double-stringified shape explicitly
## Scope Review: NEEDS_REFINEMENT Review note: `review-585-2026-03-28` Ticket has correct traceability and is well-scoped for a single agent, but file targets are wrong and the Type header contradicts the board label. **Issues found:** - Type header says "Feature" but board label and title say Bug - Primary file target is `hooks/post-mcp-merge-rebase.sh` but the actual fix target is `hooks/forgejo-helper.sh` (lines 333-380, `_parse_merged_status` function) - Missing file target: `tests/test_parse_merged_status.sh` (existing test file needs new test case) - Root cause confirmed: double-stringified `tool_response` shape not handled (string -> parse -> `{result: string}` -> parse -> `{merged: true}`) - AC1 is vague -- should specify the double-stringified shape explicitly
forgejo_admin commented 2026-03-28 22:59:18 +00:00
Author
Contributor
Copy link

Scope Refinement Applied (review-585-2026-03-28)

5 changes per review recommendations:

  1. Type: Feature → Bug (matches board label and title)
  2. File target: post-mcp-merge-rebase.sh → forgejo-helper.sh (lines 333-380, _parse_merged_status)
  3. Added file target: tests/test_parse_merged_status.sh
  4. Added root cause: double-stringified shape documentation with exact unwrap path
  5. Clarified AC1: specifies the double-stringified shape explicitly

Re-running scope review.

## Scope Refinement Applied (review-585-2026-03-28) 5 changes per review recommendations: 1. **Type**: Feature → Bug (matches board label and title) 2. **File target**: `post-mcp-merge-rebase.sh` → `forgejo-helper.sh` (lines 333-380, `_parse_merged_status`) 3. **Added file target**: `tests/test_parse_merged_status.sh` 4. **Added root cause**: double-stringified shape documentation with exact unwrap path 5. **Clarified AC1**: specifies the double-stringified shape explicitly Re-running scope review.
forgejo_admin commented 2026-03-28 23:01:17 +00:00
Author
Contributor
Copy link

Scope Review: READY

Review note: review-585-2026-03-28-v2
Re-review after refinement -- all 5 body fixes from previous review verified as correctly applied. Scope is solid, file targets verified against codebase, traceability complete, fits in a single agent pass (<5 min). Ready for dispatch.

## Scope Review: READY Review note: `review-585-2026-03-28-v2` Re-review after refinement -- all 5 body fixes from previous review verified as correctly applied. Scope is solid, file targets verified against codebase, traceability complete, fits in a single agent pass (<5 min). Ready for dispatch.
forgejo_admin 2026-03-28 23:11:45 +00:00
Sign in to join this conversation.
No Branch/Tag specified
Branches Tags
main
217-enhance-skill-review-ticket-verify-arch
86-epilogue-nits-hook-updates-dottie-config
76-add-sessionstart-hook-to-detect-missing
69-fix-block-docs-writes-sh-security-gap-ad
51-phase-3-agent-label-behavior-posttooluse
No results found.
Labels
Clear labels   
domain:backend

   
domain:devops

   
domain:frontend

   
status:approved

QA passed, awaiting merge approval

  
status:in-progress

Dev agent is actively working

  
status:needs-fix

QA found issues, back to dev

  
status:qa

PR submitted, awaiting QA review

  
type:bug

Bug fix

  
type:devops

Infrastructure/CI/config work

  
type:feature

New functionality

No labels
domain:backend
domain:devops
domain:frontend
status:approved
status:in-progress
status:needs-fix
status:qa
type:bug
type:devops
type:feature
Milestone
Clear milestone
No items
No milestone
Projects
Clear projects
No items
No project
Assignees
Clear assignees
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference
ldraney/claude-custom#216
No description provided.