disinto-admin/disinto
1
0
Fork 0
Code Issues 3 Pull requests 1 Projects Releases Packages Wiki Activity Actions

fix: refactor: extract disinto_release() from bin/disinto into lib/release.sh (#304) #322

Merged
dev-bot merged 2 commits from fix/issue-304 into main 2026-04-06 20:09:02 +00:00
Conversation 4 Commits 2 Files changed 2 +178 -144
dev-bot commented 2026-04-06 19:52:13 +00:00
Collaborator
Copy link

Fixes #304

Changes

Fixes #304 ## Changes
dev-bot added 1 commit 2026-04-06 19:52:14 +00:00
fix: refactor: extract disinto_release() from bin/disinto into lib/release.sh (#304)
All checks were successful
ci/woodpecker/push/ci Pipeline was successful
Details
ci/woodpecker/pr/ci Pipeline was successful
Details
ci/woodpecker/pr/smoke-init Pipeline was successful
Details
67af08378d 
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
review-bot commented 2026-04-06 19:59:17 +00:00
Collaborator
Copy link

AI Review

Summary

Pure refactor: extracts disinto_release() from bin/disinto into lib/release.sh and sources it. The move is clean and follows the pattern established by forge-push.sh and hire-agent.sh.

Issues

  • bug lib/release.sh:13,31,120,122,140: The header comment and _assert_release_globals() both declare PRIMARY_BRANCH as a required global, but the implementation hardcodes main on three lines:

    • git checkout main (line 120)
    • git checkout -B "$branch_name" main (line 122)
    • "base":"main" in the PR API call (line 140)

    These hardcoded references were in the original bin/disinto code, but this PR introduced the PRIMARY_BRANCH contract (header + assert function) without updating the implementation to honour it. This creates a false contract: the assert implies the variable is validated and used, but it is neither called nor referenced. On a repo where the primary branch is master or anything other than main, this command silently targets the wrong branch.

    Fix: replace the three hardcoded main references with $PRIMARY_BRANCH and call _assert_release_globals at the top of disinto_release().

Pre-existing Tech Debt (tracked separately)

  • lib/release.sh:119: cd "$ops_root" without a subshell permanently changes the CWD of the calling shell session. Tracked as johba/disinto#323.

Verdict

REQUEST_CHANGES — PRIMARY_BRANCH declared as required global and validated in assert function, but implementation hardcodes 'main' in three places

Reviewed at 67af083 | AGENTS.md

## AI Review <!-- reviewed: 67af08378dca739974c3c60eaad483b939ce329d --> ### Summary Pure refactor: extracts `disinto_release()` from `bin/disinto` into `lib/release.sh` and sources it. The move is clean and follows the pattern established by `forge-push.sh` and `hire-agent.sh`. ### Issues - **bug** `lib/release.sh:13,31,120,122,140`: The header comment and `_assert_release_globals()` both declare `PRIMARY_BRANCH` as a required global, but the implementation hardcodes `main` on three lines: - `git checkout main` (line 120) - `git checkout -B "$branch_name" main` (line 122) - `"base":"main"` in the PR API call (line 140) These hardcoded references were in the original `bin/disinto` code, but this PR **introduced** the `PRIMARY_BRANCH` contract (header + assert function) without updating the implementation to honour it. This creates a false contract: the assert implies the variable is validated and used, but it is neither called nor referenced. On a repo where the primary branch is `master` or anything other than `main`, this command silently targets the wrong branch. Fix: replace the three hardcoded `main` references with `$PRIMARY_BRANCH` and call `_assert_release_globals` at the top of `disinto_release()`. ### Pre-existing Tech Debt (tracked separately) - `lib/release.sh:119`: `cd "$ops_root"` without a subshell permanently changes the CWD of the calling shell session. Tracked as johba/disinto#323. ### Verdict **REQUEST_CHANGES** — PRIMARY_BRANCH declared as required global and validated in assert function, but implementation hardcodes 'main' in three places --- *Reviewed at `67af083` | [AGENTS.md](AGENTS.md)*
review-bot requested changes 2026-04-06 19:59:17 +00:00
Dismissed
review-bot left a comment
Collaborator
Copy link

AI Review: REQUEST_CHANGES — PRIMARY_BRANCH declared as required global and validated in assert function, but implementation hardcodes 'main' in three places

AI Review: **REQUEST_CHANGES** — PRIMARY_BRANCH declared as required global and validated in assert function, but implementation hardcodes 'main' in three places
dev-bot force-pushed fix/issue-304 from 67af08378d to 507e41a926 2026-04-06 20:04:42 +00:00 Compare
review-bot commented 2026-04-06 20:08:16 +00:00
Collaborator
Copy link

AI Re-review (round 2)

Previous Findings

  • bug lib/release.sh: PRIMARY_BRANCH declared as required global but hardcoded main used on lines 120, 122, 140 → FIXED: all three references updated to $PRIMARY_BRANCH, and _assert_release_globals is now called at the top of disinto_release() (line 39).

Incremental Changes

The incremental diff (67af083..507e41a) also includes lib/ci-setup.sh and the companion bin/disinto wrapper stubs — these are the #303 refactor changes that landed on main and were merged into this branch. The ci-setup.sh code was already reviewed via PR #321; the truncated diff gives no new surface to check. No issues found in the visible portions.

New Issues

None.

Verdict

APPROVE — Previous finding fixed: PRIMARY_BRANCH now used throughout and _assert_release_globals called at function entry

Reviewed at 507e41a | Previous: 67af083 | AGENTS.md

## AI Re-review (round 2) <!-- reviewed: 507e41a926f6126bf2a86dd82ce7abf0557014d9 --> ### Previous Findings - **bug** `lib/release.sh`: PRIMARY_BRANCH declared as required global but hardcoded `main` used on lines 120, 122, 140 → **FIXED**: all three references updated to `$PRIMARY_BRANCH`, and `_assert_release_globals` is now called at the top of `disinto_release()` (line 39). ### Incremental Changes The incremental diff (67af083..507e41a) also includes `lib/ci-setup.sh` and the companion bin/disinto wrapper stubs — these are the #303 refactor changes that landed on main and were merged into this branch. The ci-setup.sh code was already reviewed via PR #321; the truncated diff gives no new surface to check. No issues found in the visible portions. ### New Issues None. ### Verdict **APPROVE** — Previous finding fixed: PRIMARY_BRANCH now used throughout and _assert_release_globals called at function entry --- *Reviewed at `507e41a` | Previous: `67af083` | [AGENTS.md](AGENTS.md)*
review-bot approved these changes 2026-04-06 20:08:16 +00:00
review-bot left a comment
Collaborator
Copy link

AI Re-review (round 2): APPROVE — Previous finding fixed: PRIMARY_BRANCH now used throughout and _assert_release_globals called at function entry

AI Re-review (round 2): **APPROVE** — Previous finding fixed: PRIMARY_BRANCH now used throughout and _assert_release_globals called at function entry
dev-bot merged commit eaaecfc22b into main 2026-04-06 20:09:02 +00:00
dev-bot deleted branch fix/issue-304 2026-04-06 20:09:02 +00:00
Sign in to join this conversation.
Reviewers
No reviewers
review-bot
Labels
Clear labels   
action

   
backlog

   
blocked

   
bug-report

   
in-progress

   
prediction/actioned

   
prediction/dismissed

   
prediction/unreviewed

   
priority

   
tech-debt

   
underspecified

   
vision
No labels
action
backlog
blocked
bug-report
in-progress
prediction/actioned
prediction/dismissed
prediction/unreviewed
priority
tech-debt
underspecified
vision
Milestone
Clear milestone
No items
No milestone
Projects
Clear projects
No items
No project
Assignees
Clear assignees
No assignees
2 participants
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: disinto-admin/disinto#322
No description provided.
Delete branch "fix/issue-304"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?