ci: eliminate the false positives, false negatives and warnings across all four pipelines - #42
Merged
Conversation
Adding .gitkeep for PR creation (default mode). This file will be removed when the task is complete. Issue: #41
The C# release job for 0.2.0 failed with 'is not on NuGet after publish' while the push had in fact succeeded: the inline verification polled the flat-container API six times over sixty seconds, and NuGet made the package visible about six and a half minutes later. NuGet documents package validation and indexing as taking up to fifteen minutes, so the budget was fifteen times too small and the red run was a false negative. scripts/wait-for-registry.mjs replaces the hand-rolled curl loops with one readback that knows each registry's own indexing window, tells slow indexing apart from a failed publish in its error message, and has a verbose mode (off by default) that prints every probe. A live probe of all four registries caught a second false negative in the readback itself: crates.io enforces an API data access policy that answers 403 to clients without a descriptive User-Agent, so an anonymous check would report a published crate as missing. Probes now identify themselves.
Auto-release stops failing on a publish that is merely still indexing, and manual-release gains the verification it never had.
python/scripts/ holds the code that performs the release, yet ruff only ever saw src and tests. Bringing it into scope surfaced two deprecated Optional annotations and seven files formatted to a line length the project does not use; both are fixed here so the wider scope starts green.
The 0.2.0 release failed with 'invalid-publisher: valid token, but no corresponding publisher'. https://pypi.org/pypi/lino-objects-codec/json answers 404, so the project has never been published, and PyPI needs a *pending* publisher registered before the first upload of a project that does not exist yet. That is a one-time action in PyPI's settings, so the repository cannot fix it — but it can stop hiding it. A preflight now prints the four claims PyPI will be asked to match and the steps to register them, and a failure handler repeats them with the claims dumped. Both report without gating, because a correctly registered pending publisher looks exactly like none at all from outside until the first upload lands. A PYPI_API_TOKEN secret, when set, carries the upload instead of OIDC so a first release is possible before trusted publishing is registered. The PyPI readback also moves off its twenty-five second budget onto the shared registry wait.
Rust was the only language that published without reading the result back, so a rejected upload would still have produced a green run, a tag and a release for a version nobody can depend on. The shared scripts/ helpers had no workflow of their own either: only the pull-request-only parity job ran a test, and only one file of it.
- checkout v4->v7, setup-node v4->v7, setup-python v5->v7, setup-dotnet v4->v6, upload-artifact v4->v7, download-artifact v4->v8, cache v4->v6, codecov-action v4->v7, create-pull-request v7->v8. - codecov v5 renamed 'file' to 'files'; rename the input and pass CODECOV_TOKEN so protected-branch uploads stop warning.
- csproj: PackageReadmeFile + packed README.md removes 'warn : Readme missing'
from 'dotnet pack'.
- pyproject: replace the deprecated 'license = {file = ...}' TOML table with the
PEP 639 SPDX expression plus 'license-files', and add python/LICENSE so
setuptools stops warning that the file cannot be found.
…nings _encodeValue (complexity 23) and _decodeLink (complexity 55, 95 statements) tripped the repository's own complexity/max-statements rules on every CI run. Extract the memoisation, scalar and collection handling into focused helpers so the two entry points become plain dispatch. Pure refactor: no behaviour or API change, all 244 tests and the example still pass, and 'eslint .' is now clean. This touches js/src/ only, so the pull request carries [skip-parity]: the other three codecs are unchanged because their linters report no such warning.
Applies best practices #7/#10 from the org CI/CD guide to all four language pipelines: - top-level 'permissions: contents: read'; release jobs keep their own elevated block, so nothing else gets a writable token by default; - 'timeout-minutes' on every job, so a hung runner fails instead of burning the 6h default; - job-level concurrency replaces the workflow-level cancellable group. A workflow-level 'cancel-in-progress: true' could cancel a release that had already started pushing a tag or a package. Read-only checks now cancel per job (matrix entries stay independent); every writer shares the repo-scoped 'main-writer-${{ github.repository }}' group with 'cancel-in-progress: false' and 'queue: max', so releases across the four languages queue instead of racing for the same branch; - 'always() && !cancelled()' collapses to '!cancelled()' (a bare always() keeps downstream work running after cancellation), with the stale comments updated; - 'github.head_ref' is passed through an environment variable instead of being interpolated into a shell script (actionlint 'expression' rule).
A malformed workflow file is a silent false negative: GitHub refuses to run it and attaches no check, so the pull request still looks green while a pipeline has stopped running. actionlint parses all six workflows on every PR and also flags shell injection through untrusted contexts.
Two whole classes of check were missing, so every pull request was green on questions nobody was asking (best practices #11 and #12): - security.yml: CodeQL over javascript-typescript, python, csharp and the workflow files themselves; dependency-review on pull requests; and a lockfile audit per ecosystem (npm, cargo-audit, pip-audit). No paths: filter and a weekly schedule, so newly published advisories surface without a code change. - links.yml: lychee over every Markdown file, excluding docs/case-studies and dev/log, which quote other repositories' issues and runs as evidence. All five audits were run locally against this tree first: npm audit, cargo audit and pip-audit report no vulnerabilities.
…heck A local run of the new lychee gate failed on links this repository cannot fix: the './**/*.md' glob picked up third-party READMEs under js/node_modules, and www.npmjs.com answers every automated request with 403 (verified with and without a browser User-Agent), so the package badges in README.md and js/README.md were reported broken. Adds .lycheeignore for the npmjs.com host and --exclude-path js/node_modules. Local run is now 107 OK / 0 errors.
The refs/pull/N/merge preview GitHub checks out is built when the PR is opened or synced; once the base branch moves it is stale, so lint, test and build validated code that is not what will land. Adds scripts/simulate-fresh-merge.sh (with an off-by-default VERBOSE mode that lists the commits the base is ahead by) and calls it from the read-only check jobs of all four language workflows, with fetch-depth: 0 so the base branch history is available. Verified: actionlint clean, all workflows parse, script runs locally.
csharp/scripts/*.test.mjs existed but no workflow ever executed them, so a broken release helper would only have surfaced during a real release. The C# sources also had no line-count ceiling, unlike Rust (scripts/check-file-size.mjs), Python (scripts/check_file_size.py) and JavaScript (ESLint max-lines). Ports the template's check-file-size.mjs with the repository-wide 1500-line limit (best practice #2 range, matching the ESLint rule) plus its test suite converted to node:test, and adds both steps to the C# lint job. Verified locally: 10/10 script tests pass, file-size check exits 0.
Nothing scanned this repository for credentials. Adds a gitleaks job that checks both the working tree and the full git history, using the pinned MIT-licensed CLI rather than gitleaks-action, which requires a paid licence for organisation-owned repositories. An unconfigured run reports 7 findings, all false positives: the generic-api-key rule matches the "key: <hex>" lines that actions/cache prints in the CI logs archived under docs/case-studies and dev/log. .gitleaks.toml allowlists those evidence directories with the reasoning recorded inline, so the scan stays actionable. Verified locally: 0 findings on the tree and across 146 commits.
Both checks were false negatives. python.yml and csharp.yml each carried an inline shell copy that printed "::warning::No changelog fragment found" / "::warning::No changeset found" and then exited 0, so a PR could change python/src or csharp/src with no release note and still show a green check. Both also counted every file in changelog.d/ and .changeset/, so a leftover fragment from an earlier unreleased PR satisfied the check for a new PR that added nothing. Both now decide from the PR diff and exit 1, matching the Rust and JavaScript implementations: - csharp/scripts/check-changeset.mjs (new, with unit tests) - python/scripts/validate_changeset.py (existed but no workflow ever ran it; extended with the diff-based requirement, with unit tests) Both use git diff --relative, without which the monorepo path prefix silently disables the check (issue #39), and both pass github.base_ref/head_ref through the environment instead of interpolating them into shell. Verified locally: 7/7 C# script tests, 9/9 new Python tests, 186 Python tests total, ruff and mypy clean, actionlint clean.
Python was the only implementation tested on a single operating system; JavaScript, Rust and C# each run a three-OS matrix. A Windows-only defect (path separators, line endings, locale-dependent encoding) would have shipped to PyPI unnoticed. Coverage is uploaded from the ubuntu leg only. Note: the job name changes from "Test (Python 3.13)" to "Test (Python 3.13 on <os>)", so the required-check list in branch protection needs updating.
`git diff --name-only` prints paths relative to the repository root, but `detect-code-changes.mjs` runs with `working-directory: ./js` (and ./rust) and compared those paths against package-relative prefixes. In this monorepo the real paths are `js/examples/demo.mjs`, not `examples/demo.mjs`, so: * the documented `examples/`, `experiments/`, `docs/` and `.changeset/` exclusions never matched -- an examples-only pull request was reported as a code change and asked for a changeset it does not need (false positive); * `package-changed` could never become true (false negative); * Rust's `toml-changed` fired on any .toml in the repository, including `python/pyproject.toml`. This is the same class of defect as issue #39. The prefix is now resolved at run time with `git rev-parse --show-prefix`, which keeps the scripts correct in a single-package checkout too, where the prefix is empty. experiments/detect-code-changes-monorepo-paths.sh reproduces the original behaviour end to end; the new unit tests pin it down per case. The js lint job now runs `node --test scripts/*.test.mjs`, which nothing did before.
All four organisation templates ship a `detect-changes` job; this repository had one for JavaScript and Rust only, so a documentation-only pull request ran the full .NET and Python matrices. The new scripts follow the fixed package-relative semantics from the previous commit and ship with unit tests. The workflow-level `paths:` filter decides whether the workflow runs at all; `detect-changes` decides which jobs inside it are worth running. `!cancelled()` guards every gated job because `detect-changes` is skipped on `workflow_dispatch`.
This was referenced Aug 20, 2026
Reading the templates in full disproved two of the three hypothesised upstream defects: tpl-rust does verify the crates.io publish (wait-for-crate.rs, called from release.yml:632) and tpl-python's PyPI smoke test retries 6 x 20 s and fails the release. Both claims are withdrawn in place rather than deleted. The defect that is real is the one this pull request just fixed downstream: the JavaScript and Rust templates match their change-detection exclusion lists against repository-root paths, so nothing is excluded in the multi-language layout their own path helpers support. Reported with reproductions: link-foundation/js-ai-driven-development-pipeline-template#141 link-foundation/rust-ai-driven-development-pipeline-template#139
konard
changed the title
|
You are seeing this message because GitHub Code Scanning has recently been set up for this repository, or this pull request contains the workflow file for the Code Scanning tool. What Enabling Code Scanning Means:
For more information about GitHub Code Scanning, check out the documentation. |
The pushed branch's Lint Workflows job failed with nine SC2086 reports for unquoted `>> $GITHUB_OUTPUT`. Local actionlint had exited 0 because actionlint only runs shellcheck when the binary is on PATH and skips that analysis silently otherwise -- a green local lint that was itself a false negative. Quotes all 10 such redirections across csharp.yml, python.yml and rust.yml, not only the 4 lines the linter pointed at. Verified with `actionlint -shellcheck <path>`, which now exits 0.
The dependency-review job failed with "Dependency review is not supported on this repository. Please ensure that Dependency graph is enabled" -- a repository setting a pull request cannot change, leaving a check that could only ever fail. It is removed rather than marked continue-on-error, since an ignored failure is the same false negative issue #41 is about. Its only unique coverage was NuGet, which is replaced by `dotnet list package --vulnerable --include-transitive`, so all four ecosystems are audited again. That command exits 0 even when it finds advisories, so the job inspects the report instead of the exit code. Both paths verified locally.
konard
marked this pull request as ready for review
August 20, 2026 09:10
Member
Author
Working session summaryDone. PR #42 is green across all nine workflows and marked ready for review: #42 This session's work, on top of what was already committed: Two further defects, both found only by CI itself:
Verification — all nine workflows pass on Both new findings are recorded as §9.5 and §9.7 in Two things need a human, neither blocking the merge:
One caveat on the parity gate: it fails on this branch and the PR body carries This summary was automatically extracted from the AI working session output. |
Member
Author
🤖 Solution Draft LogThis log file contains the complete execution trace of the AI solution draft process. 💰 Cost: $27.192996📊 Context and tokens usage:Claude Opus 5: (6 sub-sessions)
Total: (11.4K new + 538.5K cache writes + 30.2M cache reads) input tokens, 262.8K output tokens, $27.126745 cost Claude Haiku 4.5:
Total: 41.0K input tokens, 1.1K output tokens, $0.066251 cost 🤖 Models used:
📎 Log file uploaded as Gist (10046KB)Now working session is ended, feel free to review and add any feedback on the solution draft. |
Member
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
2 participants
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.
Closes #41.
[skip-parity]— see "Why the parity gate is skipped" at the bottom.What this pull request does
Issue #41 asked for all false positives, false negatives, warnings and
errors in CI/CD to be found and fixed, for the four pipelines here to be
brought in line with the org's language templates and with
CI-CD-BEST-PRACTICES.md,
and for any defect that also exists upstream to be reported there.
The full evidence trail, timeline, per-requirement root-cause analysis and
execution record live in
dev/log/issues/41/pulls/42/ANALYSIS.md.The headline bug: change detection classified every path wrongly
git diff --name-onlyprints paths relative to the repository root, butscripts/detect-code-changes.mjsruns withworking-directory: ./jsandcompared those paths against package-relative prefixes (
examples/,docs/,package.json). In this monorepo the real paths arejs/examples/..., so:js/examples/demo.mjsproducedany-code-changed=true, because none of the documented exclusions couldever match.
package-changedcould never betrue, because thepath is
js/package.json, notpackage.json.This is the same class of defect as #39. Reproduction, runnable end to end
against a scratch repository:
Before the fix it reports
any-code-changed=trueand listsjs/examples/demo.mjsas a code change; after the fix it printsPackage prefix: js/,Files considered as code changes: (none)andany-code-changed=false.The fix resolves the prefix at run time with
git rev-parse --show-prefix,which is empty at the repository root — so a single-package checkout keeps
working unchanged.
Changes
Change detection (js, rust, python, csharp)
git rev-parse --show-prefix;files belonging to sibling packages are now ignored instead of counted.
.github/workflows/change still counts as a code change, since it canalter how the package is built and published.
repo-root no-op, examples/changeset-only changes, manifest detection,
workflow changes and foreign-package changes.
python/scripts/detect_code_changes.pyandcsharp/scripts/detect-code-changes.mjs, which did not exist before.Workflows
detect-changesgating job inpython.ymlandcsharp.yml(js andrust already had one), so a documentation-only pull request no longer runs
the full lint/test/build matrix. Gates use
!cancelled()becausedetect-changesis skipped onworkflow_dispatch.Run CI script testsstep injs.yml:npm testonly coverstests/, so the helper scripts the release jobs depend on had noautomated coverage at all.
security.yml: CodeQL (javascript-typescript, python, csharp,actions),
npm audit,cargo audit,pip-audit, a NuGet vulnerabilityaudit, and a gitleaks working-tree + full-history scan. Deliberately no
paths:filter — a security scan that can be skipped by touching the rightfile is not a security scan.
permissions:, and concurrencygroups applied across every workflow.
Full per-check detail, including the checks that were found to be
structurally incapable of failing, is in §9.3 of the analysis.
False positives found by the new gates
Both were investigated and are documented in §9.2:
npmjs.com— bot protection, not a broken link.key: macOS-cargo-<hex>— a cache key, not a secret.Two more defects that only CI could find
Both were found by the first pipeline runs of this branch and are documented
in §9.5 and §9.7:
>> $GITHUB_OUTPUTredirections. Localactionlintexited 0 while CI exited 1, because actionlint runs shellcheckonly when the binary is on
PATHand silently skips that analysisotherwise. The green local lint was itself a false negative. All 10 such
redirections are now quoted, not only the 4 lines the linter pointed at.
repository's Dependency graph enabled, which a pull request cannot change.
Rather than hide it behind
continue-on-error— an ignored failure beingthe same false negative the issue is about — it is removed. Its only unique
coverage was NuGet, now handled by
dotnet list package --vulnerable --include-transitive. That command exits 0 even when it finds advisories,so the job inspects the report rather than the exit code; both paths were
verified locally.
Both are handled by configuration rather than by disabling the check.
Upstream reports (requirement 3)
The path-prefix defect was verified to exist in two of the four templates by
running their own scripts in synthetic multi-language repositories. The
C# template already solves it (
prefixCsharpRoot()) and the Python templatepartially solves it (
removeprefix("python/")); js and rust do not, despitejs-paths.mjsandrust-paths.rsdocumenting multi-language support.Each report carries a runnable reproduction, actual vs. expected output, how
the sibling templates solve it, a suggested code fix and a workaround.
Two further defects hypothesised earlier in the analysis were disproved
by reading the templates in full —
tpl-rustdoes verify the crates.iopublish, and
tpl-python's PyPI smoke test does retry and fail the release.Both are marked withdrawn in place in §7 rather than deleted; a withdrawn
hypothesis is part of the record.
Verification
All four pipelines pass locally:
cargo fmt --check,clippy -D warnings, 22 script testsruff check/formatonsrc tests scripts,mypy src, 196 tests, 89% coverageactionlintexit 0 on every workflowNotes for the reviewer
Branch protection will need attention after merge:
Test (Python 3.13 on <os>).Detect Changes(Python and C#),
Scan for Committed Secrets,Check Links.Separately, and not blocking this pull request: enabling Dependency graph,
Dependabot alerts and secret-scanning push protection under
Settings → Security
would let the dependency-review job be restored on top of the audits above.
Why the parity gate is skipped
The parity gate reports "Changed: JavaScript, C# / Missing a matching
change: Python, Rust" and instructs: "If this change is intentionally
single-language, add
[skip-parity]to the pull request title or body."This pull request changes CI/CD infrastructure, not library behaviour — the
codec's public API is untouched in all four languages, so there is no
cross-language behaviour to keep in parity. The gate's own documented escape
hatch applies.