Repository navigation
fix(swarm): post-0.5.0 review fixes — merge winner, in-flight validate, timeouts, port ranges - #28
Merged
Merged
Conversation
…validate, parse timeouts first Fixes from the review of the 0.4 roadmap work (#21), all still present on 0.5.0: - Merge winner took "finished" from the compare table. A marker finish is never cleared, so an agent given a follow-up was skipped and, on a clean worktree, archived (which kills it). It now runs settle after the merge and re-reads compare; a slot is skipped only if still finished with the tip and dirty count the operator saw. It also checks base drift against the base at confirm: re-reading it first made the check compare new to new. - Archive, discard and abort refuse while a slot's validate hook runs. The hook runs with the repo lock released and could write files after the cleanup inventory or discard snapshot. - with_timeout / with_group_timeout parse their duration before forking. "2m" or "0900" errored after the child started, killing the caller and orphaning cp or the validate hook with no deadline. Fan-out refuses a bad clone or setup timeout before any worktree exists. - Per-repo port ranges via git config swarm.portStart/portSpan. The env-only override never reaches an action-invoked pane, so two repos collided on 4100 with no way out. (The plugin config dir would not fix it: it is shared by every repo.) - validate exits 36, not 30, when the slot moved during the hook; 30 means the base moved. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…e candidate gate Second pass on the post-0.5.0 review fixes. - Compare kept its own copy of "is this Swarm's validation record for this run and slot". It had drifted from the strict candidate gate's copy: it did not require schema_version 1 or the hook's own `validate` check, so compare could rank `passed` a record the handoff then rejects. Both now call isSwarmValidationRecord from pr-handoff.mjs; a test pins compare to the import. - A timed-out `validate` in the harvest pane named HERDR_SWARM_STEP_TIMEOUT_MS, but validate's budget follows HERDR_SWARM_VALIDATE_TIMEOUT. The banner now names the right knob. - Tests: the renderer's GIT_ROUTING list is held in lockstep with lib.sh clear_git_routing_env; the bin/ read-only-git check now also covers direct spawn/exec of git; compare ranking ties are asserted (commits > finished > neither, never diff size). - CONCEPTS gains Finished, Finish marker, Compared tip and Validation result, and notes the two meanings of "settle". README, CHANGELOG and the residual review findings are brought in line. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info
📝 Walkthrough
Merge Risk: ⚪ Minimal · up to
|
| Check name | Status | Explanation | Resolution |
|---|---|---|---|
| Docstring Coverage |  |
Docstring coverage is 55.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 20 functions across 14 files. (5 skipped:… | Write docstrings for the functions missing them to satisfy the coverage threshold. |
-
Check name Status Explanation Description Check Check skipped - CodeRabbit’s high-level summary is enabled. Title check The title clearly summarizes the main changes: merge-winner fixes, in-flight validation protection, timeout handling, and port-range configuration. Linked Issues check Check skipped because no linked issues were found for this pull request. Out of Scope Changes check Check skipped because no linked issues were found for this pull request.
Full details: Docstring Coverage
-
Explanation
Docstring coverage is 55.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 20 functions across 14 files. (5 skipped: 5 unsupported.)
- Fix all pre-merge checks with AI
✨ Finishing Touches: 💡 1
-
📝 Generate docstrings: 💡
-
- Commit to this branch
- Create a new PR
🧪 Generate unit tests (beta)
-
- Commit to this branch
- Create a new PR
-
- Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.
Comment @coderabbitai help to get the list of available commands.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes from the review of the 0.4 roadmap work (#21) that were still present in 0.5.0. This work was done on Oct 6 and stayed on this machine until now. There's no behavior change beyond what's listed here.
Safety
with_timeout/with_group_timeoutparse their duration before forking.2mor0900used to error after the child had started, which killed the caller and leftcpor the validate hook running with no deadline. Fan-out now refuses a bad clone or setup timeout before any worktree exists.Correctness
git config swarm.portStart/swarm.portSpan. The env-only override never reaches a pane opened by a plugin action, so two repos collided on 4100.validateexits 36, not 30, when the slot moved while the hook ran. 30 means the base moved.isSwarmValidationRecord). Compare's copy had drifted and could rankpasseda record the handoff rejects.HERDR_SWARM_VALIDATE_TIMEOUT, not the generic step timeout.Tests and docs
lib.sh; directgitspawns inbin/must be read-only; compare ranking ties.[Unreleased], and residual review findings addenda.Verification
npm run validateon macOS: manifest and source checks,shellcheck -x scripts/*.shclean,node --test371 passed, 0 failed, 1 skipped (the Linux-only copy-on-write test, skipped on macOS by design).bash -n scripts/*.shclean. CI covers ubuntu.🤖 Generated with Claude Code
Summary by CodeRabbit