Harden release CI retries and setup tests#129
Conversation
|
Important Review skippedAuto incremental reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughThe release workflow adds retry and idempotent creation helpers for GitHub releases, applies them to asset uploads and note edits, and updates structural tests. SQLite fixture setup now uses transactions and guaranteed cleanup. ChangesGitHub release reliability
SQLite fixture cleanup
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant GitHubActions
participant ghRetry
participant GitHubReleases
GitHubActions->>ghRetry: Create or inspect release
ghRetry->>GitHubReleases: Execute retried gh command
GitHubReleases-->>ghRetry: Release status
GitHubActions->>ghRetry: Upload assets and edit notes
ghRetry->>GitHubReleases: Execute retried mutations
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
There was a problem hiding this comment.
Pull request overview
This PR improves release reliability and test stability in the Rosetta codebase by hardening the GitHub Release publishing step against transient GitHub API/CLI failures and by speeding up the fixture DB writes used in timeout-sensitive setup tests.
Changes:
- Add bounded retry + exponential backoff wrappers around
gh release uploadandgh release edit, and make release creation idempotent viaensure_release_exists()inrelease.yml. - Split release creation from asset upload/edit so partial release creation can be recovered within the same run or via rerun.
- Wrap
setup.test.tsfixture inserts in a single transaction and explicitly finalize prepared statements to avoid per-insert autocommit sync overhead.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
src/setup.test.ts |
Speeds up fixture DB creation by batching inserts in one transaction and finalizing statements, reducing timeout risk. |
src/release.test.ts |
Extends workflow-contract tests to assert the new retry/idempotency helpers are present in release.yml. |
.github/workflows/release.yml |
Adds gh_retry() and ensure_release_exists() to make GitHub Release creation/upload/edit resilient to transient failures and reruns. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/release.yml:
- Around line 667-673: The gh_retry() loop currently captures the exit status
after the conditional, so a failed upload or edit can return success; capture
the command’s status inside the else branch before any subsequent commands and
return it when max_attempts is reached. In src/release.test.ts lines 804-806,
add a behavioral assertion verifying the helper returns the failing exit code,
in addition to the existing helper-string assertions.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 747109a5-f1fa-4d2e-95c6-335812c530c3
📒 Files selected for processing (3)
.github/workflows/release.ymlsrc/release.test.tssrc/setup.test.ts
📜 Review details
⏰ Context from checks skipped due to timeout. (1)
- GitHub Check: copilot-pull-request-reviewer
🧰 Additional context used
📓 Path-based instructions (3)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Use Bun and TypeScript, including Bun-native runtime APIs such as
bun:sqlite,Bun.serve, andbunx.Rosetta uses Bun and TypeScript; prefer
bun,bun test, andmake verifywhere applicable, rather than Node/npm-oriented substitutes.
Files:
src/setup.test.tssrc/release.test.ts
src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Rosetta is read-only documentation/schema context and must not connect to or modify a user's router.
Files:
src/setup.test.tssrc/release.test.ts
**/*.{ts,tsx,md}
📄 CodeRabbit inference engine (AGENTS.md)
Keep the attribution boundary visible when community RouterOS skills surface, because they are supplemental rather than official MikroTik documentation.
Files:
src/setup.test.tssrc/release.test.ts
🧠 Learnings (8)
📚 Learning: 2026-07-12T20:33:29.883Z
Learnt from: mobileskyfi
Repo: tikoci/rosetta PR: 52
File: .github/workflows/qa.yml:101-101
Timestamp: 2026-07-12T20:33:29.883Z
Learning: In tikoci/rosetta, GitHub Actions workflow steps that reference official actions (e.g., `actions/checkout`, `actions/download-artifact`) may intentionally use version tags (e.g., `v4`, `v3`) instead of full commit SHAs. During code review, do not flag these unpinned `actions/*` tag references as a GitHub Actions security issue for this repository, because Dependabot’s `github_actions` updates tagged refs and GitHub surfaces action security advisories against tagged refs.
Applied to files:
.github/workflows/release.yml
📚 Learning: 2026-07-12T20:33:35.489Z
Learnt from: mobileskyfi
Repo: tikoci/rosetta PR: 52
File: .github/workflows/codeql.yml:29-29
Timestamp: 2026-07-12T20:33:35.489Z
Learning: In this repo’s GitHub Actions workflows, preserve the maintainer’s security/maintenance intent: action references under `actions/*` may use version tags (e.g., `actions/checkoutv7`) instead of full commit SHAs, and `actions/checkout` may intentionally keep `persist-credentials: true`. During review, do not raise issues suggesting SHA pinning for `actions/*` or proposing `persist-credentials: false` unless the project adopts an org-wide policy that enforces SHA pinning.
Applied to files:
.github/workflows/release.yml
📚 Learning: 2026-07-12T20:33:29.883Z
Learnt from: mobileskyfi
Repo: tikoci/rosetta PR: 52
File: .github/workflows/qa.yml:101-101
Timestamp: 2026-07-12T20:33:29.883Z
Learning: In this repository’s GitHub Actions workflows (under .github/workflows), do NOT flag actions/checkout steps for missing `persist-credentials: false` when the workflow is intentionally relying on the persisted GitHub token for later authenticated steps in the same job (e.g., release or subsequent steps that require auth). This exception applies to workflows where the checked-out workspace is not being uploaded as an artifact and the token persistence is an intentional part of the job’s authentication flow.
Applied to files:
.github/workflows/release.yml
📚 Learning: 2026-07-12T20:33:31.549Z
Learnt from: mobileskyfi
Repo: tikoci/rosetta PR: 52
File: .github/workflows/release.yml:433-433
Timestamp: 2026-07-12T20:33:31.549Z
Learning: In this repo, GitHub Actions used in workflow files should be pinned to version tags (e.g., actions/checkoutvX, not full commit SHAs). During code review, do not flag or fail checks that look like “unpinned-uses”/“SHA pinning required” (e.g., zizmor unpinned-uses) for actions/checkout, actions/download-artifact, or other third-party actions referenced from `.github/workflows`—keep the existing tag-pinning policy so Dependabot’s `github_actions` tracking and GitHub’s action security advisory visibility against tagged refs are preserved.
Applied to files:
.github/workflows/release.yml
📚 Learning: 2026-07-12T20:33:42.621Z
Learnt from: mobileskyfi
Repo: tikoci/rosetta PR: 52
File: .github/workflows/test.yml:19-19
Timestamp: 2026-07-12T20:33:42.621Z
Learning: In tikoci/rosetta workflows, follow the maintainer policy: do not require GitHub Actions step refs under `actions/*` (e.g., `actions/checkout`) to be pinned to full commit SHAs, and do not require `persist-credentials: false` to be set on `actions/checkout` (or other `actions/*`) by default. Only flag missing SHA pinning or missing `persist-credentials: false` if the maintainer explicitly indicates the policy has changed (for example, adopting an org-wide SHA-pinning enforcement).
Applied to files:
.github/workflows/release.yml
📚 Learning: 2026-07-12T20:33:36.397Z
Learnt from: mobileskyfi
Repo: tikoci/rosetta PR: 52
File: .github/workflows/release.yml:74-74
Timestamp: 2026-07-12T20:33:36.397Z
Learning: For this repo (tikoci/rosetta), workflow files in .github/workflows should keep GitHub Actions `actions/*` pinned to version tags (e.g., `v3`, `v4`) rather than full commit SHAs. Do not raise/retain findings that would otherwise complain about unpinned uses (e.g., zizmor `unpinned-uses`) or SHA pinning, unless the maintainer later enables an org-wide enforcement policy requiring SHA pinning.
Applied to files:
.github/workflows/release.yml
📚 Learning: 2026-07-12T20:33:31.549Z
Learnt from: mobileskyfi
Repo: tikoci/rosetta PR: 52
File: .github/workflows/release.yml:433-433
Timestamp: 2026-07-12T20:33:31.549Z
Learning: In tikoci/rosetta release-related GitHub Actions workflows, do not treat the default `actions/checkout` behavior for `persist-credentials` (i.e., not explicitly setting `persist-credentials: false`) as a security issue when the checked-out workspace is not uploaded as an artifact. The jobs rely on the persisted token for later authenticated steps within the same job. Only raise/require changing `persist-credentials` if the workflow uploads the checked-out workspace (or otherwise packages the token-bearing credential context) as an artifact, making the persisted credentials usable outside the job.
Applied to files:
.github/workflows/release.yml
📚 Learning: 2026-07-12T20:33:36.397Z
Learnt from: mobileskyfi
Repo: tikoci/rosetta PR: 52
File: .github/workflows/release.yml:74-74
Timestamp: 2026-07-12T20:33:36.397Z
Learning: In this repo’s release-related GitHub Actions workflows, do not flag `actions/checkout` steps for missing `persist-credentials: false`. The maintainer intentionally relies on the default `persist-credentials: true` because the job does not upload the checked-out workspace as an artifact and later authenticated steps in the same job require the persisted token. Only require changing `persist-credentials` if the job structure does not rely on the persisted credentials for subsequent steps.
Applied to files:
.github/workflows/release.yml
🔇 Additional comments (1)
src/setup.test.ts (1)
37-66: LGTM!
Opus ReviewThird-eye review of PR #129 — verdict: sound, both failures correctly addressed I reviewed the diff, reproduced the test root cause locally, and re-ran all validation. Both distinct failures are handled correctly, and neither fix masks a signal. Failure 1 — release HTTP 500 on gh release create This is not a code bug — it's a transient GitHub API 5xx after the OCI images were already pushed. There is no rosetta-side root cause to fix; the correct mitigation is retry + idempotency, which is exactly what landed:
Failure 2 — test timeout in waitForUsableDb / probeDb closes statements… The PR blames 1,100 autocommit fsyncs in writeUsableDb. I reproduced it (per the grounding rule — didn't take CI's word): ┌──────────────────────────┬───────────────────┐ A 200× gap on fast local disk. Both failing tests call writeUsableDb, and both already carried 15 s timeouts (issue #32) — so this area was known-flaky and the write was the hot spot. On a contended shared runner where fsync is many times slower, 855 ms plausibly blows past 15 s. The fix (BEGIN/COMMIT batch + explicit finalize() in a try/finally) genuinely removes the cost, keeps every assertion (still 100 pages / 1000 commands / release_tag), and is not a skip/disable. This is a real fix, not a mask — consistent with your grounding-and-masking policy. Validation (re-run here) bun test src/release.test.ts src/setup.test.ts → 164 pass / 0 fail · bun tsc --noEmit clean · actionlint release.yml exit 0 · all 8 PR checks green · mergeStateStatus: CLEAN, mergeable: MERGEABLE · the one CodeRabbit thread is resolved. Minor, non-blocking nits
None affect correctness. I'd approve/merge as-is. One thing worth flagging for your judgment: failure #1 is inherently a single-data-point transient (one 500). The retry/idempotency here is the right defensive posture regardless, so it's not the "generalize one failure into a fact" trap — but if a gh release create 500 recurs frequently, that'd be worth raising with GitHub rather than absorbing silently. |
What changed
gh release create,gh release upload, andgh release editin the Release workflow.setup.test.tsusable-DB fixture inserts in one transaction and finalize prepared statements explicitly, avoiding 1,100 autocommit disk syncs in timeout-sensitive lock/probe tests.src/release.test.ts.Root cause
Two distinct failures were present:
publishatCreate or update GitHub ReleasewithHTTP 500 (https://api.github.com/repos/tikoci/rosetta/releases). Build and reusable QA had already passed, and OCI images had already been pushed. There were nov0.11.2-alpha.107GitHub Release/tag or npm package after the failure.Run tests (fixture DB). The failing tests weredownload lock helpers > waitForUsableDb returns true when another process finishes the DBandprobeDb > closes statements so a probed temp DB can be renamed immediately, both timing out before artifact download or DB-content gates. The fixture helper wrote 100 pages and 1,000 commands as separate autocommit transactions, which can stall long enough on a shared runner to trip Bun's per-test timeout.PR #128 is related because it placed this QA path directly in the release pre-publish graph, but the observed failures were not CLI-Reference content or link-drift regressions.
Validation
bun run typecheckmake lintbun test src/release.test.ts src/setup.test.tsbun testactionlint .github/workflows/release.ymlNote:
actionlint .github/workflows/release.yml .github/workflows/qa.ymlalso reports an existing ShellCheck SC2129 style warning inqa.yml; this PR does not touch that unrelated warning.Summary by CodeRabbit
Bug Fixes
Tests