docs: compact README; split details into docs/; add release script - #146
Conversation
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
Reviewer's GuideCompact the top-level README by moving detailed usage, setup, skills, roles, memory, troubleshooting, and comparison content into new docs/* files, add a concise quick-start plus docs links, and introduce a scripts/release.sh helper to cut tagged releases safely. Flow diagram for the release scriptflowchart TD
A["scripts/release.sh vX.Y.Z"] --> B["Validate repository and tag format"]
B --> C["Require clean main synced with origin/main"]
C --> D["Run go test ./..."]
D --> E["Show changes since previous tag"]
E --> F{"Confirm release"}
F -->|yes| G["Create annotated git tag"]
G --> H["Push tag to origin"]
H --> I["CI publishes binaries, Homebrew, and npm"]
F -->|no| J["Abort"]
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
There was a problem hiding this comment.
Hey - I've found 2 issues
Prompt for AI Agents
Please address the comments from this code review:
## Individual Comments
### Comment 1
<location path="scripts/release.sh" line_range="1" />
<code_context>
+git rev-parse -q --verify "refs/tags/$TAG" >/dev/null && { echo "$TAG already exists"; exit 1; }
+
+echo ">> go test ./..."
+go test ./...
+
+prev=$(git describe --tags --abbrev=0)
</code_context>
<issue_to_address>
**issue (bug_risk):** The script does not enable `set -e`, so a failing `go test ./...` is ignored and the script continues to tag and push the release. The same missing error propagation causes failed `git tag` or `git push` commands to be followed by successful `echo` commands, allowing the script to report success despite not publishing the tag.
**Triggers:** When the test suite or either tag operation fails.
**Suggested fix:** Add `set -eu` (or explicitly check every fallible command, especially `go test`, `git tag`, and `git push`) before performing the release.
```suggestion
#!/usr/bin/env sh
set -eu
```
</issue_to_address>
### Comment 2
<location path="scripts/release.sh" line_range="13" />
<code_context>
+YES="${2:-}"
+
+[ -f .goreleaser.yaml ] || { echo "run from the dotagents repo root"; exit 1; }
+case "$TAG" in v[0-9]*.[0-9]*.[0-9]*) ;; *) echo "usage: scripts/release.sh v0.7.0 [--yes]"; exit 1 ;; esac
+confirm() {
+ [ "$YES" = "--yes" ] && return 0
</code_context>
<issue_to_address>
**issue (bug_risk):** The shell pattern `v[0-9]*.[0-9]*.[0-9]*` is not a semantic-version check: each `*` accepts arbitrary characters and the pattern also permits extra components. Values such as `v1foo.2bar.3` or `v1.2.3.4` pass validation and are then pushed as release tags, causing the release workflow or npm versioning step to reject the malformed version.
**Triggers:** When a maintainer supplies a tag that is not strictly `vMAJOR.MINOR.PATCH`.
**Suggested fix:** Validate with a stricter POSIX-compatible check, such as extracting the version and matching each numeric component explicitly, or use a dedicated semver validator.
```suggestion
case "$TAG" in v*) printf '%s\n' "${TAG#v}" | grep -Eq '^[0-9]+\.[0-9]+\.[0-9]+$' || { echo "usage: scripts/release.sh v0.7.0 [--yes]"; exit 1; } ;; *) echo "usage: scripts/release.sh v0.7.0 [--yes]"; exit 1 ;; esac
```
</issue_to_address>Sourcery assessment
Needs a human reviewer. 2 findings to address first, and the change is documentation plus a release helper that can create and push a version tag, triggering publication of binaries, Homebrew updates, and an npm release. If the script is wrong and run, those published artifacts outlive a revert, though the impact is bounded and can be corrected with a follow-up release or package retirement.
Blocking findings: scripts/release.sh:1, scripts/release.sh:13
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
| @@ -0,0 +1,41 @@ | |||
| #!/usr/bin/env sh | |||
There was a problem hiding this comment.
issue (bug_risk): The script does not enable set -e, so a failing go test ./... is ignored and the script continues to tag and push the release. The same missing error propagation causes failed git tag or git push commands to be followed by successful echo commands, allowing the script to report success despite not publishing the tag.
Triggers: When the test suite or either tag operation fails.
Suggested fix: Add set -eu (or explicitly check every fallible command, especially go test, git tag, and git push) before performing the release.
| #!/usr/bin/env sh | |
| #!/usr/bin/env sh | |
| set -eu |
| YES="${2:-}" | ||
|
|
||
| [ -f .goreleaser.yaml ] || { echo "run from the dotagents repo root"; exit 1; } | ||
| case "$TAG" in v[0-9]*.[0-9]*.[0-9]*) ;; *) echo "usage: scripts/release.sh v0.7.0 [--yes]"; exit 1 ;; esac |
There was a problem hiding this comment.
issue (bug_risk): The shell pattern v[0-9]*.[0-9]*.[0-9]* is not a semantic-version check: each * accepts arbitrary characters and the pattern also permits extra components. Values such as v1foo.2bar.3 or v1.2.3.4 pass validation and are then pushed as release tags, causing the release workflow or npm versioning step to reject the malformed version.
Triggers: When a maintainer supplies a tag that is not strictly vMAJOR.MINOR.PATCH.
Suggested fix: Validate with a stricter POSIX-compatible check, such as extracting the version and matching each numeric component explicitly, or use a dedicated semver validator.
| case "$TAG" in v[0-9]*.[0-9]*.[0-9]*) ;; *) echo "usage: scripts/release.sh v0.7.0 [--yes]"; exit 1 ;; esac | |
| case "$TAG" in v*) printf '%s\n' "${TAG#v}" | grep -Eq '^[0-9]+\.[0-9]+\.[0-9]+$' || { echo "usage: scripts/release.sh v0.7.0 [--yes]"; exit 1; } ;; *) echo "usage: scripts/release.sh v0.7.0 [--yes]"; exit 1 ;; esac |
Summary by Sourcery
Streamline the README, move detailed guidance into dedicated documentation, and add a validated release workflow entry point.
New Features:
Enhancements:
Deployment:
Documentation: