pal-e-docs

Review: Fix reconciliation script requires_approval + nickname

review-308-2026-03-27 Review

review ready
Project

Verdict: READY

Template Completeness

  • [x] Type — Bug
  • [x] Lineage — discovered during E2E test (2026-03-24)
  • [x] Repo — forgejo_admin/basketball-api
  • [x] User Story — present in body
  • [x] What Broke — groups default to requires_approval=true
  • [x] Repro Steps — 4 clear steps
  • [x] Expected Behavior — share link click = instant join
  • [x] Environment — GroupMe API default behavior documented
  • [x] File Targets — single script with specific fix guidance
  • [x] Acceptance Criteria — 3 criteria
  • [x] Test Expectations — unit test + pytest command
  • [x] Constraints — don't change SDK, keep idempotent
  • [x] Checklist — standard PR/test checklist
  • [x] Related — links to project page and parent issue #156
  • [ ] Environment (template field) — missing cluster/namespace/SHA, but acceptable for a script-level bug (not a deployed service issue)

Traceability

  • [ ] story:X label — missing from board item #308. The issue body contains a user story ("As a parent clicking a GroupMe share link...") but no story:X label is set on the board item. Should add a story label (e.g., story:GM-1 or a new story tag).
  • [x] arch:basketball-api label — present on board item
  • [x] Forgejo issue — forgejo_admin/basketball-api#159, open

File Targets

  • [x] scripts/create_groupme_groups.py — VERIFIED EXISTS. Confirmed both bugs:

SDK Verification

  • [x] groupme_sdk.GroupMeClient.update_group(group_id, **kwargs) — EXISTS at ~/groupme-sdk/src/groupme_sdk/groups.py:54. Posts to /groups/{group_id}/update. The fix can use client.update_group(group_id, requires_approval=False).
  • [x] groupme_sdk.GroupMeClient.add_member(group_id, nickname, ...) — EXISTS at ~/groupme-sdk/src/groupme_sdk/members.py:14. nickname is the second required positional arg. Ticket correctly identifies this needs a fallback (team name).

Repo Placement

OK. Issue filed on forgejo_admin/basketball-api, fix is in scripts/create_groupme_groups.py within that repo. Ticket explicitly says "Don't change the SDK" — correct, the SDK already supports both operations. Single-repo fix.

Dependencies

  • Board item #304 (basketball-api: GroupMe data model + create groups + auto-invite) — DONE. This was the original feature that introduced the script. No blocker.
  • Board item #303 (groupme-sdk) — DONE. SDK is deployed and has the needed methods.
  • Issue #156 (parent ticket) — referenced in the issue. No active dependency.
  • No items in in_progress that block this. No items that depend on this fix.

Acceptance Criteria

  • [x] "New groups created with requires_approval=false" — testable: mock create_group, assert update_group called with requires_approval=False
  • [x] "add_member calls include nickname parameter" — testable: mock add_member, assert nickname kwarg present
  • [x] "Script is idempotent (existing groups with correct settings are skipped)" — testable: the script already skips teams with existing group_id (line 105-108). The idempotency for the new update_group call needs clarification: should the script also call update_group on MATCHED (existing) groups to fix their requires_approval setting? The ticket doesn't specify this.
Note on test command: pytest tests/ -k reconciliation — no reconciliation tests exist yet. The agent will need to CREATE test files. This is expected for a bug fix adding new behavior.

Blast Radius

  • No other scripts in basketball-api use create_group or add_member — isolated fix.
  • The groupme-mcp server has its own add_member tool (at groupme_mcp/tools/members.py) which correctly requires nickname. No blast radius there.
  • The 9 groups that were manually fixed (mentioned in "What Broke") are already corrected. This fix prevents the problem on future runs.

Decomposition (5-minute rule)

  • 1 file target, 1 repo — no decomposition needed
  • 3 acceptance criteria — under threshold
  • Estimated agent work: under 5 minutes (add 2 lines to script + write test file)

Recommendation

READY with one minor refinement suggestion (non-blocking):
  • Add story label: Board item #308 is missing a story:X label. Suggest adding story:GM-1 (parent auto-join) or creating a new story tag matching the user story in the issue body.
  • Clarify idempotency scope: Should update_group also run on MATCHED (existing) groups to retroactively fix requires_approval? The 9 groups were manually fixed, but if the script is re-run after a DB reset, matched groups might still have requires_approval=true. This is a nice-to-have, not a blocker.