mirror of
https://github.com/cozystack/cozystack.git
synced 2026-10-29 16:17:20 +09:00
feat/vm-disk-http-auth
387
Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
 Andrei Kvapil
|
774f788016 |
ci(agents): check commit trailers on every pull request (#4103)
## What this PR does `docs/agents/contributing.md` makes a few properties of a commit message blocking at review: the attribution trailer takes the value `LLM` and nothing else and appears once, no message carries a session trailer or a link to a transcript, no line is a generation byline naming the tool. Nothing checked any of it, so the only reader was a human scrolling `git log`. That reader is expensive and misses things. One open PR carried eight commits naming a model in the trailer, another four, all found by hand after the work was done, each one costing a review round and a rebase. `hack/check-commit-trailers.sh` decides all of it from the text, and the `Commit trailers` job in `.github/workflows/pre-commit.yml` runs it over the commits between the merge base and the head. Over all 6062 commits on `main` it rejects 1300, every one a true positive and no prose false-fires: 1289 attribution trailers naming a model, 24 `Generated with <tool>` bylines, 15 commits carrying two trailers, two `claude.ai/code/session_...` links. ### Where it lives Next to the other per-PR text lint in `pre-commit.yml`. The unit-test job in `pull-requests.yaml` was the other candidate and is wrong for this: it is gated on the diff touching code, so a docs-only PR skips it, and a trailer is wrong or right whatever files the commit changed. The range is `merge-base..head`, recomputed every run, so a rebase or a force-push is read as it now is. It never reaches the history on `main`, roughly a quarter of which carries the old form. ### Decisions worth arguing with It fails rather than warns. A warning in a job nobody watches guards nothing. It is not a required check, and that part is deliberate. Branch protection on `main` requires `pre-commit` and `E2E Tests`, and `Commit trailers` is a new context, so a PR can still merge with it red. 77 of the 200 open PRs carry at least one commit this rejects, so requiring it today turns those red at their next push. Flipping it is a branch-protection change once the open set has drained, and somebody else's call. Fork PRs are checked like any other. The job runs on `pull_request` with a read-only token and reads commit messages as data. A merge commit with no trailer of its own passes. Nothing is required of a commit, only what is written is judged, so a merge commit that does carry a bad trailer fails like the rest. Backport branches are exempt. The backport action cherry-picks merged commits verbatim, so those messages are history and not the author's to rewrite. The exemption matches the generated name `backport-<pr>-to-<branch>`, which is the only signal a `pull_request` event carries, since the label sits on the original PR. So it is a convention, not something enforced. The checker is read from the base branch instead of the checked-out PR. A `pull_request` event takes the workflow definition from the merge of base and head, so the job shows up on every open PR the moment it lands, including the ones whose head predates the script, where reading it from the head is exit 127 and a red check saying nothing about trailers. It also keeps the checker out of reach of the PR being measured, though the job around it is not. Two shapes are left to the reviewer. A model named in `Co-authored-by:` needs a list of model names to tell it from a person, and such a list either misses next month's model or fires on somebody's colleague; most of the 771 such lines on `main` name a model, so it would be deciding a lot of PRs. A `Generated-by:` trailer has never appeared here at all. The byline rule matches a capital `G` for a related reason: case-insensitively, one line in 25 on `main` is an ordinary sentence that wrapped onto "generated with cozyvalues-gen". ### Known costs `hack/check-commit-trailers.sh` matches `hack/[^/]+\.sh$` in `select-e2e.sh`'s `full_suite_pattern`, so this PR and any later one touching the script escalates to the full e2e suite. That pattern's own comment scopes it to the e2e harness, which this script is not, and `inert_config_pattern` already carries `hack/[^/]+\.bats$`. Escalation is fail-safe so it is cost rather than breakage, and reclassifying it wants its own review. The step drops the explicit `git fetch origin "$BASE_REF"` that `api-review-gate.yaml` keeps before its own `merge-base`. `fetch-depth: 0` already fetches `+refs/heads/*:refs/remotes/origin/*`, and if that were ever wrong the failure is a loud `merge-base` error under `set -euo pipefail`, not a silent pass. ### Screenshots Not a UI change. ### Downstream repositories Walked the trigger map against the diff. The two entries that could fire are `cozystack/ccp` and `cozystack/community`, both keyed on a change to the commit convention itself. The convention text is untouched here, only its enforcement is new, so both copies stay accurate. `ccp`'s `package-bump` skill already writes `Assisted-by: LLM`, which this check accepts. - [x] No downstream repository is affected by this change - [ ] [cozystack/website](https://github.com/cozystack/website) - follow-up: - [ ] [cozystack/terraform-provider-cozystack](https://github.com/cozystack/terraform-provider-cozystack) - follow-up: - [ ] [cozystack/ansible-cozystack](https://github.com/cozystack/ansible-cozystack) - follow-up: - [ ] [cozystack/ccp](https://github.com/cozystack/ccp) - follow-up: - [ ] [cozystack/talm](https://github.com/cozystack/talm) - follow-up: - [ ] [cozystack/cozyhr](https://github.com/cozystack/cozyhr) - follow-up: - [ ] [cozystack/cozy-proxy](https://github.com/cozystack/cozy-proxy) - follow-up: - [ ] [cozystack/cozystack-telemetry-server](https://github.com/cozystack/cozystack-telemetry-server) - follow-up: - [ ] [cozystack/external-apps-example](https://github.com/cozystack/external-apps-example) - follow-up: - [ ] [cozystack/examples](https://github.com/cozystack/examples) - follow-up: - [ ] [cozystack/community](https://github.com/cozystack/community) - follow-up: ### Release note ```release-note ci(agents): a pull request now fails when one of its commits carries an attribution trailer whose value is not `LLM`, a second attribution trailer, a session trailer or transcript link, or a `Generated with <tool>` byline. The check reads only the commits the pull request adds; backport branches are exempt. ``` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Pull requests now include automated checks for commit attribution trailers, session links, and generation bylines. * Backport branches are excluded from this validation. * **Documentation** * Added guidance describing the commit-message checks, accepted attribution formats, exceptions, and how to run the validation locally. * **Tests** * Added coverage for valid and invalid commit-message formats, range handling, merge commits, and exempted content. <!-- end of auto-generated comment: release notes by coderabbit.ai --> |
||
|
Aleksei Sviridkin
|
d949099f5e |
fix(ci): map the recurring app scopes to their area labels
Scopes that PR titles use for apps under packages/apps were missing from the labeler's scopeToArea table. A PR whose title carries only such a scope falls through to the area/uncategorized fallback, and the area label has to be set by hand afterwards. Each scope added here clears the bar the table's own comment sets for a new entry, counted over the whole PR history in both title forms the labeler parses, and has an unambiguous home among the labels that already exist. A scope that clears the bar but has no such home is left out rather than pushed into the nearest label: AGENTS.md asks for a new area/* in that case, which is a separate decision. The homes come from the label descriptions. area/virtualization names cdi, and vm-disk renders a DataVolume while vm-instance renders a VirtualMachine. area/kubernetes is the tenant Kubernetes app, and kubernetes-nodes is the worker half split out of it. area/database is described as managed databases and already lists kafka, so rabbitmq, nats, qdrant and foundationdb sit there without stretching it. vpc renders Vpc and Subnet, and area/networking already names kube-ovn. Assisted-by: LLM Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
Andrei Kvapil
|
54621bc046 |
feat(migration): tenant self-service VM import from VMware vSphere
Hidora's customers are leaving VMware now, and a tenant has to be able to import their own machines without a platform administrator. The chart approach could not express that: a tenant can neither create the Secret holding vCenter credentials nor name the VDDK image, so both had to come from somewhere they cannot reach. Two CRDs in a new `forklift.cozystack.io` group, split by lifecycle. A VMImportSource is a long-lived connection -- type, endpoint, credentials, readiness -- and a VMImportTask is the one-shot operation that names machines and produces VMDisks and VMInstances. The outputs carry no owner reference back to the task, so deleting a finished import leaves the machines alone. Konveyor Forklift does the transfer underneath; the controller renders its objects, mirrors its verdicts, and hands each finished volume into a VMDisk without copying it. What a live vCenter migration forced out, and no amount of source reading would have: - A guest copied verbatim keeps the drivers it had under VMware and no virtio, so on a virtio disk it does not fail the import -- it succeeds and then does not boot. Imported disks go on SATA. - VDDK connects straight to the ESXi host at the address vCenter advertises, which is routinely unreachable, and on the first cluster collided with the Service CIDR. `spec.hosts` redirects per host. - Firmware must be carried across or a UEFI guest imports as BIOS and never boots (stacked on #3002). - Deleting a task mid-transfer orphaned its DataVolume; a finalizer now removes the engine's volumes and leaves the outputs. The console gains a Migration section: connections are editable, and the VM field is a picker built from a machine list the controller publishes, since the aggregated API cannot reach the inventory and should not hold a source's credentials to try. Two general mechanisms fall out of that -- an option source may take an argument, and one named in a CRD annotation may reference a sibling field. Design: cozystack/community#62. Docs: cozystack/website#673. Signed-off-by: Andrei Kvapil <kvapss@gmail.com> |
||
|
Myasnikov Daniil
|
62f2234aea |
fix(e2e): stop a failed teardown posting a failed test verdict
The teardown ends both merge-gating lanes under `if: always()`, and this branch made it `exit "$cleanup_rc"`. So a `docker compose down` hiccup, a busy zpool or a failed rmdir after a fully passing suite posts the required "E2E Tests" status as a failure, and nothing downstream can tell that apart from a test that actually broke. The runner is discarded when the job ends, so there is no next run for the strictness to protect; a reused host is a developer machine, and an annotation serves it. Loud, not fatal. Two comments described mechanisms that are gone. The breakpoint step still explained reading a soft-red output that this branch removed, so a maintainer would go looking for it; the node-join deadline now fails its suite like any other, which is what makes `failure()` sufficient. The build-talos header still described building the nocloud disk and e2e needing it, when the job builds neither and the container lane downloads no disk -- the profile is compiled by nightly and by the tag build's `make assets` instead, and what a PR still compiles is installer.yaml, which shares every input with it bar the platform and output format. And the platform-package override now states its consequence for both packages it replaces, not only for CDI: the merge-gating lane exercises neither the shipped CDI default nor `drbd.enabled: true`. Signed-off-by: Myasnikov Daniil <myasnikovdaniil2001@gmail.com> Assisted-By: LLM |
||
|
Aleksei Sviridkin
|
d45663f1af |
ci(agents): check commit trailers on every pull request
The attribution and session-link rules in docs/agents/contributing.md are decided by the text of a commit message alone, and until now the only reader was a human scrolling git log. That reader is expensive and misses things: one open pull request carried eight commits naming a model in the attribution trailer, another four, all found by hand after the work was otherwise done. Every one of them cost a review round to report and a rebase to fix. The check reads what is written and requires nothing, so a commit with no trailer passes and a merge commit that carries none passes with it. A trailer that exists and is wrong fails, naming the commit and the accepted form. It runs over the commits between the merge base and the head, so it never reaches the history on main, roughly a quarter of which predates the rule. A backport branch is exempt: its commits are cherry-picked verbatim and their messages are not the author's to rewrite. Assisted-by: LLM Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
Myasnikov Daniil
|
ad00f35f2d |
test(e2e): correct two comments that describe the opposite of the code
pull-requests.yaml said the QEMU substrate "is not wired into any workflow now" and that prepare-cluster and hack/e2e-prepare-cluster.bats stay in the tree for a nightly lane that will carry the coverage. Both halves are wrong in the direction that costs the most: nightly.yaml and e2e-tag.yaml already run `make prepare-env`, which calls prepare-cluster, so anyone trusting that comment deletes live code and takes nightly and every release-candidate run with it. docs/agents/e2e-testing.md states it correctly, so the tree disagreed with itself. hack/e2e-platform-packages.sh said the lane replaces "only LINSTOR with an otherwise-identical Package". It replaces two, and the kubevirt-cdi one is not identical: it raises the importer memory limit from CDI's 600M to 4Gi and the request from 60M to 256Mi. The consequence belongs next to the override -- the merge-gating lane does not exercise the shipped CDI default, and whether a tenant worker disk imports at 600M is answered by the QEMU lanes, which take no override. Also drops the dead half of `resolve_assets`. It resolved nocloud-amd64.raw.xz and exported disk_id; the container lane boots no disk and nothing in this workflow reads that output any more, so all the lookup could still do was fail a release-labelled PR over an asset it does not use. The job stays -- e2e's release arm gates on its result -- and now proves what is still worth proving, that the draft release exists. e2e-tag.yaml keeps resolving the asset, because it consumes it. Signed-off-by: Myasnikov Daniil <myasnikovdaniil2001@gmail.com> Assisted-By: LLM |
||
|
Myasnikov Daniil
|
186547d835 |
test(e2e): run the merge-gating lanes on Talos containers
The srv1-srv3 nodes of both lanes that gate a merge -- the same-repo one in pull-requests.yaml and the fork one in e2e-fork.yaml -- run as Talos containers instead of QEMU guests, so a tenant worker sits at L2 rather than L3. Measured in CI on commits where both substrates ran the same tree: whole-job wall clock down 24-49 minutes, essentially all of it in Chainsaw, while the two tenant-Kubernetes suites go from 1143-2040s to 466-809s and stop depending on the runner's vgif flag. The node-join soft-red gate goes with the substrate, which is what it was for. It existed because nested virtualisation on the shared runners degraded far enough that a worker registering in minutes elsewhere could miss any deadline this test could afford; #3513 established that the discriminator was the host's kvm_amd vgif flag, and removing a nesting level removes what it acted on. So hack/e2e-node-join-soft-red.sh, the SOFT-RED-node-join.txt marker and the soft_red job outputs are gone from all four workflows, and a missed deadline is an ordinary red again. The ::warning annotation survives, because the reason a run went red should be legible without opening the log, and so does the 124-versus-everything -else test, because "the workers were slow" and "the wait never ran" send a reader to different places. Done by swapping the substrate inside the existing e2e jobs rather than by promoting a separate container job. Those jobs carry more than a substrate -- the GitHub App token, the report-overrun guard, the images list, the SSH breakpoint -- and promoting a job that never had them would have dropped four features without saying so. Three properties of container mode drive the design, and each fails quietly rather than loudly. machine.kernel.modules is a silent no-op -- kernel_module_spec.go returns early on ModeContainer with no error and no event -- so the host loads openvswitch and zfs before the nodes start and the workflow step asserts both rather than trusting them; a missing module surfaces an hour later looking like a CNI or a storage regression. Docker gives a privileged container its own tmpfs /dev seeded once at creation, so /dev/kvm works but ZFS zvols created later never appear and linstor-csi fails ControllerPublishVolume; the compose file binds the host devtmpfs, which is also why talosctl cluster create docker can never carry LINSTOR. And there is no ARP VIP, so 192.168.123.11 stands in for the QEMU lane's .10. The QEMU machinery stays in the tree and keeps running: nightly.yaml and e2e-tag.yaml still call `make prepare-env`, so DRBD, the replicated StorageClass, its Immediate binding mode, the tenant StorageClass propagation that only applies to remotely-accessible classes, and the Cozystack Talos node image with its extensions all keep nightly and release-candidate coverage. None of them is exercised per pull request any more. That is the trade. Nothing on the PR path builds or downloads a nocloud disk now, so build-talos drops the talos-nocloud step and the talos-image artifact. build-talos itself stays: finalize consumes its digest fragment. One property this does not fix, recorded beside the runner class because it will be read off a red run eventually: a container node reports the HOST's CPU and memory as capacity, and hack/e2e-container-up.sh counters that with kubelet systemReserved. That corrects the scheduler and not what a pod reads from /proc, so a workload sizing itself from /proc/cpuinfo or /proc/meminfo is configured for the whole runner. Also carried here because the same files carry them: the ghcr.io pull-through mirror removal (see #4007 -- whichever lands first makes the other a no-op), the authoritative HelmRelease readiness gate, the backup-credential preflight, the prepull guard against an image-less container fragment, and folding the two standalone OIDC suites into the latest-version tenant cluster. Signed-off-by: Myasnikov Daniil <myasnikovdaniil2001@gmail.com> |
||
|
Myasnikov Daniil
|
e49633208c |
ci(tests): parallelize unit test targets
The BATS half of `make unit-tests` ran as one shell loop over ~60 files, which is a nine-minute serial tail no amount of runner CPU could touch. Each file becomes its own Make prerequisite instead, so `-j4` schedules them, and the workflow asks for `make unit-tests test-controllers -j4 --output-sync=target` in a single step: one make process owns all four runner CPUs, the Helm and Go targets share the same bounded pool rather than waiting for each other, and output stays grouped by target instead of four logs interleaving. The existence check moves into its own prerequisite rather than disappearing. It guards against the wildcard matching nothing, which would otherwise make the whole target succeed by having no work. COZYTEST_TRACE rides per target. It arrived with the serial loop it now replaces, and a target that dropped it would go back to streaming full xtrace for every suite -- so the parallelism guard requires the prefix rather than tolerating it. Signed-off-by: Myasnikov Daniil <myasnikovdaniil2001@gmail.com> |
||
|
Andrei Kvapil
|
440c3b58f3 |
test(valkey): add e2e test and wire build, labeler and docs
Add the Valkey e2e bats test (mirrors redis), build the compat image in the top-level build target, map the valkey scope to area/database in the PR labeler, and list valkey among charts with an immutable storageClass. Assisted-By: Claude <noreply@anthropic.com> Signed-off-by: Andrei Kvapil <andrei.kvapil@aenix.io> |
||
|
Myasnikov Daniil
|
a0c297a08a |
docs(ci): mark the legacy backport label arms as transitional
The dual acceptance exists so the rename can land in any order relative to disabling dosubot's PR labelling, not because two spellings are wanted long term. Naming the deletion sites keeps that from becoming permanent by default. Assisted-By: Claude <noreply@anthropic.com> Signed-off-by: Myasnikov Daniil <myasnikovdaniil2001@gmail.com> |
||
|
Myasnikov Daniil
|
624a424d29 |
fix(ci): keep the backport concurrency guard working for namespaced labels
The guard from #3569 appends -label to the concurrency group for any labeled event that is not a backport label, so a backport label never cancels a run already in flight. It enumerated the two legacy names only, so once the labels are namespaced a kind/backport event takes the -label branch and the guard stops covering the case it exists for. Both spellings are listed for the same reason the trigger lists both: the org-level dosubot still applies the legacy names. Assisted-By: Claude <noreply@anthropic.com> Signed-off-by: Myasnikov Daniil <myasnikovdaniil2001@gmail.com> |
||
|
IvanHunters
|
0fa6cf72a5 |
chore(ci): complete the backport label rename across all consumers
The initial rename missed several literal consumers of the old label names beyond labels.yml and backport.yaml: - stale.yaml exempt-pr-labels still listed backport,backport-previous. actions/stale matches label names exactly and does not resolve label-sync aliases, so after the rename a kind/backport PR would lose its stale exemption and get auto-closed after inactivity. This is the one behavioural regression; the rest are documentation drift. - PULL_REQUEST_TEMPLATE.md told contributors to apply the backport label. - release.md label-target table and the gh pr list --search commands referenced the old names (the searches would silently return nothing). - contributing.md claimed a backport label is auto-applied on the [Backport] prefix; pr-labeler only adds area/release, so the line was already stale and is corrected while renaming. Historical narrative in release.md and the backport scope-to-area doc rows (a Conventional-Commit scope, not the label) are left as-is. Signed-off-by: IvanHunters <xorokhotnikov@gmail.com> |
||
|
IvanHunters
|
7b71053a0b |
chore(ci): namespace the backport labels under kind/
The labels.yml header declares that names follow the Kubernetes scheme, but backport and backport-previous were top-level, violating it. Rename them to kind/backport and kind/backport-previous, grouped under the kind/ section, and carry the old names as aliases so EndBug/label-sync renames in place without dropping history on already-tagged issues and PRs. Update the label literals in backport.yaml in lockstep (the if filter and the label-detection script). pr-labeler.yaml's 'backport' entry is a Conventional-Commit scope-to-area mapping, not a label reference, so it is left unchanged. The top-level release label is a functional trigger keyed by name in several workflows; renaming it is a wider cross-workflow change out of scope here, so it is left as-is intentionally. Signed-off-by: IvanHunters <xorokhotnikov@gmail.com> |
||
|
Andrei Kvapil
|
6663c44dd5 |
ci(labeler): map the kubeovn-webhook scope to area/networking
The package has its own scope in commit history but no entry, so its PRs land on the area/uncategorized fallback. It is a kube-ovn admission component, which is the same area as the kube-ovn scope beside it. Assisted-By: Claude Signed-off-by: Andrei Kvapil <andrei.kvapil@aenix.io> |
||
|
Aleksei Sviridkin
|
ad62a8d62b |
test(e2e): stop blocking merges on the tenant node-join deadline
The tenant node-join wait no longer discriminates. Nested virtualisation on the shared runners degrades far enough that a guest which registers in minutes on real hardware can miss any deadline this test can afford, so a red on that one wait says as much about the machine the run landed on as about the product, and it teaches a reader to re-run the job. The suite still fails on it. That is what keeps Chainsaw's catch running: the previous-instance container logs, the host snapshot, the data-plane capture and the event dump are all keyed on the test failing, and the report stays honest. What changes is one layer up. The wait writes a marker beside its diagnostics, and a new lane-side gate reads that marker to decide whether the job blocks. It opens only when every failed test in the run carries one, and blocks on anything it cannot attribute: a report it could not read, a failed test the report does not name, a failure with no marker. Only the lanes that block a merge ask it. The two pull-request lanes do; nightly and the tag lane run the same suites and stay hard. A soft pass on the tag lane would ship a release candidate whose tenant clusters brought up no worker nodes, and nightly is the only place an e2e failure reaches anyone who did not open a pull request, since no lane in this tree notifies anywhere. A softened red finishes the job green, so the job's result stops answering whether the suite passed, and both kinds of consumer are handed the decision instead of inferring it. A step gated on the job failing would silently stop running: the SSH breakpoint is that case, and it still opens for a pull request marked for debugging. A job that reports the result would silently start reporting a pass: the required commit status now says the deadline was missed and that the merge is not blocked on it, rather than that the suite passed. The deadline goes to 29m. That is the test's own budget and nothing more: its clock starts after the machinedeployment wait, the LoadBalancer assignment and the healthz probe, while the chart's autoscaler and health-check timers have been running since the Machine was created, so no ordering against them is claimed. The chart's 20m and 30m give a scale to pick against, and a budget over the first leaves room for a remediation cycle without promising that one fits. The operation ceiling both suites give the script rises with the deadline, and the diagnostics phase budget rises inside it, because the collectors gated ahead of the serial console cost more between them than the old budget covered. Both budget guards read their terms from the sources that set them. The e2e job cap rises with them, and its guard holds what a guard can hold: the pair's operation and catch ceilings, and one figure across every lane that runs the suite. It also records that the suites' full summed ceilings have never fit any cap in that file, so the margin above the bound is taken from a measured run instead. The prose that restates the cap elsewhere moves with it, and the sweep that holds the deadline's restatements now reads the figure spelled in words as well as in minutes. Assisted-By: Claude <noreply@anthropic.com> Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
Aleksei Sviridkin
|
61f7939ab7 |
ci(workflows): make E2E breakpoint opt-in via labels only
Drop the author-association branch from the "Breakpoint on E2E failure" step condition, so a red end-to-end run only opens an SSH breakpoint when the pull request carries the debug or release label. The dropped branch held an ephemeral CI runner open for up to twenty minutes on every red maintainer-authored pull request, whether or not anyone intended to attach. Trim the step's explanatory comment to match: drop the paragraph about the removed authorship path, including its stale claim that only the debug label affects runner selection, since this workflow has no conditional runs-on to begin with. Assisted-By: Claude <noreply@anthropic.com> Signed-off-by: Aleksei Sviridkin <3811295@gmail.com> |
||
|
Myasnikov Daniil
|
7ac2ffe504 |
fix(ci): resolve fork PRs by open-PR head SHA, not commit association
The fork-PR e2e lane never ran. `resolve` looked the PR up with
`listPullRequestsAssociatedWithCommit` against the base repo, and that
endpoint returns nothing for a fork PR head: the commit reaches
cozystack/cozystack only as `refs/pull/<n>/head`, and the association
index does not walk `refs/pull/*`. So every fork PR fail-closed on
"expected exactly one open PR for <sha>, found 0", stamped a red
required "E2E Tests" its author could not clear by pushing, and skipped
publish/e2e/report.
It resolves same-repo heads correctly, which is why the hole stayed
invisible — same-repo PRs get "E2E Tests" from pull-requests.yaml and
never enter this workflow. Of 935 runs, 930 skipped as designed and all
5 that reached `resolve` stopped there; three on `found 0`, and one of
those reds was later overwritten by a mirrored same-repo run, which hid
it again.
Enumerate the base repo's open pull requests instead and match on the
head SHA. Same trust properties as the call it replaces — plain
base-repo API data the fork cannot forge — and `state: 'open'` at the
source keeps the rule that a closed or merged PR never counts. The
exactly-one guard and the head_sha match are unchanged, so a stacked
lower PR still passes and two PRs genuinely at the same head still fail
closed. Pagination stops being a corner case and becomes load-bearing:
the repo has 180 open PRs, and the fork PR that surfaced this sits on
page two.
Verified against the live API for the failing case: PR #1982 at
|
||
|
Aleksei Sviridkin
|
467e431765 |
chore(ci): add area/cilium label and map the cilium PR scope to it
Cilium has enough open issues of its own (version pins and bumps, the Gateway API implementation, network policy, IPAM next to kube-ovn, LB-IPAM) that area/networking no longer narrows a search. Declare area/cilium in labels.yml, route the cilium scope of PR titles to it instead of area/networking, and update the scope table in the contributing guide. Assisted-By: Claude <noreply@anthropic.com> Signed-off-by: Aleksei Sviridkin <3811295@gmail.com> |
||
|
myasnikovdaniil
|
9f5550abdd |
fix(release): publish rewritten packages during RC promotion (#3795)
## What this PR does Fixes #3477. - Publishes a new packages OCI candidate after rewriting RC image tags to stable tags. - Pins the candidate digest in the stable installer without rebuilding container images. - Verifies the candidate against the prospective merge and original RC artifact before creating stable names. - Refuses promotion when the target maintenance branch lacks the candidate-aware pipeline. - Protects candidates referenced by open release PRs from retention cleanup. - Leaves already published v1.6.0 and v1.6.1 unchanged. ### Screenshots Not applicable. ### Downstream repositories - [x] No downstream repository is affected by this change - [ ] [cozystack/website](https://github.com/cozystack/website) - follow-up: - [ ] [cozystack/terraform-provider-cozystack](https://github.com/cozystack/terraform-provider-cozystack) - follow-up: - [ ] [cozystack/ansible-cozystack](https://github.com/cozystack/ansible-cozystack) - follow-up: - [ ] [cozystack/ccp](https://github.com/cozystack/ccp) - follow-up: - [ ] [cozystack/talm](https://github.com/cozystack/talm) - follow-up: - [ ] [cozystack/cozyhr](https://github.com/cozystack/cozyhr) - follow-up: - [ ] [cozystack/cozy-proxy](https://github.com/cozystack/cozy-proxy) - follow-up: - [ ] [cozystack/cozystack-telemetry-server](https://github.com/cozystack/cozystack-telemetry-server) - follow-up: - [ ] [cozystack/external-apps-example](https://github.com/cozystack/external-apps-example) - follow-up: - [ ] [cozystack/examples](https://github.com/cozystack/examples) - follow-up: ### Testing - `bats --no-tempdir-cleanup hack/promote-packages-artifact_test.bats hack/verify-promoted-packages_test.bats hack/promote-packages-contract.bats hack/promotion-retention-contract.bats` - `actionlint .github/workflows/promote-rc.yaml .github/workflows/pull-requests.yaml .github/workflows/pull-requests-release.yaml .github/workflows/retention.yaml` - `sh -n hack/promote-packages-artifact.sh hack/verify-promoted-packages.sh` - `git diff --check origin/main...HEAD` ### Release note ```release-note fix(release): publish the rewritten stable packages artifact when promoting a release candidate ``` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit - **New Features** - Release candidates now use temporary, immutable package artifacts verified before stable publication. - Stable promotions validate package contents, installer references, container digests, and release metadata before tagging. - Temporary promotion artifacts are retained while needed and cleaned up after expiration. - **Bug Fixes** - Improved handling of release labels, fork pull requests, and promotion pull requests. - Added end-to-end verification before release promotion. - Prevented invalid, ambiguous, or untrusted package references from being promoted. - **Documentation** - Updated release and promotion guidance for candidate verification, digest pinning, and transactional promotion. <!-- end of auto-generated comment: release notes by coderabbit.ai --> |
||
|
Aleksei Sviridkin
|
db2ddfab2b |
fix(ci): give the nightly e2e runner headroom over its guest vCPUs
The e2e sandbox boots three QEMU management nodes at `-smp 8`, so 24 guest vCPUs are committed before any tenant VM starts. The nightly lane ran that sandbox on a 24-vCPU runner, a 1:1 commit that leaves the host nothing for QEMU's emulation and IO threads, containerd, the kernel and the runner agent. The tag, fork and pull-request lanes already run the same sandbox on the 32-vCPU class, so move nightly there too. This aligns headroom and claims no performance effect. Measured boot data shows the load a workflow puts on the host, not the runner class, dominates guest starvation, and the two are confounded in that data; no red run is attributed to the class here. The case is the arithmetic: a 1:1 commit leaves no headroom, and two lanes running the identical sandbox on different classes is drift nothing was watching. Add hack/sandbox-runner-headroom.bats to keep the two sides paired. It derives the guest vCPU and memory demand from the sandbox bats file, and the runner class of every job that drives the sandbox make targets, then requires the runner to be strictly larger on both axes. Neither side is a literal list, so raising `-smp`, adding a management node or adding a lane on a smaller runner turns it red instead of drifting apart silently. Input the derivation cannot read in full is refused rather than guessed: a guess produces a demand smaller than the real one, which passes a runner that cannot hold the sandbox, while a refusal names the line. That covers a launch written outside any `@test` block, a block left open at end of file, a `for` opener the strict pattern misses, a non-literal loop list, and a `while` loop sharing the block. The derivation runs against the live tree, which passes whether or not those refusals still work, so each one is pinned by a fixture feeding a synthetic file to the same program. Loosening the parser turns a fixture red rather than quietly widening what the check accepts. Record the rule alongside the other environment invariants in docs/agents/e2e-testing.md, so a new lane picks its runner class from the documentation rather than from a failing unit suite. Assisted-By: Claude <noreply@anthropic.com> Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
Myasnikov Daniil
|
ffae003d6b |
fix(release): stop a failed org package listing from exiting green
The nightly sweep's own listing shared one `|| true` with the grep that filters it, and grep legitimately exits 1 when no cozystack/* package matches. A failed /orgs/.../packages read therefore arrived as an empty list, took the "No cozystack/* container packages found" branch and exited 0 -- below the nightly sweep but ABOVE the deferred guard, so a protection set that could not be resolved reported success having swept nothing. That bypasses the guard rather than varying it: replaying the step against a stub gh, a failed /pulls combined with a legitimately empty org listing exited 0 after printing its own ::error::. List and filter as two statements so one `|| true` stops meaning two things, handle the listing failure like the other two (::error::, blocked flag, emptied list), and consult the flag before the early exit. That exit is non-zero, matching the other two listing failures: no sweep is left to carry the run down to the guard, so this is the last place the flag can be honoured, and an unresolved protection set is exactly what fail-closed means. The legitimate empty-org case still prints the same line and exits 0. Assisted-By: Claude <noreply@anthropic.com> Signed-off-by: Myasnikov Daniil <myasnikovdaniil2001@gmail.com> |
||
|
Myasnikov Daniil
|
dee398f3b3 |
fix(release): defer a retention listing failure to the promotion guard
Both candidate-protection sources open with a `gh api --paginate` listing and neither handled a failure. Under `set -euo pipefail` a transient /pulls or /branches blip therefore exited the step before the unrelated nightly GHCR sweep had run -- the exact coupling the deferred guard at the bottom exists to remove, and which hack/promotion-retention-contract.bats already asserts positionally. Route both listings into that guard instead: raise the blocked flag and empty the list, so the promotion sweep is skipped and the job still fails while the nightly sweep finishes. Emptying is not cosmetic -- a half-paginated list left in place is a smaller protected set, and a smaller protected set is the one shape that can authorise a delete. Replaying the step's real shell against a stub gh: with /pulls failing it exited 5 and never reached the nightly line; it now exits 1 with the nightly sweep done and the promotion sweep skipped. Assisted-By: Claude <noreply@anthropic.com> Signed-off-by: Myasnikov Daniil <myasnikovdaniil2001@gmail.com> |
||
|
Myasnikov Daniil
|
04fa8471fe |
docs(release): say what the gates actually guarantee
Three claims that were stronger than the code, plus one legibility fix. The PR gate's comment said a `packages/` edit "on the PR or its base" fails the status before merge. The base half is not true: GitHub fires no `pull_request` event when the base branch advances, it only recomputes refs/pull/N/merge, so a change landing on the base after this went green is never re-verified and the PR merges on the stale result. Checking the merge tree does make base drift visible WHEN the job runs, which is worth keeping — but finalize is what actually catches the drift case, after the merge and before any stable name. Say both, in the workflow and in the reviewer checklist, so nobody reads the green as covering more than it does. "Transactional at every consumer-visible name" is a judgement call dressed as an absolute, inherited from the "this dispatch performs NO registry mutation" it replaced. The dispatch now does write to the registry: the `promotion-*` candidate is a real, publicly listable tag. What it creates is no *stable* name — no vX.Y.Z, no :latest, no stable installer. Stated that way the sentence stops needing the reader to already know which writes were meant. The retention paragraph listed two of the conditions the workflow enforces. It now lists all of them, including the ones the workflow gained here: same-repository, bot-authored, `release-X.Y.Z` branch, the `release` label, exactly one valid digest pin, and protection from a base-branch tip as well as an open PR. Also: `flux pull artifact` reads its credentials from ~/.docker/config.json, which is why finalize now runs `docker login` — but its runner class is not one of the docker-using build ones and the setup step installs everything except docker. Check for it there, where the message can name what is missing, instead of failing with "command not found" halfway down the release path. And the verifier's exclusion list is a restatement of flux's `excludeOCI`, not a derivation — the CLI does not expose it — so the coupling and the pinned version are now written down next to the list. Assisted-By: Claude <noreply@anthropic.com> Signed-off-by: Myasnikov Daniil <myasnikovdaniil2001@gmail.com> |
||
|
Myasnikov Daniil
|
820e72d572 |
fix(release): protect a candidate until its retag, not until PR merge
Retention protected a packages candidate only while an OPEN bot release PR pinned its digest. The digest stops being eligible for deletion only when finalize's retag adds a stable tag to the manifest, because that is what makes the `all(tags match promotion-*)` predicate stop matching — and finalize runs after the merge that closes the PR. The window between them is where a half-finished release sits. Finalize creates the write-once stable git tag, then dies before the retag: a lost runner, a 5xx publishing the draft, a non-fast-forward on the maintenance branch. What is left pins the candidate from a merge commit and a tag nobody can move, with no open PR and only a temporary tag on the manifest. Thirty days later retention deletes it, after which re-running finalize 404s at the candidate check and re-dispatching promote is refused because the stable tag exists. The release can then only be finished by hand — the exact class of manual cleanup this design exists to remove. Protect the tips of the branches a promote PR can merge into as well. The two sources are read differently on purpose. An open promote PR's head is supposed to carry exactly one candidate pin, so anything else is an anomaly and blocks. A branch tip is an additional source: every digest pin found is protected and a branch without one contributes nothing, because a maintenance line cut before this pipeline existed cannot pin a candidate and must not wedge retention forever. Blocking now stops the promotion sweep alone. It used to `exit 1` inside the resolution loop, which also killed the unrelated nightly prune further down the same step — so a WIP commit adding a trailing comment to a pin on a release branch would silently stop GHCR nightly pruning. The job still fails; it fails after the nightly sweep has run. Verified by running the step's script against a stubbed `gh`: a digest pinned by main's tip and referenced by no open PR is now excluded while a genuinely abandoned one is still listed; a manifest that also carries a stable tag stays excluded by the existing predicate; and a malformed pin on a PR head leaves the nightly sweep running, skips the promotion sweep and exits 1. Assisted-By: Claude <noreply@anthropic.com> Signed-off-by: Myasnikov Daniil <myasnikovdaniil2001@gmail.com> |
||
|
Myasnikov Daniil
|
9fa8eeeda2 |
ci(release): pin the promotion toolchain and cap the candidate gate
Three review findings on the new promotion steps.
`yq` was fetched from `releases/latest/download` in all three steps this
pipeline installs it in — the promote dispatch, the promote PR gate and
finalize. That makes an unpinned moving dependency part of what decides
whether a stable release may be created, and part of what rewrites the
installer pin: two promotions of the same rc can be served different
binaries. Pin it the way FLUX_VERSION already is, version-gated so a
matching preinstalled yq is reused, on the version the e2e sandbox image
already tracks.
A version authenticates nothing about the bytes it names, though, and
these are the workflows that mint write-once stable names. yq is
therefore also pinned BY CONTENT: its SHA-256 is committed beside its
version and verified before install.
Scoped to yq deliberately. It is the one downloaded binary with write
authority over what a release contains — it stamps the candidate digest
into installer values at promote time and platformVersion into the
stable chart at finalize. skopeo, helm and oras only move bytes that are
already digest-addressed, so a substituted one is caught downstream; a
substituted yq writes the wrong pin and every later check verifies that
pin faithfully.
The Flux installer is left as it is, on purpose. It fetches
flux_${FLUX_VERSION}_checksums.txt from the pinned release and verifies
the binary against it, so the binary is not the exposure — the ~6 KB
bootstrap script from fluxcd.io is, and it is mutable by design and
fetched identically in ten other workflows here. Pinning its bytes in
these three files would break on the next regeneration of a script the
project does not control while leaving the same fetch unpinned
everywhere else. The comment names the residual risk instead.
No precedent existed either way: nothing under .github/workflows/ or
hack/ verified a downloaded tool before this, and every version-pinned
download in the tree (oras 1.2.0, cozyvalues-gen 1.6.0) is unverified.
This is a new convention rather than an application of one, confined to
the release path and justified by write authority, and the comment says
so where the next reader will be standing. The three unpinned fetches
outside this pipeline (nightly.yaml, e2e-fork.yaml) are left alone.
The checksum is trust-on-first-use, which is the honest description: it
fixes the bytes to what was checked when the version was chosen and
makes any later substitution loud. Provenance is the release's own
published checksums list cross-checked against the downloaded asset;
both agree on
fa52a4e758c63d38299163fbdd1edfb4c4963247918bf9c1c5d31d84789eded4, and
that binary self-reports v4.53.3.
Verified locally against the real asset: the version gate skips a
matching preinstalled yq and re-installs a mismatched one, the checksum
line reports OK for the genuine binary and exits 1 for a tampered copy,
which aborts the step under the runner's `bash -e`.
`verify-release-candidate` had no `timeout-minutes`, so it inherited the
6-hour default while `e2e-report` waits on it through `needs` — a
stalled `flux pull artifact` would hold the required `E2E Tests` status
pending for the whole of it. 30 minutes, matching `checks` and
`finalize`.
The contract pins that all three install steps carry both versions and
the checksum, run the checksum check, name no `latest/download`, and
agree on one yq version and one checksum. So the unpinned one-line idiom
cannot come back into a release step by copy-paste from the workflows
that still use it, and bumping the version in one file without its
checksum cannot pass. Both mutations are caught.
Signed-off-by: Myasnikov Daniil <myasnikovdaniil2001@gmail.com>
Assisted-By: Claude <noreply@anthropic.com>
|
||
|
Myasnikov Daniil
|
2d734fb229 |
fix(release): preflight the rc tree's own pin before publishing
hack/promote-packages-artifact.sh pushes the candidate BEFORE rewriting installer values, so the artifact carries the rc tree's platformSourceUrl/platformSourceRef verbatim — and hack/verify-promoted-packages.sh reads exactly those two fields back out of it as the candidate's rc baseline, requiring a trusted repository and an immutable `digest=sha256:<64 hex>` pin. The publisher checked neither. A tag-form ref or an untrusted URL in the rc tree published a candidate and pushed two commits regardless; the refusal arrived later, in the promote PR gate or in finalize, once the staging branch and the draft release already existed. Check both at dispatch time instead, while nothing has been written. A copied regex would drift from the guard it is bringing forward and quietly restore the late failure, so the trusted repository and the digest pattern move into hack/lib/promoted-packages.sh and both halves source it. That makes the library part of what the verifier needs at run time, so promote's base-branch precondition gains it: the list exists because the base's own workflows run after merge, and a base carrying the verifier without its library fails at the same two late points. Tests: both preflights are covered, each asserting the mock registry client was never invoked — "exited non-zero" alone does not say nothing was written. Both fail with the preflight removed. The publisher fixtures now pin the trusted repository the way a real rc tree does, and still send the candidate elsewhere via REGISTRY so the rewrite assertions keep their teeth. Also covers a malformed PROMOTION_ID. It is not attacker-influenced, but it is the run-uniqueness suffix of the candidate tag and retention.yaml deletes a candidate only when every tag on the manifest matches `promotion-v…-run-[0-9]+-[0-9]+`; a value outside that shape publishes an artifact no cleanup pass can ever collect. Assisted-By: Claude <noreply@anthropic.com> Signed-off-by: Myasnikov Daniil <myasnikovdaniil2001@gmail.com> |
||
|
Myasnikov Daniil
|
5d56681f8c |
fix(ci): put every label guard in lockstep, forks included
Promotion needs the `release` label to start work, so this PR added it to the set of labels whose `labeled` event is not discarded. That set is stated in four places and was updated in three. `gate-concurrency-contract.bats` caught the shape of the problem and then became the first casualty of it. It asserted that the concurrency key excludes exactly ONE label from the `-label` group, by counting occurrences of `github.event.label.name != '`. A second publishing label made the count 2, turning the suite red — and with it `checks`, then `finalize`, then `e2e`, then the required `E2E Tests` status. Raising the constant to 2 would re-hardcode the same brittleness one label later and throw away the invariant the test exists for: the set of labels excluded from the `-label` suffix is exactly the set `plan` runs for. Both sets are now extracted from the workflow and compared, so the assertion holds at any number of labels. `resolve_assets` was the guard that got missed. It sits on `e2e`'s release arm, so a label event `plan` runs for and it skips for leaves `e2e` with both arms false — `finalize` is force-skipped for release PRs — while `e2e-report` still runs. On a promote PR already carrying `full-e2e`, re-applying `release` therefore superseded the green run and posted `E2E Tests` = failure, because `full-e2e` bypasses the "e2e not repeated" branch and `E2E_RESULT` is `skipped`. `unlabeled` is not a trigger, so removing the label could not clear it; only a push could. `release` must also not publish on the fork lane. e2e-fork.yaml's `resolve` decides "this run built nothing, so it is not a verdict" by asking whether `Plan build` skipped. Admitting `release` at `plan` made that false: a fork run reaches `plan`, has `build`/`finalize` gated off by its own label, succeeds having produced no pr-patch, and the fork lane fails closed on the missing artifact — red on a suite that was never going to run, on a PR whose only fault is a mislabel. `plan` and `resolve_assets` now admit `release` for same-repo PRs only, and the concurrency key carries the complementary term, so NOT(A OR B OR (C AND NOT F)) stays exactly (NOT A) AND (NOT B) AND (NOT C OR F). For a same-repo PR F is false and every one of them reduces to what it was, so none of this changes non-fork behaviour. The same shape still reaches the fork lane from a PUSH to a fork PR somebody mislabelled `release` — `build` is gated on the label, not on the event, so this predates promotion using the label. Left alone; the comment in e2e-fork.yaml now records it instead of asserting an invariant this PR would have made false. The contract enumerated `plan` and `e2e-report` by name, which is why it did not catch `resolve_assets`: a named-job loop cannot notice a job nobody thought to name. It now derives the job list from the file and requires every job that decides from a label name to decide from the same set, and pins the fork term on both halves of the complement. The file's existing properties are kept — extraction runs over executable-only lines, so a commented-out key cannot satisfy the contract, and every comparison asserts a non-empty input first, so an empty extraction fails closed under both bats and cozytest.sh. Verified by mutation, each caught: excluding a label `plan` skips for, admitting one the key does not exclude, reverting `resolve_assets`, deleting `e2e-report`'s guard, narrowing `verify-release-candidate`'s, renaming the label in one guard only, commenting out the group key, and dropping the fork term from either side alone. Deleting `plan`'s step-level fork guard is caught too, by an assertion that had to stop matching on the bare identifier once `plan`'s own `if:` named it. Assisted-By: Claude <noreply@anthropic.com> Signed-off-by: Myasnikov Daniil <myasnikovdaniil2001@gmail.com> |
||
|
Myasnikov Daniil
|
f21d277eba |
fix(release): retain live promotion candidates
Protect package candidates referenced by open release PRs while pruning abandoned run-specific promotion artifacts. Document the stable-candidate lifecycle and maintenance-branch rollout requirement. Assisted-By: Claude <noreply@anthropic.com> Assisted-By: GPT-5 <noreply@openai.com> Signed-off-by: Myasnikov Daniil <myasnikovdaniil2001@gmail.com> |
||
|
Myasnikov Daniil
|
c2e145709b |
fix(release): publish stable packages candidate
Serialize the tag-rewritten packages tree during RC promotion and pin its immutable digest in the installer. Verify the candidate against the prospective merge and the original RC artifact before creating stable names. Assisted-By: Claude <noreply@anthropic.com> Assisted-By: GPT-5 <noreply@openai.com> Signed-off-by: Myasnikov Daniil <myasnikovdaniil2001@gmail.com> |
||
|
Aleksei Sviridkin
|
ea78d09dd0 |
feat(ci): auto-apply triage labels to issues
Issues created via the API or gh bypass the issue templates and arrive without any triage/* label; 252 of the 396 open issues carry none. Label issues on arrival (opened/reopened) and run a daily sweep as a backstop: issues already prioritised, assigned, or marked as epics get triage/accepted, the rest triage/needs-triage. A signal has to be on stale.yaml's exempt-issue-labels, because triage/accepted is itself exempt and a signal that is not would grant a permanent stale reprieve as a side effect of being labeled. That is necessary and not sufficient: lifecycle/frozen is exempt and still not a signal, because a lifecycle/* label says what the stale bot may do with an issue and nothing about whether anybody reviewed it. The sweep paces its writes and caps how many it makes per run, so a pass over a large backlog stays inside GitHub's secondary rate limits instead of failing partway through. Each run writes a job summary with the counts, the issue numbers per label, and both remainders it can leave behind, because a backlog pass logs a line per issue and nobody reads 252 of those to find out what happened. The manual dispatch defaults to dry-run, so the one entry point a human can reach writes nothing until somebody unchecks the box. The schedule is the one event that writes without being asked. Deleting a triage label does not undo a decision: an issue left with none is relabeled by the next sweep, so AGENTS.md's labeling section says to change a decision by replacing the label instead. No PR lane can exercise this workflow, so the first real run is a sweep over every open issue. A contract test pins the executable lines that bound that: the pull-request skip, the already-triaged skip, the pacing and the cap, the dry-run default, the pinned action, the token scopes, that no signal comes from the lifecycle/* namespace, and that the summary is written with the await that makes it flush. Assisted-By: Claude <noreply@anthropic.com> Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
Aleksei Sviridkin
|
e0d6c6be92 |
fix(ci): stop labeled events from cancelling and re-running backports
A run joins its concurrency group before any job-level `if` is evaluated, so `prepare`'s guard cannot stop a label event from disturbing the run already backporting a PR. Labels set with the default GITHUB_TOKEN start no run, but labels set by third-party GitHub Apps do, and they arrive in bursts, so each one cancelled the backport the merge had just started. The delivery survived anyway, which is why this went unnoticed: the old guard admitted a run on the PR's cumulative labels, so the last label of a burst requalified and redid the work. In the bursts on record the kill lands within seconds, well before the cherry-pick, so the cost was a restart rather than a lost backport. Move a label event that requests no backport into a group of its own, and let one that does request a backport queue rather than cancel. Both halves are needed. `backport` and `backport-previous` are separate requests that both have to complete, so they share a group and must not cancel each other, while every other label does no work here and only has to stay out of the way. Scope `prepare` to the labels each trigger is entitled to read at the same time. Reading the cumulative label set on a `labeled` event let any later unrelated label on a merged PR still carrying `backport` re-enter the job to redo a backport already delivered. That redo is also what redelivered a killed backport, so narrowing the guard alone would turn a noisy delivery into a missing one: neither change is safe to land without the other. Bound both jobs with `timeout-minutes` for the same reason, since a queued request now waits on the run ahead of it rather than replacing it. The ceiling bounds execution and not the wait for a runner, which no job-level key reaches, so 30 is generous rather than calculated: every execution measured on this workflow has been seconds, against minutes of queue that nothing here can bound. Document the recovery path the narrowing removes: with no accidental re-run left, re-applying whichever backport label the PR already carries is how an operator retries a merge-triggered run that died. Assisted-By: Claude <noreply@anthropic.com> Signed-off-by: Aleksei Sviridkin <3811295@gmail.com> |
||
|
Aleksei Sviridkin
|
1b178886dc |
ci(backport): skip a target whose backport already merged (#3616)
<!-- Thank you for making a contribution! Here are some tips for you: - Use Conventional Commits for the PR title: `type(scope): description` - Types: feat, fix, docs, style, refactor, perf, test, build, ci, chore - Scopes are not an exhaustive list — pick the most specific scope for the change and extend the list when a genuinely new area appears. Examples: - System components: dashboard, platform, operator, cilium, kube-ovn, linstor, fluxcd, cluster-api - Managed apps: postgres, mariadb, redis, kafka, clickhouse, virtual-machine, kubernetes - Development and maintenance: api, hack, tests, ci, docs, maintenance - Breaking changes: append `!` after type/scope (`feat(api)!: ...`) or add a `BREAKING CHANGE:` footer - If it's a work in progress, consider creating this PR as a draft. - Don't hesistate to ask for opinion and review in the community chats, even if it's still a draft. - Add the label `backport` if it's a bugfix that needs to be backported to a previous version. --> ## What this PR does Adds a guard step to the `backport` job in `.github/workflows/backport.yaml` that looks up whether a merged pull request already exists on a target's deterministic backport branch (`backport-<pr>-to-<target>`) and skips the checkout and the backport-action step for that leg when one does. A second label event on a PR whose first backport already merged, for example `backport-previous` added shortly after `backport` merged, re-enters this job for the already-delivered target, and cherry-picking the same commits onto a branch that already has them produces an empty pick, which git still reports with a non-zero exit code. Under the `draft_commit_conflicts` setting this workflow already passes to the action, that non-zero exit is read as a real conflict, and the action's attempt to commit the (empty) working tree fails too, surfacing as a "Backport failed" comment on the original PR, for a branch the change has been on for a while. Relates to #3588, which has the full mechanism and a reproduction. The guard only treats a target as done when a PR on that branch actually merged, so a real retry after someone manually closed a conflicting, unmerged backport PR still runs normally; both cases are covered by the new tests. This workflow triggers on `pull_request_target`, which, like `workflow_run`, executes the workflow definition from the PR's base branch (the default branch for the PRs this fires on), not from the PR head, for the same fork-isolation reason. That means this guard only takes effect once this PR merges to `main`; a `labeled` event on an in-flight PR before that still runs whatever version of `backport.yaml` is currently on `main`. The empty-cherry-pick-reported-as-conflict behavior itself lives in `korthout/backport-action` and stays out of scope here; issue #3588 links the upstream report. ### Screenshots Not applicable, no UI change. ### Downstream repositories <!-- Cozystack is upstream for repositories that are NOT kept in sync with it automatically, and nothing in CI compares the two sides, so a change here can break them silently. Which change forces what, and which file to touch, is the trigger map in docs/agents/contributing.md under "Downstream Repositories". Walk it before you tick anything. IF YOU ARE AN AI AGENT OR AN LLM, THIS IS PART OF THE TASK, NOT A FORM TO FILL IN: 1. Walk the trigger map against the actual diff of this PR, file by file. Do not decide from the PR title or from your memory of what you changed. 2. For every repository you tick, open the follow-up PR there and link it on the same line. A ticked box with no link claims work that does not exist, which is worse than an empty box. Search that repository for an open PR or issue covering it first, and link that instead of filing a duplicate. 3. When the follow-up is out of scope, or needs a decision that is not yours to make, open an issue in that repository instead, link that, and say so in the PR body. Do not push a speculative PR into another repository just to fill a line here. 4. Do not tick "No downstream repository is affected" to make the checklist go away. If you are unsure, leave every box empty and say so in the PR body, so a human decides. --> - [x] No downstream repository is affected by this change - [ ] [cozystack/website](https://github.com/cozystack/website) - follow-up: - [ ] [cozystack/terraform-provider-cozystack](https://github.com/cozystack/terraform-provider-cozystack) - follow-up: - [ ] [cozystack/ansible-cozystack](https://github.com/cozystack/ansible-cozystack) - follow-up: - [ ] [cozystack/ccp](https://github.com/cozystack/ccp) - follow-up: - [ ] [cozystack/talm](https://github.com/cozystack/talm) - follow-up: - [ ] [cozystack/cozyhr](https://github.com/cozystack/cozyhr) - follow-up: - [ ] [cozystack/cozy-proxy](https://github.com/cozystack/cozy-proxy) - follow-up: - [ ] [cozystack/cozystack-telemetry-server](https://github.com/cozystack/cozystack-telemetry-server) - follow-up: - [ ] [cozystack/external-apps-example](https://github.com/cozystack/external-apps-example) - follow-up: - [ ] [cozystack/examples](https://github.com/cozystack/examples) - follow-up: ### Release note <!-- Write a release note: - Explain what has changed internally and for users. - Start with the same `type(scope):` prefix as in the PR title - Follow the guidelines at https://github.com/kubernetes/community/blob/master/contributors/guide/release-notes.md. --> ```release-note ci(backport): skip re-running a backport whose target already has the merged change, instead of posting a false failure comment ``` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Bug Fixes** * Prevented duplicate backport pull requests by detecting matching merged backports before creating new ones. * Preserved the existing backport flow when no merged backport is found. * **Tests** * Added coverage for merged, closed, and empty pull request scenarios. * Verified that duplicate checks run before checkout and backport creation. <!-- end of auto-generated comment: release notes by coderabbit.ai --> |
||
|
Aleksei Sviridkin
|
1abc653cfb |
fix(ci): treat a disabled chainsaw suite as inert in test selection
Editing hack/e2e-chainsaw/backup/chainsaw-test.yaml.disabled selects the whole Chainsaw suite. The per-suite rule reads the suite name off the directory and ignores the suffix, so the change selects "backup"; the final intersection against the suites that actually exist finds no such suite and empties; and the safety net for an empty selection escalates to the full run. A disabled suite is registered nowhere and executed by nothing, which makes its edits the one file class that provably cannot regress a test. The mapping had it backwards: they bought the most expensive outcome the selector has. Rare in practice, and paid on every commit by anyone grooming or re-enabling such a suite. Classify *.disabled as inert ahead of the per-suite rule, and record it alongside the other rules that select nothing so the header stays an accurate enumeration. The safety net keeps its job for paths no rule has looked at, which is what it was added for. Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
Aleksei Sviridkin
|
a6c94c4528 |
fix(ci): correct an inverted caveat and clean up after the assertion
Two defects introduced by the previous two commits. The in-tree lane's comment said `skip=true` cannot mean "a workflow nobody classified", and then explained, after a colon, exactly how it can: `.github/` is inert as a directory and the e2e workflows are exempted from it by name, so a new one reads as inert. The clause meant to warn a reader instead told them the one unsafe case is safe, in the copy consulted when deciding whether to trust a green gate. The other three copies of the caveat state it correctly. The scratch-directory cleanup landed before the last assertion in every test added here. That passes the EXIT-trap guard and inverts the reason the guard exists: both runners set `-e`, so cleanup placed last is unreachable on failure and the fixture survives for inspection, which `docs/agents/e2e-testing.md` §3 states outright and the already-converted `hack/select-install_test.bats` follows. Placed early, a failing test deletes the evidence of its own failure, and one test had the call between two assertions, so which of them failed decided whether the directory survived. Assisted-By: Claude <noreply@anthropic.com> Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
Aleksei Sviridkin
|
bbee3d7a1b |
docs(ci): finish the sweep for the third copy of the inert claim
The previous pass corrected the same explanation in the selector header and in the in-tree lane and missed this one, which sits in the fork lane's selection step. It still said an empty selection means "verified inert" and pointed only at the inert pattern, and it lacked the caveat the other two copies gained: that a workflow added under `.github/workflows/` reads as inert until it is named in the escalation list. Three copies of one explanation is the underlying problem; leaving one of them a version behind is the way that problem shows up. A correction is not finished while a copy of the old claim is still alive. Assisted-By: Claude <noreply@anthropic.com> Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
Aleksei Sviridkin
|
55f3c79e9f |
docs(ci): stop pointing at the inert list for paths it does not hold
Four places said an empty selection means the inert pattern matched, and sent the reader to `inert_config_pattern` to see which paths those are. Documentation, dashboards and `*.md` are dropped before that pattern is ever consulted, and a backups example whose app has no suite is dropped where backup examples are matched. A reader checking the pattern for any of them finds nothing and has to reconstruct the order by reading the script. Name the rules that produce an empty selection instead, and keep the claim to the one the lanes rely on: empty is a decision some rule reached, not a path nothing looked at. The same bullet claimed every path is classified. One is not. The inert pattern makes `.github/` inert as a whole directory and the workflows that run the suite are exempted from it by name, so a workflow added under `.github/workflows/` reads as inert and the fall-through never sees it. Today's enumeration is correct and I checked it against `rg -l test-chainsaw`, so this is not a live hole, but the sentence saying it cannot exist is what makes the reminder next to that list look decorative. Record it as the guard it is. Separately, the fork lane's rationale for appending a newline described a mechanism this branch removes: the reader now guards its own loop, so the terminator is no longer what keeps the last path alive. Say that it is now the text-file contract and a second line of defence across two files, which is a reason to keep it and a different reason than the one written there. Assisted-By: Claude <noreply@anthropic.com> Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
Aleksei Sviridkin
|
548dfa9520 |
docs(ci): say what an empty selection actually means
Four places on this branch describe an empty selection as "classified inert", against a list of three outcomes that every path is said to land in. There is a fourth: an edit under examples/backups/<app>/ where the app has no Chainsaw suite selects nothing, decided inline where backup examples are matched rather than through the inert list. The rule is deliberate and explained where it lives; it just is not one of the three the summary enumerates. The property the two lanes actually depend on is weaker and true: an empty selection means some rule decided the change cannot affect a suite, never that nothing looked at the path. State that instead, in the header that defines the classes and in the privileged lane's comment that consumes the result. Assisted-By: Claude <noreply@anthropic.com> Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
Aleksei Sviridkin
|
a1496727a1 |
docs(ci): describe the classification the fork lane now gets
The privileged resolve step explains why the TIA inputs come from the base repo, and closed with a caveat about the selector: that it escalates only for an unrecognised path inside packages/ and silently selects nothing for a path it does not recognise at all, naming hack/*.mk as the example. That was true of the selector this branch replaces. Measured against the selector as it now stands, hack/common-envs.mk escalates to the full suite, and so does the fork workflow file itself. An empty selection no longer means the path went unrecognised; it means the path was classified as unable to affect what e2e exercises. The difference is the whole point of the change, and the caveat now contradicts it in the one comment a reader consults before touching the privileged lane. Assisted-By: Claude <noreply@anthropic.com> Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
Myasnikov Daniil
|
ee4a48278f |
refactor(ci): drop the unreachable empty-selection branch in both lanes
Both lanes ran `if [ "$FULL_E2E" = "true" ] || [ -z "$SELECTED_APPS" ]` under a step gated on `steps.select.outputs.skip != 'true'`. When `select` runs, an empty result sets `skip=true` and the step never starts, so the only way to arrive with no selection is the `full-e2e` label — which sets `FULL_E2E=true`. The second test could never decide anything the first had not, while the comment above it advertised "full suite when the selection is empty" in the file whose whole point is that a green status means tests ran. Testing `FULL_E2E` alone keeps the real invariant visible. The comments now say what an empty selection means after the preceding commit: every changed path was explicitly classified inert, not that the selector had no idea. Assisted-By: Claude <noreply@anthropic.com> Signed-off-by: Myasnikov Daniil <myasnikovdaniil2001@gmail.com> |
||
|
myasnikovdaniil
|
7dca642894 |
fix(ci): restore e2e for fork PRs and close the merge-gate bypass (#3262)
## What this PR does Fixes #3257 — fork PRs never ran e2e, and the skipped required check let them merge anyway. Fork PRs get no secrets on `pull_request`, so `make image` pushed anonymously and died with `denied`. That failed every build, collapsed the `build → finalize → e2e` chain, and left the required **E2E Tests** job *skipped* — which branch protection treats as satisfied, so an approved fork PR could merge with e2e never having run. This implements **Option A** from the issue (privileged `workflow_run` split), pushing fork e2e images to the existing CI registry (OCIR): **Build primitive (`hack/common-envs.mk`)** - New `OCI_EXPORT_DIR` mode: build each image to a per-image OCI archive instead of pushing, and force `PUSH`/`LOAD` off. The digest is captured via `--metadata-file` regardless of output type, so the refs baked into `pr.patch` match what the privileged run later pushes. Threaded through the `image-tags` macro every package already uses — no per-package Makefile change. **`pull-requests.yaml` (unprivileged, fork branch)** - `build` / `build-talos` / `finalize` export images to OCI archives and upload them as artifacts instead of pushing → the red `denied` build wall disappears and the chain no longer collapses. - The in-tree e2e job is renamed off `"E2E Tests"` and guarded to same-repo PRs. A new `e2e-report` job concludes the required `"E2E Tests"` context as an **explicit check-run** (success on a docs-only PR or when e2e passed, failure otherwise). A skipped job can no longer satisfy the required check — this also closes the same hole for same-repo PRs whose build fails. **`e2e-fork.yaml` (new, privileged `workflow_run` from the default branch)** - Resolves the fork PR, opens a pending `"E2E Tests"` check-run, and fails-closed if the fork build failed. - Pushes the pre-built image archives to OCIR **by digest** and flux-pushes the digest-pinned packages tree. The OCIR token is live only for trusted binaries (`git`/`skopeo`/`flux`/`yq`) over data — never for `make` or any fork-authored script. - Runs the e2e suite against the pushed closure with **no push token** and concludes the check-run on the PR head SHA. Fork build code never receives registry credentials; the fork's images and test scripts run only in the credential-less, ephemeral e2e job — a stronger posture than a `pull_request_target` run that would hand fork code the secrets directly. ### Validation Local: buildx OCI-archive output + digest capture, macro expansion for single- and multi-image packages, non-fork push path unchanged, `actionlint` clean. Needs live validation (why this is a **draft**): `workflow_run` artifact download across runs; that the `"E2E Tests"` commit status satisfies the required context and keeps a fork PR non-mergeable until the privileged run concludes it; `refs/pull/<N>/merge` fetch + `git apply --3way`; `skopeo --preserve-digests` end-to-end. `workflow_run` only runs from the default branch, so the privileged half is exercised by mirroring the workflow onto a personal fork's default branch and opening a throwaway fork PR into it — not by merging first. _Since review_: the fork `e2e` job ran the pre-#2826 BATS app loop that the merged Chainsaw migration deleted (already broken); it now runs the same Chainsaw + TIA path as the in-tree job, with suite selection and the `full-e2e` override sourced from `resolve`'s trusted base-repo file list / labels (not a merge-ref diff). Publish job hardened per the non-blocking follow-ups: base-tree image allowlist, `skopeo copy --all`, pinned flux CLI. The required `E2E Tests` gate is now published as a commit **status** (default token + `statuses:write`), not a COZYSTACK_CI app check-run — the app lacked `checks:write` (`checks.create` 403'd) and a default-token check-run floated under a labeler suite; a commit status has neither problem, and forks can't post one (their `pull_request` token is read-only). Two review-bot findings fixed too: fork `OCI_EXPORT_DIR` is now absolute (a relative path misplaced archives under `make -C <pkg>` and every fork build would fail at upload), and the "≥1 archive" guard no longer no-ops under `nullglob`. ### Repo-admin prerequisites One repository setting lives outside this diff: 1. **Branch protection: keep `E2E Tests` required.** The gate is now a commit **status** named `E2E Tests` (posted with the default `GITHUB_TOKEN` — see `docs/agents/e2e-testing.md` §10), not a job or an app check-run. Branch protection already requires `E2E Tests` (currently the e2e job's name, renamed here to `E2E (in-tree)`), so the required context now resolves to the status — **no GitHub App, no `checks:write` grant, no app-id pin needed**. Just don't pin the context to a specific app. 2. **Actions → "Require approval for all outside collaborators".** Each fork run must be a deliberate maintainer click before it executes, since the fork's unprivileged run produces the artifacts the privileged `e2e-fork.yaml` consumes. ### Downstream repositories Walked the trigger map in `docs/agents/contributing.md` against the diff (7 files: two CI workflows, `docs/agents/e2e-testing.md`, `hack/common-envs.mk` + its bats suite, and two package `Makefile`s). The one trigger worth naming is "change developer tooling (`cozyvalues-gen`, `cozypkg`, the package `Makefile`s) → `development.md`" on the website. It does not fire here: `OCI_EXPORT_DIR` is a CI-only build mode that defaults off, and the local developer workflow is unchanged — `make image` still builds and pushes exactly as before, which the new default-path case in `hack/common-envs_test.bats` now pins. No chart values, no API surface, no app list, no platform variant, no Talos version bump and no release asset rename are touched. - [x] No downstream repository is affected by this change ### Release note ```release-note fix(ci): fork pull requests now run e2e via a privileged workflow, and a fork PR can no longer merge on a skipped (never-run) required E2E Tests check ``` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit - **New Features** - Added a dedicated fork E2E workflow that runs after the base PR and reports results via the required **E2E Tests** commit status. - Fork PRs now export build images as OCI archive artifacts for later publishing. - Added a same-repo E2E reporting job and renamed the in-tree E2E flow to **E2E (in-tree)**. - **Bug Fixes** - Added tighter, fail-closed gating to ensure merge blocking when **E2E Tests** isn’t correctly finalized. - **Documentation** - Documented fork vs same-repo E2E split, OCI export behavior, and required fork security settings. - **Tests** - Added Bats coverage validating `OCI_EXPORT_DIR` and preventing unintended registry pushes. <!-- end of auto-generated comment: release notes by coderabbit.ai --> |
||
|
Aleksei Sviridkin
|
3af1ad381b |
fix(ci): count only the pull requests at the head, not near it
The guard requiring exactly one open pull request was written for PRs that share a head commit, but the endpoint it reads is broader than that. Per the API reference it lists pull requests ASSOCIATED with a commit, which for a commit outside the default branch means any open PR whose branch contains it. Stacking one branch on another and opening both therefore counts as two, and the lower one fails closed on a state its author cannot clear by pushing, because pushing does not remove the PR above it. That is the same unfixable red this workflow refuses to inflict elsewhere, and it costs a contributor the one thing fail-closed is supposed to protect: a way forward. Filter the associations by head SHA as well as state. Two pull requests genuinely pointing at the same commit still fail closed, which is the case the guard exists for and the case where a verdict computed for one PR would otherwise satisfy the other. Checked against all four shapes: stacked branches now resolve, a real collision still refuses, a lone PR resolves, and an association with no open PR at the head still refuses. Assisted-By: Claude <noreply@anthropic.com> Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
Aleksei Sviridkin
|
e2e591ac6a |
fix(ci): claim the fork status before the lookups that can throw
`resolve` posted its `pending` last, after four paginated API calls. Any of them returning 5xx fails the job, `should_run` is never set, `report` skips on it, and nothing writes the context at all. A head SHA that was already green then stays green with no suite behind it, for good rather than for the length of a build, and the privileged run's own red is invisible on the pull request because a `workflow_run` check suite hangs off the default branch. That is the exact state the opener exists to prevent, and the same-repo lane already treats this failure mode twice, with `continue-on-error` on its opener and a try/catch around its terminal post. Post the pending as soon as the run is known to own the SHA, which is immediately after the two silent returns. Three of the four lookups now happen with the context already claimed, so a failure in any of them leaves `pending` standing, and pending blocks. The jobs lookup keeps its place ahead of the opener because the discarded-label decision reads it, and posting before that decision is exactly what lets a run stamp a status it does not own. A failure there still leaves the SHA untouched; that half is a deliberate trade between two ways of being wrong, and it is now written down next to the code rather than implied by the ordering. Assisted-By: Claude <noreply@anthropic.com> Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
Aleksei Sviridkin
|
cc8b01d5cd |
docs(ci): say when each lane's status opener actually lands
The reference section claimed both lanes open the required context when their run starts, and enumerated the triggers it protects against: a re-run, a reopen, a `full-e2e` label. Every one of those starts a Pull Request run, and on a fork that run cannot open the status at all. The opener is guarded same-repo only, because a fork's `pull_request` token is read-only, and the fork's opener lives in the privileged workflow, which starts only once the unprivileged build has finished. So on the lane the sentence was most likely to be read about, an already-green head SHA keeps reading green for the whole build. The gap is structural. A read-only token cannot post a status at any point in the run, and that same read-only token is what makes publishing the gate with the default token safe in the first place. It is bounded by the build rather than open-ended. What was wrong was only the claim that it does not exist. Say so in all three places that carried the overstatement: the section itself, the opener's own comment, and the contract test, which pins that each lane has an opener and cannot express when it lands. Assisted-By: Claude <noreply@anthropic.com> Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
Aleksei Sviridkin
|
c45179a9d0 |
docs(ci): correct the reason the build matrix keeps the rename-detected list
The comment justified the split by saying a path that no longer exists has nothing to build. That is only half the picture: on a rename between packages the source package is still there and still needs rebuilding, and rename detection drops it from the list the matrix reads, so the matrix under-builds. The behaviour predates this change and is out of scope for a docs-only decision, but the comment presented it as deliberate and correct rather than as inherited and left alone. Assisted-By: Claude <noreply@anthropic.com> Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
Aleksei Sviridkin
|
9c68b1f83e |
fix(ci): stop a cancelled fork run from publishing a verdict
The invariant that a cancelled run publishes nothing was written down in three places and held in two of them. The fork lane's terminal reporter ran under `always()`, which includes cancellation. When a second privileged run supersedes the first, the dying one still posted `failure` on a head SHA it no longer owned, for a suite that never finished. It self-corrected once the superseding run reported, but that can be two hours later, and until then a legitimate pull request reads red. The same-repo lane was moved off `always()` for exactly this reason. Move this one too: a failed publish or e2e still reports, since neither is a cancellation, and a cancelled run now leaves the `pending` it opened standing until its successor concludes. The same-repo guard's rationale cited a scenario that no longer exists. It described a label event cancelling the run that opened the pull request, which the concurrency split made impossible for every label the workflow discards. The guard is still needed, for a push, for a `full-e2e` label superseding the head run, and for a manual cancel, so the comment now names those instead of an extinct one. The reference section listed the pieces this invariant rests on and omitted the reporters, which is the half a reader is most likely to touch, and counted them, which was already wrong and would go wrong again on the next addition. It now enumerates without counting. The contract pins both reporters, because both lanes have had this defect at different times in the same shape. Assisted-By: Claude <noreply@anthropic.com> Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
Aleksei Sviridkin
|
8a21085324 |
fix(ci): decide docs-only from both sides of a rename
The two lanes answered "is this docs-only" from different sources and could disagree. `plan` reads `git diff --name-only`, which applies rename detection and reports a rename as its new path alone, so moving a code file into docs/ read as docs-only. The fork lane reads the PR file list, where a rename carries `previous_filename`, and reads the same change as code. Disagreement is not a near-miss here. The same-repo lane builds nothing and uploads no patch artifact; the fork lane concludes e2e is required and then fails closed because that artifact is missing, reporting that the fork build did not complete. It completed — it correctly built nothing. There is no push the contributor can make to clear it and no label that overrides it, so a legitimate contribution is permanently red. Point the docs-only decision at the rename-blind list, which the job already computes for the downstream-trigger-map flag and whose comment names this exact trap. Both lanes now agree, and the same-repo side stops shipping a removed code path untested — until now such a change skipped every build and reported e2e as not required. The build matrix keeps reading the rename-detected list: it picks packages to rebuild, and a path that no longer exists has nothing to build. The contract file gains the agreement as a pin, and grows a header to match what it now covers. The fork half is pinned on the docs-only expression rather than on `previous_filename`, which also appears in the file list a few lines down and would have kept the assertion satisfied with the check deleted. Assisted-By: Claude <noreply@anthropic.com> Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
Aleksei Sviridkin
|
d3515509c8 |
fix(ci): let a status-API blip skip the opener instead of failing the run
The step that claims the required context is the first in `plan`, and `checks`, `build`, `build-talos`, `finalize` and `e2e` all hang off that job. A transient failure from the status API would therefore discard a full build and suite for a call that is an optimisation, not a gate. Mark it `continue-on-error`. Losing the opener degrades exactly to the behaviour that preceded it — the head SHA keeps whatever status it had until `e2e-report` concludes — while a run that dies before reporting leaves the merge blocked either way. The terminal post keeps its own error handling, because that one failing silently is what would let a run finish with no verdict at all. Assisted-By: Claude <noreply@anthropic.com> Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
Aleksei Sviridkin
|
ee844c98a5 |
fix(ci): open the "E2E Tests" status when the same-repo run starts
Moving the gate from a job to a commit status lost a property the job had for free. A commit status is latest-wins per (sha, context) and never expires, so a head SHA that already went green keeps reading green from the moment a new run starts until `e2e-report` posts at the end. Re-running all jobs, reopening, or adding full-e2e therefore leaves the pull request mergeable for the whole of a suite that may be about to fail. While the gate was a job, GitHub created a fresh non-success check run each time and the window did not exist. Claim the context at the start of `plan`, the way the fork lane already claims it in `resolve`, and let `e2e-report` conclude it as before. The call is same-repo only: a fork's `pull_request` token is read-only, so posting from there would 403 and fail the job on every fork PR — and that read-only token is what makes the whole commit-status design safe. Fork PRs get their opening status from the privileged workflow instead. The contract test grows a case pinning that each lane writes the context from two points, one of them pending. Both assertions on the same-repo opener are scoped to the `plan` job: the bare fork guard occurs on several steps in other jobs, so a whole-file "at least one" passed with the opener's guard deleted, pinning unrelated guards and reporting that as coverage. Assisted-By: Claude <noreply@anthropic.com> Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
Aleksei Sviridkin
|
3722b97fb5 |
fix(ci): terminate the fork TIA file list so its last path is read
The privileged lane builds the changed-file list in JavaScript and hands it to `hack/select-e2e.sh` through base64. `Array.join` puts a separator between entries and none at the end, so the decoded file had no trailing newline — and the selector reads it with `while IFS= read -r file`, which POSIX defines as returning non-zero at EOF without a newline. The loop body never runs for that last path, so it is silently dropped. For a single-file pull request, the modal fork contribution, that is the whole list: the selection comes back empty, the Chainsaw step is skipped on it, the job concludes success and the required status is published green with no suite executed. With more files the last one is dropped instead, which under-selects — and it can also suppress an escalation, since a path the selector does not recognise inside packages/ is exactly what would have widened the run to the full suite. Nothing in the log gave it away: the step prints the list with `sed`, which renders an unterminated last line the same as a terminated one, so the log showed a path the selector had never seen. Terminate the list where it is built. The in-tree lane is unaffected and always was — it feeds `git diff --name-only`, whose output ends with a newline — which is why this is the first caller to meet the behaviour. The read loop itself still drops an unterminated last line for any future caller; fixing that belongs with the selector and its own test surface, not here. Assisted-By: Claude <noreply@anthropic.com> Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
Aleksei Sviridkin
|
fd06fb32e0 |
docs(ci): say what fork TIA hardening buys, and what it does not
Four comments and a documentation bullet claimed that routing the changed-file list through the base-repo API stops a fork from making test selection pick nothing. It does not. The `e2e` job runs from the fork's own tree, so `hack/select-e2e.sh` is the fork's script: a PR that edits it to print nothing gets `skip=true`, the suite step never runs, the job succeeds and the required status goes green — the exact outcome the paragraph presented as prevented. The same holds for the suites under `hack/e2e-chainsaw/`, the install bats and the testing package's recipes, all of which the fork authors and this job executes. Hardening the inputs is still worth doing, since a merge-ref diff would be a second bypass on top of the one that exists, so the code does not change. What changes is the claim: the inputs are trusted, the decision is not, and the fork lane is a correctness gate against an honest contributor's regression rather than a boundary against a hostile fork. State plainly what does contain a hostile fork — the maintainer approval required before any fork run executes, the absence of any credential in that job, and a diff editing the harness being visible in review. Assisted-By: Claude <noreply@anthropic.com> Signed-off-by: Aleksei Sviridkin <f@lex.la> |