Fix all false positives, false negatives, warnings and errors in CI/CD - #288
Merged
Conversation
Adding .gitkeep for PR creation (default mode). This file will be removed when the task is complete. Issue: #287
…nd packaging Root causes found by reproducing the failing CI runs locally (issue #287): 1. Rust "Auto Release" failed with a bare `exit code 3` and no step output. `grep -Po '(?<=^name = ")[^"]*' rust/Cargo.toml` matched every section that declares a name ([package], [lib], [[bin]] and ~38 [[test]] blocks), so CRATE_NAME became a newline-joined list, the crates.io URL was malformed and curl exited 3 under `set -e`. Added scripts/rust-package-info.mjs, which parses only the [package] table, and wired it into rust.yml. curl now also retries and tolerates transient network failures instead of failing the job. 2. The npm release job reported "Failed to publish after 3 attempts" while attempt 1 had actually published. Verification ran after a single 2s sleep and lost the race with registry propagation, so the loop republished and hit EPUBLISHCONFLICT. Ported the template's publish-retry / npm-registry / publish-failure-classifier modules: a publish success or an already-published conflict now moves to bounded exponential-backoff verification and never re-enters the publish path. 3. js/package.json shipped four `../` entries in `files`, which npm silently drops, so the published tarball contained no LICENSE. Added js/LICENSE and removed the dead entries; also normalized `bin.agent` and `repository.url` so `npm publish` stops emitting fixup warnings. 4. js.yml ran 4 of 52 unit test files from a hardcoded list, hiding regressions behind a green check. It now runs the same `bun run test` suite as local development and as documented in TESTING.md. Added js/tests/ci-scripts.js covering all of the above (21 tests).
The Rust "Auto Release" job failed on main in five further runs, in the "Publish to crates.io" and "Collect changelog and bump version" steps. publish-to-crates.mjs scanned `cargo publish --verbose` output for the substrings "error: " and "error[E" *before* consulting the exit code, and did so even when cargo had exited 0. Verbose cargo output routinely contains both (dependency diagnostics, test names, doc examples), so a successful publish could be classified as failed and retried. Unlike `changeset publish`, cargo reports failure reliably through its exit code, so classification now starts there (scripts/cargo-publish-result.mjs) and text matching is used only to recognise "already uploaded" and to mark auth errors as non-retryable. The retry/verify split is shared with the npm path via publish-retry.mjs, and verification goes through a dedicated crates.io client (scripts/crates-registry.mjs) that distinguishes 404 from a registry outage. An accepted upload that the index has not exposed yet is still reported as published rather than failing a completed release. rust-version-and-commit.mjs fetched origin/main without tags, so checkTagExists() reported "not released" for a version whose tag already existed upstream; the release proceeded and `git push --tags` was rejected. It now fetches tags, pushes only the tag it created instead of every local tag, reads and rewrites the version scoped to the [package] table (via the new setPackageVersion helper) instead of matching the first `version = "..."` line in the file, and builds commit/tag messages with execFileSync so quotes, backticks and $VAR in a description cannot be interpreted by the shell. Extends js/tests/ci-scripts.js to 31 tests covering the new modules.
`eslint .` and `prettier --check .` were run from js/, so the release helper scripts in scripts/ - which the whole CI/CD pipeline depends on - were never checked. Fixing that surfaced 4 real lint violations. - add scripts/eslint.config.mjs re-exporting the js/ flat config (ESLint 9 refuses to lint files outside its config file's base path) - move .prettierrc/.prettierignore to the repo root so prettier's upward config resolution covers both js/ and scripts/ (without it prettier fell back to defaults and reported 757 false errors) - wire scripts/ into lint, lint:fix, format and format:check - extend .husky/pre-commit to cover staged scripts/*.mjs - fix the violations: unused statSync import, two useless escapes, and a needless async main()
…-safety policy
Adds js/tests/workflow-policy.js, an executable version of the rules in
hive-mind docs/CI-CD-BEST-PRACTICES.md and the pipeline templates'
docs/BEST-PRACTICES.md (mirroring their workflow-permissions.test.js and
ci-timeouts.test.js). Six of its rules failed on this repository:
- no workflow declared a top-level permissions block, so every job
inherited the repository default token scope
- none of the 17 jobs declared timeout-minutes, so a hang ran for the
GitHub Actions default of six hours
- 9 job conditions used always(), which keeps dependent jobs running
after a run is cancelled (hive-mind #1278); now ${{ !cancelled() }}
- workflow_dispatch inputs (custom_model, test_pattern, description,
bump_type) and github.head_ref were interpolated straight into run:
scripts, so a value with shell metacharacters executed as code; they
now travel through env:
- integration-tests.yml and model-tests.yml had no concurrency group
- the bun unit test script had no per-test timeout
Also fixed while auditing the two manual workflows:
- integration-tests.yml duplicated its whole job body for the single-OS
and all-OS cases, and both were named "Integration Tests (${{ matrix.os }})",
rendering as "Integration Tests ()" for the job that has no matrix.
Replaced by one matrix job fed by a resolver job.
- the MCP config check ended in '|| echo "Config not found"', so a
broken 'mcp add' still produced a green step
- integration tests ran with the pwsh default shell on windows-latest,
where the POSIX 'if [ -n ... ]' is a syntax error
- get-model-info.mjs output was appended verbatim to $GITHUB_OUTPUT;
any non key=value line fails the step with 'Invalid format'
- the model-test Summary step was skipped exactly when a test failed
…bose test The verbose-hi integration test asserted `"status": 200` from a real free-tier provider. js.yml gates the release job on this job succeeding, so a provider-side 429 or 503 reported the repository as broken and blocked a release even though verbose logging worked correctly. Extract the logging contract into tests/lib/verbose-http-log.js, which reports the observed status but never asserts it, and unit test it in tests/verbose-http-log.js against synthetic 200 and 429 logs.
Both pipelines used workflow-level `cancel-in-progress: true`, so a push to main while a release run was publishing cancelled it, potentially leaving the version bumped and tagged but never published. The release/instant-release jobs' own `release-main` group does not protect against this because workflow-level cancellation cancels the whole run. Matches the js pipeline template rule enforced in tests/workflow-reliability.test.js.
actions/checkout runs `git init` before any git config exists, so every job in every workflow logged the 'Using master as the name for the initial branch' hint (observed in run 30657021842). Setting GIT_CONFIG_COUNT/KEY/VALUE at the workflow level, as the js pipeline template does, removes it without an extra step.
Two best practices from the pipeline templates that this repository was missing (principles 7 and 11 of the hive-mind CI/CD best practices): - refs/pull/N/merge is built when the pull request is synchronized, so commits landing on the base branch afterwards were never checked: a pull request could be green while its merge result was broken. Both lint jobs now merge the base branch first via scripts/simulate-fresh-merge.sh, covered by tests/simulate-fresh-merge.js. - The js lint job now runs secretlint over the repository.
konard
changed the title
The sleep() timer was unref'd, so while a retry wait is in flight the only pending work is a timer the runtime may stop waiting on. On Bun for Windows the unit test job wedged in tests/retry-fetch.ts and burned its 20 minute timeout. Every caller awaits the sleep and the abort path clears the timer, so the #213 concern (not delaying process exit) still holds.
konard
marked this pull request as ready for review
July 31, 2026 19:51
Contributor
Author
Working session summaryThe Windows hang is resolved and the pipeline is green. Root cause of the Windows job timeout (run 30658208241): That fix exposed one more Windows-only defect: Also in this round: Verification:
PR #288 is updated and marked ready for review. One item is explicitly out of scope and stated in the PR body: the templates' 💰 Cost estimation:
📊 Context and tokens usage:Claude Opus 5: (5 sub-sessions)
Total: (19.4K new + 448.1K cache writes + 22.3M cache reads) input tokens, 159.5K output tokens, $19.721387 cost This summary was automatically extracted from the AI working session output. |
Contributor
Author
🤖 Solution Draft LogThis log file contains the complete execution trace of the AI solution draft process. 💰 Cost: $19.721387📊 Context and tokens usage:Claude Opus 5: (5 sub-sessions)
Total: (19.4K new + 448.1K cache writes + 22.3M cache reads) input tokens, 159.5K output tokens, $19.721387 cost 🤖 Models used:
📎 Log file uploaded as Gist (7652KB)Now working session is ended, feel free to review and add any feedback on the solution draft. |
Contributor
Author
🎉 Auto-mergedThis pull request has been automatically merged by hive-mind.
Auto-merged by hive-mind with --auto-merge flag |
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 #287.
Audit of every CI/CD workflow and helper script in this repository against the two pipeline templates (js, rust) and the hive-mind CI/CD best practices. Every defect below is covered by a test that fails before the fix.
False negatives — CI was green while something was broken
npm publishverification did not wait for the registryrefs/pull/N/mergeis built at synchronize timescripts/was linted and formatted by neither the hook nor CIRunning the full suite on all three matrix platforms surfaced two real defects that the four-file list had hidden:
sleep()insrc/provider/retry-fetch.tsunref'd its retry timertests/retry-fetch.tsand burned the whole 20 minute timeout (run 30658208241); with the timer referenced the same suite finishes in ~2 minutestests/storage-migration.tsasserted a literal POSIX pathpath.resolvereturnsD:\workspace\...on Windows, so the assertion could only ever hold on the two POSIX runnersFalse positives — CI was red or a release was blocked while nothing was wrong
verbose-hi.jsasserted a literal HTTP200from a real free-tier providerreleaseneeds it, so a rate limit blocked releases (run 24122902674)grepcargo publishfailure classifier treated "already published" as a failurecat ~/.config/opencode/opencode.json || echo "Config not found"Warnings and errors
hint: Using 'master' as the name for the initial branch, becauseactions/checkoutrunsgit initbefore any config exists (run 30657021842). Fixed with theGIT_CONFIG_*env the js template uses.js/package.jsonpackaging metadata produced npm warnings and omitted files from the tarball.$GITHUB_OUTPUT, which fails the step withInvalid format.Security and reliability policy applied to all four workflows
permissions: contents: read; publishing jobs escalate individually.timeout-minuteson all 17 jobs that lacked one.always()→${{ !cancelled() }}in 9 job conditions, so cancelling a run stops the job graph (hive-mind #1278).workflow_dispatchinputs andgithub.head_refare passed throughenv:instead of being interpolated intorun:scripts, where they would execute as shell code.concurrencygroups on the two workflows that had none, andcancel-in-progress: ${{ github.ref != 'refs/heads/main' }}so a push to main can no longer cancel an in-flight publish and leave a version tagged but unpublished.bun test --timeout 30000, so a hung test reports before burning the job timeout.actions/cache@v4→@v5,peter-evans/create-pull-request@v7→@v8), with a policy test rejecting floating refs such as@main.integration-tests.ymlgaineddefaults.run.shell: bash(windows-latest defaults to pwsh) and a matrix resolver job;model-tests.ymlgained the missing jobname.Tests
New, all failing before their fix:
js/tests/workflow-policy.js— 12 executable policy rules (permissions, timeouts, cancellation, input interpolation, concurrency, main-cancellation, default-branch env, fresh merge, secret scan). 6 of the original 7 rules failed when written: 4 workflows without permissions, 17 jobs without timeouts, 9always()conditions, 9 input interpolations, 2 workflows without concurrency.js/tests/verbose-http-log.js+js/tests/lib/verbose-http-log.js— the verbose logging contract, including "still passes when the provider rate limits the request".js/tests/simulate-fresh-merge.js— merge simulation, including the conflict path.js/tests/ci-scripts.js— the Rust/npm publish helpers.js/tests/retry-fetch-wait.js— runs a rate limit wait in a fresh process, where nothing else keeps the event loop alive.js/tests/workflow-policy.jsis now 13 rules (the 13th rejects unpinned actions).Local verification:
npm run checkclean,bun run test→ 684 tests, 0 failures. CI on this branch: JS and Rust pipelines both green (runs 30660339994 and 30660339979), including all three unit test platforms.Deliberately out of scope: the templates' documentation link checker (
lycheeverse/lychee-action). It is an added check rather than a fix for an existing false result, and it would need a separate pass over the repository's docs; it is not part of this pull request.Reported upstream
The same defects exist in the templates and the best-practices document, as the issue asks:
github.head_refand the free-formdescriptioninput interpolated intorun:scripts; two workflows missing the default-branch env.descriptioninput interpolated intorun:in the crates.io publishing job.cancel-in-progresson main, which the js template's own test explicitly forbids; following it verbatim is what produced the release-cancellation defect fixed here.