Fix CLI releases to use pushed tags - #1362
Conversation
|
Greptile (@greptileai) review |
|
Claude (@claude) review |
|
Claude finished Nikhil (@shadowfax92)'s task in 3m 16s —— View job Code Review
Overall this is a well-designed PR. Moving from manual Bug: heredoc indentation will break code fence rendering in release notes
The heredoc uses Fix: use run: |
cat >> /tmp/release-notes.md <<-'EOF'
## Install `browseros-cli`
### npm / npx
```bash
npx browseros-cli --help
npm install -g browseros-cli
```
EOFDead code in
|
✅ Tests passed — 1415/1419
|
Greptile SummaryThis PR migrates the CLI release workflow from a manually-triggered
Confidence Score: 5/5Safe to merge — the tag-driven trigger replaces a manual version input with a well-validated, annotated-tag gate that checks branch reachability and monotonic versioning before any build or publish step runs. The validation logic in release-policy.ts is thorough and well-tested, the legacy tag format is handled for changelog baseline selection, the repair-rerun path is explicitly covered, and the npm postinstall URL change (encodeURIComponent for the slash in cli/vX.Y.Z) is correct. The two issues noted in previous review threads (dead code guard and unquoted dist glob) are the only known gaps and neither blocks correct operation of the release. The unquoted ${CLI_DIST}/* glob in the Create GitHub release step of release-cli.yml (previously flagged) is worth addressing before the first production run to avoid silent failures if the dist directory is unexpectedly empty. Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[Developer: git tag -a cli/vX.Y.Z] --> B[git push origin cli/vX.Y.Z]
B --> C[GitHub Actions: push trigger on cli/v*]
C --> D[Checkout repo - fetch-depth 0]
D --> E[Validate release tag\nrelease-policy.ts validate]
E --> E1{Tag format\ncli/vX.Y.Z?}
E1 -- No --> FAIL1[Fail: bad tag format]
E1 -- Yes --> E2{Annotated tag?}
E2 -- No --> FAIL2[Fail: not annotated]
E2 -- Yes --> E3{Commit reachable\nfrom default branch?}
E3 -- No --> FAIL3[Fail: not on main]
E3 -- Yes --> E4{Version > CDN latest\nor same-tag repair?}
E4 -- No --> FAIL4[Fail: not incrementing]
E4 -- Yes --> E5[Emit outputs: version, tag,\nprevious_tag, target_commit]
E5 --> F[Build all platforms\nmake release VERSION=X.Y.Z]
F --> G[Upload to CDN\nwrites manifest tag=cli/vX.Y.Z]
G --> H[Generate release notes\ngit log PREV_TAG..TAG]
H --> I{GitHub release\nalready exists?}
I -- Yes repair rerun --> J[gh release edit + upload --clobber]
I -- No --> K[gh release create --verify-tag]
J --> L[npm publish\npostinstall downloads from\ngithub.com/.../cli%2FvX.Y.Z/...]
K --> L
%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
flowchart TD
A[Developer: git tag -a cli/vX.Y.Z] --> B[git push origin cli/vX.Y.Z]
B --> C[GitHub Actions: push trigger on cli/v*]
C --> D[Checkout repo - fetch-depth 0]
D --> E[Validate release tag\nrelease-policy.ts validate]
E --> E1{Tag format\ncli/vX.Y.Z?}
E1 -- No --> FAIL1[Fail: bad tag format]
E1 -- Yes --> E2{Annotated tag?}
E2 -- No --> FAIL2[Fail: not annotated]
E2 -- Yes --> E3{Commit reachable\nfrom default branch?}
E3 -- No --> FAIL3[Fail: not on main]
E3 -- Yes --> E4{Version > CDN latest\nor same-tag repair?}
E4 -- No --> FAIL4[Fail: not incrementing]
E4 -- Yes --> E5[Emit outputs: version, tag,\nprevious_tag, target_commit]
E5 --> F[Build all platforms\nmake release VERSION=X.Y.Z]
F --> G[Upload to CDN\nwrites manifest tag=cli/vX.Y.Z]
G --> H[Generate release notes\ngit log PREV_TAG..TAG]
H --> I{GitHub release\nalready exists?}
I -- Yes repair rerun --> J[gh release edit + upload --clobber]
I -- No --> K[gh release create --verify-tag]
J --> L[npm publish\npostinstall downloads from\ngithub.com/.../cli%2FvX.Y.Z/...]
K --> L
Reviews (2): Last reviewed commit: "test(cli): cover release tag safety gate..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| export function selectPreviousCliReleaseTag( | ||
| tags: string[], | ||
| currentVersion: string, | ||
| ): string { | ||
| const current = parseReleaseVersion(currentVersion) | ||
| const candidates = tags | ||
| .map(parseKnownCliTag) | ||
| .filter((tag): tag is ParsedCliReleaseTag => tag !== null) | ||
| .filter( | ||
| ({ version }) => compareReleaseVersions(version, currentVersion) < 0, | ||
| ) | ||
| .sort((a, b) => { | ||
| const versionOrder = compareReleaseVersions(b.version, a.version) | ||
| if (versionOrder !== 0) { | ||
| return versionOrder | ||
| } | ||
| return tagPriority(b.tag) - tagPriority(a.tag) | ||
| }) | ||
|
|
||
| if (candidates.length === 0) { | ||
| return '' | ||
| } | ||
|
|
||
| const selected = candidates[0] | ||
| if ( | ||
| compareVersionTuple(parseReleaseVersion(selected.version), current) >= 0 | ||
| ) { | ||
| throw new Error( | ||
| `Previous release tag ${selected.tag} is not before ${currentVersion}`, | ||
| ) | ||
| } | ||
| return selected.tag | ||
| } |
There was a problem hiding this comment.
The current variable (line 98) and the compareVersionTuple guard at lines 117–124 can never fire. The .filter at line 103 already guarantees every candidate satisfies compareReleaseVersions(version, currentVersion) < 0, so selected.version is always strictly less than currentVersion. The final check is logically dead and the current binding is only referenced there.
Rule Used: Remove unused/dead code rather than leaving it in ... (source)
Learned From
browseros-ai/BrowserOS-agent#126
Prompt To Fix With AI
This is a comment left during a code review.
Path: packages/browseros-agent/scripts/build/cli/release-policy.ts
Line: 94-126
Comment:
**Unreachable defensive check and unused variable**
The `current` variable (line 98) and the `compareVersionTuple` guard at lines 117–124 can never fire. The `.filter` at line 103 already guarantees every candidate satisfies `compareReleaseVersions(version, currentVersion) < 0`, so `selected.version` is always strictly less than `currentVersion`. The final check is logically dead and the `current` binding is only referenced there.
**Rule Used:** Remove unused/dead code rather than leaving it in ... ([source](https://app.greptile.com/browseros-org-2/-/custom-context?memory=9b045db4-2630-428c-95b7-ccf048d34547))
**Learned From**
[browseros-ai/BrowserOS-agent#126](https://github.com/browseros-ai/BrowserOS-agent/pull/126)
How can I resolve this? If you propose a fix, please make it concise.Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
| if gh release view "$TAG" >/dev/null 2>&1; then | ||
| gh release edit "$TAG" \ | ||
| --title "BrowserOS CLI - v${VERSION}" \ | ||
| --notes-file /tmp/release-notes.md | ||
| gh release upload "$TAG" ${CLI_DIST}/* --clobber | ||
| else | ||
| gh release create "$TAG" \ | ||
| --verify-tag \ | ||
| --title "BrowserOS CLI - v${VERSION}" \ | ||
| --notes-file /tmp/release-notes.md \ | ||
| ${CLI_DIST}/* | ||
| fi |
There was a problem hiding this comment.
gh sees it
Both the gh release upload and gh release create paths pass ${CLI_DIST}/* unquoted. Shell glob expansion happens before the command runs, so if the working directory is ever not the workspace root or the dist path is empty the argument becomes the literal string packages/browseros-agent/apps/cli/dist/* (unexpanded) and gh receives a non-existent path. The gh release create path has the same pattern. Quoting the glob sidesteps both failure modes.
Prompt To Fix With AI
This is a comment left during a code review.
Path: .github/workflows/release-cli.yml
Line: 145-156
Comment:
**Unquoted glob expands in shell before `gh` sees it**
Both the `gh release upload` and `gh release create` paths pass `${CLI_DIST}/*` unquoted. Shell glob expansion happens before the command runs, so if the working directory is ever not the workspace root or the dist path is empty the argument becomes the literal string `packages/browseros-agent/apps/cli/dist/*` (unexpanded) and `gh` receives a non-existent path. The `gh release create` path has the same pattern. Quoting the glob sidesteps both failure modes.
How can I resolve this? If you propose a fix, please make it concise.
`wait` means "not judged yet". The sweep deliberately re-reads wait rows so a torn verdict gets another look, and its own comment states the limit of that: "an unchanged transcript yields the same wait". The transcript of a FINISHED bee never changes, so re-reading is not re-judging - the same input gives the same answer every round, while the policy reports "N of M criteria judged SO FAR" and there is no later. Measured 2026-09-04: browseros-ai#1361 and browseros-ai#1362 sat in wait for hours, holding their boundaries and counted in `claimed` against every candidate touching them. Six hours rather than the send-back's one. A wait CAN resolve by itself - a transcript merely slow to flush parses on a later sweep - so the clock must be long enough that only a genuinely frozen one is released. `escalate` is deliberately untouched. It asks for a person, and a timer is not a person. One existing assertion covered escalate, wait and null together and now fails, correctly: the behaviour it described was changed on purpose. It was narrowed to escalate rather than deleted, because the rule it still states is the one that matters most. Tests: 6 new, 13 in the file, 147 in the queen suite, 0 failures. Closes gHashTag/trios#1408
A wait verdict on a FINISHED dispatch is re-read every round and can never change, because the transcript it would be judged from is immutable: the same 'N of M criteria judged so far' for ever, holding the dispatch's boundary paths against every candidate that touches them until the 48-hour window expires. Measured 2026-09-04: browseros-ai#1361 (finished after 289 s) and browseros-ai#1362 (after 1870 s) both sat in wait hours later. tools/frozen-wait-report.mjs names the condition and writes nothing (FR-001). Rows arrive as a caller-supplied JSON file, never a live connection, and an unreadable file is an error rather than an empty report (FR-002). Fresh, frozen and unreadable are three separate outcomes with three separate counts (FR-003); the fresh/frozen threshold is the named constant FRESH_WINDOW_MS and every run prints it (FR-004). Node standard library only (FR-005). --selftest builds a fixture with one fresh row, one frozen row, one fully-judged row (not listed - it resolves on the next sweep) and one unreadable transcript, asserts the four outcomes, and guards that an unreadable transcript is never counted as zero criteria judged; FROZEN_WAIT_DEFECT=unreadable-as-zero removes that distinction and the run fails on the guard, exit 1. classifyWaitRow is exported for reuse. Co-authored-by: Trinity Bee <bee@trinity.local>
…, and close-done kept its own copy of the rule (#360) `tri why` warned ahead: all nine remaining accepted branches conflict, so nothing can land, close-done will close nothing, and the pipeline stops as soon as those boundaries are all that is left. Four of the nine were finished work. THE ROUTE NO COMPARISON OF BYTES CAN TAKE. `isLanded` had four routes - ancestry, an identical merged tree, a patch-id match, and a hand-applied change. Every one of them compares CONTENT. They are all blind to the thing this loop does constantly: when a bee's branch goes stale, I re-cut its change against the current base and squash-merge THAT. The carry is a new commit with a new tree and a new patch-id, so nothing content-shaped connects it back, and the bee's original branch becomes permanent debt - re-offered every round, conflicting every round, holding its boundary fenced for ever. So read the message. L1 of this repository is "no code merged without `Closes #N`", which makes the message a load-bearing record and not a courtesy: browseros-ai#1310 landed as PR #330 browseros-ai#1308 landed as PR #331 browseros-ai#1362 `Closes browseros-ai#1362` in a base commit browseros-ai#1421 carried by a commit that says "Carries the browseros-ai#1421 work it belongs with" Nine conflicting branches became six. The matching is done in JavaScript rather than in git's regex, because two rounds ago a BRE read as a JavaScript regex convicted a bee - the dialect belongs somewhere it is known. AND THE TIGHTENING BROKE THE CASE IT WAS WRITTEN FOR. I required `(#N)` to end the line, to reject "unlike (browseros-ai#1421), this does X". One minute later browseros-ai#1310 stopped being recognised: a squash subject here reads `feat(queen): explain idle paid slots (browseros-ai#1310) (#330)` - the issue first, then the pull request. A trailing CHAIN of references is a subject; a parenthesis in the middle of a sentence is not. CLOSE-DONE KEPT ITS OWN COPY OF THE RULE. It had the tree test and nothing else, for weeks, while `land.mjs` grew four more routes it never learned. A rule transcribed twice is two rules that agree until somebody edits one - which is L2 of this repository, and it had happened here in the file that decides whether an issue may be closed. It asks `land.mjs` now. What remains is real debt and is reported as such: browseros-ai#1387, browseros-ai#1302 and browseros-ai#1303 carry 880 insertions of finished work outside the base, all three with their issues already CLOSED - the inverse of the false statement close-done exists to prevent. A conflict is still reported for a person and never resolved by guessing. selftest 144 pass 0 fail. Co-authored-by: Dmitrii Vasilev <trackgmedernj@hotmail.com>
1 participant
Summary
cli/vX.Y.Ztags and deriveX.Y.Zfrom the tagVerification
bun test scripts/build/cli/release-policy.test.ts scripts/build/cli/upload.test.tsbun run ./scripts/run-bun-test.ts ./scripts/buildmake testinpackages/browseros-agent/apps/cli(integration tests skipped because the local server was not reachable athttp://127.0.0.1:9105)make vetinpackages/browseros-agent/apps/climake release VERSION=0.2.3 POSTHOG_API_KEY=testinpackages/browseros-agent/apps/clibunx @biomejs/biome check scripts/build/cli/release-policy.ts scripts/build/cli/release-policy.test.ts scripts/build/cli/upload.ts scripts/build/cli/upload.test.ts apps/cli/npm/scripts/postinstall.jsDraft blocker
bun run checkfails in unrelatedapps/servertypecheck errors, includingsrc/agent/format-message.tsimplicitanyparameters andResolvedLLMConfigproperty errors insrc/api/services/chat-service.ts/src/lib/clients/llm/provider.ts.Expected release command remains:
git tag -a cli/v0.2.3 -m "browseros-cli v0.2.3" git push origin cli/v0.2.3