From 00a7c20ce5ebe353e891d5ea4565e41e531ee7a7 Mon Sep 17 00:00:00 2001 From: Cameron Smith Date: Thu, 10 Sep 2026 00:03:13 -0400 Subject: [PATCH] fix(gitea-mq): match the required-check fallback to the authoritative ruleset MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Ruleset 16212553 requires nixbot/nix-eval, nixbot/nix-build and nixbot/effects; the module's fallback listed only the first two. The fallback does not fire today, because ResolveRequiredChecks prefers a non-empty forge-derived list, but a fallback weaker than the forge is worse than none: it would silently reduce the gate exactly when the forge reports nothing. The downgrade path is narrower than a missing ruleset. GetRequiredChecks errors propagate rather than falling back, so an unreadable ruleset or lost read access stalls the queue instead of weakening it. The fallback fires only on a successful empty list — a ruleset deleted, emptied, retargeted off the default branch, or reduced to gitea-mq-owned contexts, which GetRequiredChecks subtracts. Second coupling, recorded because it is the one that bites: nixbot/effects is posted only when at least one effect runs. effects_run.py returns early on an empty effect set, so effects_started is never called and the context is never posted. A required check that is never posted blocks a PR rather than failing it, so requiring it is safe only while the default branch keeps effects_on_pull_requests true with a non-empty effect set. nixbot.toml and this list must move together. Change-Id: I06bf0508cf3cd63abb483139e7fe36786a6a6964 --- .../adr-substitution-first-rollup-landing.md | 16 ++++--- modules/nixos/gitea-mq.nix | 16 ++++--- .../stand-up-gitea-mq-on-magnetite/design.md | 45 +++++++++++++------ .../stand-up-gitea-mq-on-magnetite/plan.md | 23 +++++++--- .../proposal.md | 21 ++++++--- .../specs/merge-queue-interface/spec.md | 28 ++++++++---- .../specs/merge-queue-service/spec.md | 10 ++--- .../specs/world-assumptions/spec.md | 4 +- .../stand-up-gitea-mq-on-magnetite/tasks.md | 20 ++++++--- 9 files changed, 122 insertions(+), 61 deletions(-) diff --git a/docs/notes/development/version-control/adr-substitution-first-rollup-landing.md b/docs/notes/development/version-control/adr-substitution-first-rollup-landing.md index d3c5e9521..e34c36bdb 100644 --- a/docs/notes/development/version-control/adr-substitution-first-rollup-landing.md +++ b/docs/notes/development/version-control/adr-substitution-first-rollup-landing.md @@ -109,7 +109,8 @@ gitea-mq, GitHub backend: Batch state persists in Postgres and resumes after restart (`internal/store/pg/migrations/004_batches.sql::batches`; `internal/batch/batch.go::Engine.ReconcileLive`). - Required checks are forge-derived whenever that list is nonempty; `GITEA_MQ_REQUIRED_CHECKS` is only fallback (`internal/monitor/monitor.go::ResolveRequiredChecks`). GitHub unions rulesets and classic protection, excluding MQ-owned contexts (`internal/github/forge.go::GetRequiredChecks`). - Our ruleset's `nixbot/nix-eval` and `nixbot/nix-build` therefore supply the queue's required set; the environment fallback does not fire in the intended configuration. + Our ruleset's `nixbot/nix-eval`, `nixbot/nix-build`, and `nixbot/effects` therefore supply the queue's required set; the environment fallback does not fire in the intended configuration. + The fallback must nonetheless match the authoritative ruleset so an empty forge list cannot silently weaken the landing gate. - The queue is review-blind: a case-insensitive search of `internal/` for `pull_request_review|approved|review` returned no matches, and `internal/poller/poller.go::enqueuePR` gates only on `prCheckResult`. `internal/github/setup.go::EnsureRepoSetup` enables `allow_auto_merge`, attempts to add the App as a bypass actor on every other branch-target ruleset, and returns early if a ruleset named `gitea-mq` already exists. Repository rulesets can be updated; organization-owned rulesets without bypass and insufficient setup permissions produce warnings (`internal/github/setup.go::ensureBypass`). @@ -206,9 +207,9 @@ These triggers and greedy selection explain the observed split: batch size emerg Raising the cap raises the ceiling; it does not force larger batches. Any speedup from twenty is a projection, not a measured result. -We will enable repository `allow_auto_merge` and maintain two default-branch rulesets: ours requiring deletion protection, non-fast-forward protection, `nixbot/nix-eval`, and `nixbot/nix-build`; the App's requiring `gitea-mq`. +We will enable repository `allow_auto_merge` and maintain two default-branch rulesets: ours requiring deletion protection, non-fast-forward protection, `nixbot/nix-eval`, `nixbot/nix-build`, and `nixbot/effects`; the App's requiring `gitea-mq`. We will permit the queue App's ruleset bypass and use neither `required_linear_history` nor classic branch protection. -We will derive the required CI set from our ruleset; an environment fallback will not substitute for that ruleset. +We will derive the required CI set from our ruleset; an environment fallback will not substitute for that ruleset and must never be weaker than it. Technical justification: the batch engine fast-forwards to the exact tested SHA, and nixbot admits `gitea-mq/*` builds without branch-filter configuration. We will preserve substitution by filtering check sources, except the declared whole-tree scan allow-list, and warming magnetite's store with `just check-fast auto off x86_64-linux` before publication, so that nixbot classifies those check outputs `local` and skips them (R2). @@ -267,7 +268,7 @@ Automated checks to implement in the related OpenSpec changes: - Retain `filter-check-sources-for-substitution`'s `structure-check-source-isolation` check, negative control, and transitive `check-source-audit` probe against its declared whole-tree allow-list (`openspec/changes/filter-check-sources-for-substitution/proposal.md`, “Assertion of the property”). Unrelated source changes must leave unaffected filtered checks' derivation hashes stable; the deliberately whole-tree `gitleaks` scan is the declared exception. - Add a module assertion pinning `services.gitea-mq.batchMax = 20` and `skipQueueIfUpToDate = true`; assert no merge-label override, because `nix/module.nix::services.gitea-mq` exposes batch size but not the label. -- Add a read-only GitHub ruleset check requiring both `nixbot/nix-eval` and `nixbot/nix-build` in our default-branch ruleset. +- Add a read-only GitHub ruleset check requiring all three contexts, `nixbot/nix-eval`, `nixbot/nix-build`, and `nixbot/effects`, in our default-branch ruleset, and pin the configured fallback to the same set with a module assertion. Also verify the separate App-owned `gitea-mq` gate, bypass actor, absence of linear-history and classic protection, and `allow_auto_merge = true`. Checking only `GITEA_MQ_REQUIRED_CHECKS` is insufficient because `internal/monitor/monitor.go::ResolveRequiredChecks` prefers the forge-derived set. - Report batch SHA, landed PRs, and nixbot build identities using `web/api_routes.py::create_api_router`, `Build`, and `Attribute`. @@ -286,7 +287,7 @@ Manual checks before promotion from Proposed (existing V identifiers retained fo - V2. Confirm in vanixiets that existing green checks on the selected head are accepted when authorization arrives later, including the single-entry shortcut. Source supports this: `internal/poller/poller.go::prCheckResult`, `pollMergeBranchChecks`, and `internal/batch/monitor.go::Engine.HandleCheck` have no arrive-after-enqueue timestamp requirement. -- V3. Confirm replayed and newly posted check runs are exactly `nixbot/nix-eval` and `nixbot/nix-build` on the selected SHA (`status.py::ForgeStatusReporter`; `build_reuse.py::replay_terminal_status`). +- V3. Confirm the selected SHA carries required contexts `nixbot/nix-eval`, `nixbot/nix-build`, and `nixbot/effects`, checking build-result replay separately from effect execution (`status.py::ForgeStatusReporter`; `build_reuse.py::replay_terminal_status`; `effects_run.py::enqueue_effects`). - V4. Measure uncached derivations per batch after members' authors warm magnetite with `just check-fast auto off x86_64-linux`. Inspect the API proxy and logs to distinguish interaction inputs, cache misses, uploader failures, and unrelated-source invalidation; verify that unchanged filtered derivations substitute. - V9. Confirm the deployed module exposes `batchMax` and `skipQueueIfUpToDate`, and the binary retains its default merge label. @@ -334,7 +335,10 @@ Source filtering and cache warming are the core; process policies and upstream w - R6. Commit bodies retain the Linear issue id for traceability, without a rollup-specific transport bookmark or dependency-assembly contract. - R7. Set `build_branches = []` to suppress arbitrary non-default branch builds; queue patterns remain admitted independently. - R8. Keep unconditional PR builds without orchestrator-controlled publication timing. -- R9. Required external contexts are `nixbot/nix-eval` and `nixbot/nix-build`. +- R9. Required external contexts are `nixbot/nix-eval`, `nixbot/nix-build`, and `nixbot/effects` in both the authoritative ruleset and the configured fallback. + This extends the original eval/build pair after the operator added effects to ruleset `16212553` so landing waits for effect completion; `openspec/changes/stand-up-gitea-mq-on-magnetite/design.md::D2` records the reason and the coupled invariant. + Requiring effects depends on default-branch `nixbot.toml::effects_on_pull_requests = true` and a non-empty effect set: `effects_run.py::enqueue_effects` returns before `effects_started` on an empty set, leaving a required context unposted and a PR blocked indefinitely. + The ruleset, configured fallback, and effect-production configuration must move together. - R10. Scope PR previews with `effects_on_pull_requests` and non-default branch effects with `effects_branches`, preserving the previews identified in V10. - R11. Serve `mq.scientistexperience.net` with `batchMax = 20`, `skipQueueIfUpToDate = true`, and the default `merge-queue` label; provision its database and reverse proxy. - R12. Publish stacks with `--github-native` and verify registration and selected-head ancestry before enqueue; `Depends-On:` alone does not enable queue stack resolution. diff --git a/modules/nixos/gitea-mq.nix b/modules/nixos/gitea-mq.nix index 9d9631449..a5a1e729e 100644 --- a/modules/nixos/gitea-mq.nix +++ b/modules/nixos/gitea-mq.nix @@ -18,11 +18,13 @@ # an ExecStartPre into the local Gitea unit, which this GitHub queue does # not use. # -# Before first start, apply the approved G2 ruleset edit: add nixbot/nix-eval -# beside the existing nixbot/nix-build required check on our default-branch -# ruleset. gitea-mq derives its required set from the forge whenever that set -# is non-empty, so both nixbot contexts must live in our ruleset; the -# requiredChecks below is a fallback that does not fire in this configuration. +# The original G2 edit added nixbot/nix-eval beside nixbot/nix-build; the +# operator later added nixbot/effects so landing waits for effects to finish. +# gitea-mq prefers the non-empty forge-required list, so requiredChecks below +# remains an inactive fallback. It must match all three contexts nonetheless: +# an empty forge list must not silently weaken the landing gate. +# Requiring effects also needs default-branch nixbot.toml to keep PR effects +# enabled and a non-empty effect set, or that context is never posted. # Startup setup adds its own second ruleset named gitea-mq carrying only the # queue's context, and adds the App as a bypass actor on ours. Installation # selection must remain vanixiets alone: github.repos adds repositories to @@ -72,6 +74,7 @@ requiredChecks = [ "nixbot/nix-eval" "nixbot/nix-build" + "nixbot/effects" ]; }; @@ -110,8 +113,9 @@ cfg.requiredChecks == [ "nixbot/nix-eval" "nixbot/nix-build" + "nixbot/effects" ]; - message = "services.gitea-mq.requiredChecks must be exactly nixbot/nix-eval and nixbot/nix-build per ${adr}"; + message = "services.gitea-mq.requiredChecks must be exactly nixbot/nix-eval, nixbot/nix-build, and nixbot/effects to match the authoritative ruleset without weakening the fallback per ${adr}"; } { assertion = !(config.systemd.services.gitea-mq.environment ? GITEA_MQ_MERGE_LABEL); diff --git a/openspec/changes/stand-up-gitea-mq-on-magnetite/design.md b/openspec/changes/stand-up-gitea-mq-on-magnetite/design.md index e51a2ae71..9171cb005 100644 --- a/openspec/changes/stand-up-gitea-mq-on-magnetite/design.md +++ b/openspec/changes/stand-up-gitea-mq-on-magnetite/design.md @@ -4,7 +4,7 @@ Magnetite is the Hetzner CX53 host that carries this fleet's server-side service Its composition is the repository's deferred-module pattern: `flake.nix` hands `modules/` to `import-tree`, each aspect file assigns a deferred module into `flake.modules.nixos.`, `modules/machines/nixos/magnetite/default.nix` imports upstream modules and names the aspects the host takes, and `modules/clan/inventory/machines.nix` binds the result into `clan.machines.magnetite`, which `clan machines update magnetite` deploys. nixbot sits in that pattern as `inputs.nixbot.nixosModules.nixbot` imported at the host plus the aspect `flake.modules.nixos.nixbot` in `modules/nixos/nixbot.nix`, and serves `cameronraysmith/vanixiets` and `sciexp/ironstar` under the App `sciexp-nixbot` (id `4743700`). -The ADR `docs/notes/development/version-control/adr-substitution-first-rollup-landing.md` decides that changes land through gitea-mq's own bors-style batching: up to twenty queue entries tested together, checks read from nixbot's two contexts, and `main` fast-forwarded by non-force `UpdateRef` to the exact commit CI tested. +The ADR `docs/notes/development/version-control/adr-substitution-first-rollup-landing.md` decides that changes land through gitea-mq's own bors-style batching: up to twenty queue entries tested together, checks read from nixbot's three contexts, and `main` fast-forwarded by non-force `UpdateRef` to the exact commit CI tested. Revision 2 of that ADR retired the earlier orchestrator rollup onto `staging`, because `internal/batch/batch.go::Engine.HandlePass` fast-forwards the target to the tested batch SHA, so a separately assembled linear candidate adds no substitution guarantee. The ADR assigns this change R11 (the service and its four settings), R13 (a separate GitHub App), R14 (the two rulesets, App bypass, `allow_auto_merge`, no linear history, no classic protection), the second automated Compliance item (pinning the settings), and the world-assumption entries about gitea-mq and GitHub. R15 and R16, the human-and-agent authorization procedure, and R1 and R2, source filtering and cache warming, belong to sibling changes; this change references them where a dependency needs stating and does not absorb them. @@ -28,7 +28,7 @@ The earlier orchestrator-only labelling and bypass role is retired with the roll A gitea-mq instance on magnetite, served at `mq.scientistexperience.net` over TLS, deployed by `clan machines update magnetite`. The four R11 settings in force and pinned by an assertion that fails host evaluation on drift. A dedicated GitHub App with exactly the R13 permission and subscribable-event set, its credentials supplied through clan vars with no value in the repository. -The two-ruleset arrangement on `cameronraysmith/vanixiets` per R14: our ruleset gains `nixbot/nix-eval` by an operator-approved diff, the queue's own ruleset is created by its startup setup, and the `allow_auto_merge` question is settled. +The two-ruleset arrangement on `cameronraysmith/vanixiets` per R14: our ruleset requires `nixbot/nix-eval`, `nixbot/nix-build`, and `nixbot/effects`, the queue's own ruleset is created by its startup setup, and the `allow_auto_merge` question is settled. Runtime confirmation of V2, V3, and V9, with discharged V1 re-confirmed at the first live stacked landing. **Non-Goals:** @@ -50,7 +50,7 @@ Any upstream filing to gitea-mq. ### D2: The four settings are module options, the label is a default, and an assertion pins all four -- **Choice**: `batchMax = 20`, `skipQueueIfUpToDate = true`, `requiredChecks = [ "nixbot/nix-eval" "nixbot/nix-build" ]`; no value for the merge label; `assertions` in the aspect that read `config.services.gitea-mq.batchMax`, `.skipQueueIfUpToDate`, and `.requiredChecks` from the merged configuration and compare them to those values, plus one asserting `!(config.systemd.services.gitea-mq.environment ? GITEA_MQ_MERGE_LABEL)`. +- **Choice**: `batchMax = 20`, `skipQueueIfUpToDate = true`, `requiredChecks = [ "nixbot/nix-eval" "nixbot/nix-build" "nixbot/effects" ]`; no value for the merge label; `assertions` in the aspect that read `config.services.gitea-mq.batchMax`, `.skipQueueIfUpToDate`, and `.requiredChecks` from the merged configuration and compare them to those values, plus one asserting `!(config.systemd.services.gitea-mq.environment ? GITEA_MQ_MERGE_LABEL)`. - **Reversal, zero to five**: the earlier design chose unlimited `batchMax = 0` while relying on orchestrator serialization to supply one rollup entry at a time; zero itself never guaranteed a singleton (`internal/queue/batch.go::Service.FormBatch`). The rollup rested on a false premise about queue-created merge commits. `internal/batch/batch.go::Engine.HandlePass` calls `internal/github/forge.go::FastForward` with the exact tested batch SHA, preserving nixbot's tested-tree identity (`gitrepo.py::WorkTree.tree_hash`) even with merge history. @@ -69,7 +69,19 @@ Any upstream filing to gitea-mq. Raising the cap raises the ceiling; it does not force larger batches. Any speedup from twenty is a projection, not a measured result. - **Rationale**: the module maps the three options straight to `GITEA_MQ_BATCH_MAX`, `GITEA_MQ_SKIP_QUEUE_IF_UP_TO_DATE`, and `GITEA_MQ_REQUIRED_CHECKS`; `GITEA_MQ_MERGE_LABEL` is not an option, the module's `environment` set is closed, and `config.go::Load` defaults the label to `merge-queue`, so the only drift possible is an override from another module, which is what the fourth assertion detects. The assertions read the merged configuration, so an `lib.mkForce` elsewhere or a one-sided edit fails `checks.x86_64-linux.nixos-magnetite` at evaluation. The ADR's Compliance item allows a flake check or a module assertion; the assertion needs no new check attribute. -- **Coupling, not two settings**: `requiredChecks` is a fallback the queue consults only when the forge names no required check (`internal/monitor/monitor.go::ResolveRequiredChecks`), and D7 keeps both nixbot contexts in our ruleset, so the forge list is non-empty and this fallback never fires. The assertion pins the fallback; D7 pins what is actually operative. +- **Extension, two required contexts to three**: the original G2 decision added `nixbot/nix-eval` beside `nixbot/nix-build`; the operator subsequently added `nixbot/effects` to ruleset `16212553` so landing waits for herculesCI-style effects to conclude. + The operator reported that this also fixed orphaned `gitea-mq/*` mirror check runs: they had been copied mid-flight and never updated after the queue entry finalized. + The configured fallback and its assertion now include effects, superseding the two-context fallback rather than preserving a weaker gate merely because it is inactive. +- **Coupled invariant, forge and fallback**: the ruleset is authoritative, and `requiredChecks` must never be weaker than its required external contexts. + `internal/github/forge.go::GetRequiredChecks` unions ruleset and classic-protection contexts minus queue-owned contexts; `internal/monitor/monitor.go::ResolveRequiredChecks` prefers that non-empty list, so the fallback does not fire in this configuration. + If the forge returns an empty list, the fallback must still require `nixbot/nix-eval`, `nixbot/nix-build`, and `nixbot/effects`: a fallback that silently reduces the gate is worse than no fallback. + Forge-read errors propagate from `ResolveRequiredChecks`; they do not activate the fallback. + D2's assertion pins the configured list during magnetite evaluation; D7's read-only forge verification checks the authoritative list, which the assertion cannot inspect. +- **Coupled invariant, effects gate and effect production**: nixbot posts `nixbot/effects` only when at least one effect runs. + In nixbot's `nixbot/nixbot/effects_run.py::enqueue_effects`, an empty effect set returns before `effects_started` is called, so no context is posted. + A required context that is never posted blocks a PR indefinitely rather than failing it (`internal/monitor/monitor.go::EvaluateChecks`). + Requiring `nixbot/effects` is therefore safe only while the default branch keeps `nixbot.toml::effects_on_pull_requests = true` and a non-empty effect set that runs for each gated candidate. + `nixbot.toml`, the effect declarations, the authoritative ruleset, and this required-check list must move together; disabling PR effects or removing the last effect requires a coordinated gate change, not an isolated configuration edit. - **Alternatives considered**: setting `GITEA_MQ_MERGE_LABEL` on the unit directly, rejected because it duplicates a default that already holds and creates the override the assertion exists to catch. A structure check under `modules/checks/structure/` evaluating the magnetite configuration, rejected as a second place to keep the same four values. - **Boundary**: source-versus-delivered. The assertion is a property of evaluation and says nothing about the running process; the runtime tasks read the unit's environment on the host. @@ -100,12 +112,19 @@ Any upstream filing to gitea-mq. - **Alternatives considered**: reusing `sciexp-nixbot`, rejected above. Registering without Administration so that `EnsureRepoSetup` is skipped, rejected because R13 fixes the set and the ADR relies on setup to create the queue's own ruleset and to keep the App a bypass actor on ours. - **Trust boundary**: the installation selection is externally maintained forge state, not a restriction enforced by this NixOS configuration. D11 requires complete external verification of the one-repository installation scope before deployment authorization. -### D7: Two rulesets — ours gains one check context, the queue creates its own +### D7: Two rulesets — ours requires three contexts, the queue creates its own -- **Choice**: ruleset `16212553` on `cameronraysmith/vanixiets` keeps its name, its `deletion` and `non_fast_forward` rules, its existing `nixbot/nix-build` required check pinned to App `4743700`, and its repository-admin bypass, and gains `nixbot/nix-eval` beside `nixbot/nix-build`. Nothing is renamed, nothing is removed, no linear-history rule is added, and classic branch protection stays absent. The queue's `EnsureRepoSetup` creates a second ruleset named `gitea-mq` requiring only its own status pinned to the queue's App, and adds the queue's App as a bypass actor on ours. The single-context addition is presented as a diff for approval (gate G2) before it is applied. +- **Choice**: ruleset `16212553` on `cameronraysmith/vanixiets` keeps its name, its `deletion` and `non_fast_forward` rules, its `nixbot/nix-build` required check pinned to App `4743700`, and its repository-admin bypass, and also requires `nixbot/nix-eval` and `nixbot/effects`. + Nothing is renamed, nothing is removed, no linear-history rule is added, and classic branch protection stays absent. + The queue's `EnsureRepoSetup` creates a second ruleset named `gitea-mq` requiring only its own status pinned to the queue's App, and adds the queue's App as a bypass actor on ours. + The original G2 approval covered only the eval addition; the subsequent operator-added effects requirement extends that decision for the reason recorded in D2 and is matched here without another forge mutation. - **Reversal**: an earlier revision chose the opposite edit — rename `16212553` to `gitea-mq`, add `required_linear_history`, drop `nixbot/nix-build`, and require only the queue's status — so that the forge list would be empty and `GITEA_MQ_REQUIRED_CHECKS` would become operative. It is retired for two reasons. Linear history was a consequence of the abandoned one-entry rollup and no surveyed deployment mandates it. And `internal/github/setup.go::EnsureRepoSetup` returns early only when a ruleset *named* `gitea-mq` already exists, so pre-creating one was a way to suppress a second ruleset that is not needed: SBEE-Lab/infra and mulatta/dots both run two rulesets live, a human-owned one carrying `deletion`, `non_fast_forward`, and their build service's two contexts, beside the App-owned one carrying only `gitea-mq`. -- **Rationale**: `internal/monitor/monitor.go::ResolveRequiredChecks` prefers the forge's list whenever it is non-empty and consults `GITEA_MQ_REQUIRED_CHECKS` only when it is empty, and `internal/github/forge.go::GetRequiredChecks` unions ruleset and classic contexts minus the queue's own. Keeping both nixbot contexts in our ruleset therefore makes the forge the operative source and the pair exactly what the queue requires; a ruleset naming one context only would silently shrink the queue's gate to that one. This is one invariant with D2 rather than two independent settings: the module's `requiredChecks` is the fallback, and it does not fire in this configuration. -- **Alternatives considered**: leaving our ruleset unchanged and relying on `requiredChecks` is rejected because its non-empty `[nixbot/nix-build]` list suppresses the fallback pair. Pre-creating a disabled ruleset named `gitea-mq` remains optional, solely to choose when the queue context starts blocking. Setup returns early for that name without creating, repairing, or activating it, so the operator must separately approve activation; otherwise setup creates its own active second ruleset. +- **Rationale**: `internal/monitor/monitor.go::ResolveRequiredChecks` prefers the forge's list whenever it is non-empty and consults `GITEA_MQ_REQUIRED_CHECKS` only when it is empty, and `internal/github/forge.go::GetRequiredChecks` unions ruleset and classic contexts minus the queue's own. + Keeping all three nixbot contexts in our ruleset makes the forge authoritative; a non-empty proper subset would silently shrink the queue's gate despite the stronger fallback. + D2 and D7 form one invariant: the inactive fallback must match the authoritative gate so an empty forge list cannot weaken it, and requiring effects remains coupled to default-branch PR-effect enablement and a non-empty effect set. +- **Alternatives considered**: leaving our ruleset unchanged and relying on `requiredChecks` was rejected because its non-empty `[nixbot/nix-build]` list suppressed the original fallback pair; it would likewise suppress today's three-context fallback. + Pre-creating a disabled ruleset named `gitea-mq` remains optional, solely to choose when the queue context starts blocking. + Setup returns early for that name without creating, repairing, or activating it, so the operator must separately approve activation; otherwise setup creates its own active second ruleset. - **Retired verification**: V6's `refs/landings/*` probe is removed because the revised ADR retires that provenance protocol. ### D8: `allow_auto_merge` stays on, and the enqueue signal is the authorization @@ -140,7 +159,7 @@ Any upstream filing to gitea-mq. [Risk] A later edit changes batch size, skipping, the configured fallback, or the label → Mitigation: D2's assertions reject that drift during host evaluation; merge commits in tested batches are expected, not a configuration failure. -[Risk] A nixbot context disappears from our ruleset and the queue gates only on the remaining context → Mitigation: D7 and tasks 11.4/11.5 read both rulesets and classic-protection state to establish the exact forge-derived pair; visibility of the resolved list in logs or the dashboard is unverified. +[Risk] A nixbot context disappears from our ruleset and the queue gates only on the remaining contexts → Mitigation: D7 and tasks 11.4/11.5 read both rulesets and classic-protection state to establish the exact forge-derived three-context set; visibility of the resolved list in logs or the dashboard is unverified. [Risk] Peer authentication fails because the role name and the dynamic user name diverge → Mitigation: D3 records the coupling; the deployment task checks the unit's log for a successful migration rather than inferring from the role's existence. @@ -169,7 +188,7 @@ Write the aspect with both generators, the service configuration, the PostgreSQL Generate the webhook secret. Instantiate the host's configuration as a check. Apply the DNS record and confirm resolution. -G2: present the single-context ruleset addition for `cameronraysmith/vanixiets` and wait for approval; apply it. +G2 originally presented the single-context eval addition for `cameronraysmith/vanixiets` for approval before application; the operator's later effects addition extends the required set as recorded in D2/D7. Deploy with `clan machines update magnetite`. Confirm that setup created its own ruleset if absent and added App bypass to ours without changing our rules, that the App's webhook URL matches the service, and that V2, V3, and V9 hold. Re-confirm discharged V1 at the first live stacked landing; the old V6 ref probe is retired. @@ -177,9 +196,9 @@ Re-confirm discharged V1 at the first live stacked landing; the old V6 ref probe Rollback is the ordinary one: remove `gitea-mq` from the host's aspect list and redeploy, which withdraws the unit, the vhost, and the assertions. The database and role, the cache directory, the credential entries, the App registration, and both rulesets persist after such a rollback. The App-owned queue gate still requires `gitea-mq` after service removal; an operator-approved disabling of that gate accompanies rollback so the branch is not left awaiting a stopped service. -Record that separately from the G2 reverse diff, which removes only the added eval context from ours and preserves its build check and protection rules. +Record that separately from the original G2 reverse diff, which removes only the added eval context from ours and preserves its build check and protection rules; the later effects requirement and fallback must remain coordinated under D2. -Acceptance is the integration verification in tasks.md: the hostname serves over TLS, the unit is running with the four settings visible in its environment, the database exists owned by its role and the schema is migrated, the App's webhook URL reads the service's endpoint, both rulesets read as approved and as created by startup, and an up-to-date pull request whose tip already carries both nixbot contexts is landed by fast-forward. +Acceptance is the integration verification in tasks.md: the hostname serves over TLS, the unit is running with the four settings visible in its environment, the database exists owned by its role and the schema is migrated, the App's webhook URL reads the service's endpoint, both rulesets read as approved and as created by startup, and an up-to-date pull request whose tip already carries all three nixbot contexts is landed by fast-forward. ## Gate 1 modality verdicts @@ -225,6 +244,6 @@ Whether the operator wants the App owned by `sciexp` and public, as `sciexp-nixb Whether to pre-create a disabled `gitea-mq` ruleset or let setup create it active is an optional timing preference, not a prerequisite. If pre-created, setup does not create or activate it; the operator separately chooses activation timing. -The old explicit-User-bypass question and V6 namespace question are retired: G2 adds only `nixbot/nix-eval`, and the ADR removes the orchestrator provenance protocol. +The old explicit-User-bypass question and V6 namespace question are retired: G2 originally added only `nixbot/nix-eval`, D2/D7 record the later `nixbot/effects` extension, and the ADR removes the orchestrator provenance protocol. The `world-assumptions` designation table is modified by this change and by `stand-up-nixbot-on-magnetite`, which is In Review and not yet archived; archive applies MODIFIED by full-text replacement, so this change's table carries the union of the corpus rows, the sibling's rows, and its own, and whichever change archives second must carry the other's rows or lose them. diff --git a/openspec/changes/stand-up-gitea-mq-on-magnetite/plan.md b/openspec/changes/stand-up-gitea-mq-on-magnetite/plan.md index 4a66f04fe..da16f0c23 100644 --- a/openspec/changes/stand-up-gitea-mq-on-magnetite/plan.md +++ b/openspec/changes/stand-up-gitea-mq-on-magnetite/plan.md @@ -25,11 +25,15 @@ Its positional system parameter adds `--remote magnetite.zt --no-download` (`jus ## Global constraints - Nothing under `modules/nixos/nixbot.nix`, `modules/nixos/buildbot.nix`, their generators, vhosts, or databases is edited; `sciexp-nixbot` (id `4743700`) is not touched. Verified per task by `git diff --stat`. -- The four landing settings are `batchMax = 20`, `skipQueueIfUpToDate = true`, `requiredChecks = [ "nixbot/nix-eval" "nixbot/nix-build" ]`, and no merge-label override; the assertions in the aspect are the only place `GITEA_MQ_MERGE_LABEL` appears under `modules/`. +- The four landing settings are `batchMax = 20`, `skipQueueIfUpToDate = true`, `requiredChecks = [ "nixbot/nix-eval" "nixbot/nix-build" "nixbot/effects" ]`, and no merge-label override; the assertions in the aspect are the only place `GITEA_MQ_MERGE_LABEL` appears under `modules/`. - Both generator files stay at the default owner `root`; the module reads them through `LoadCredential` and no static `gitea-mq` user exists. - `listenAddr` is `127.0.0.1:8092`; `:8080` is bound by the LiveKit JWT service (`modules/nixos/matrix.nix`). - `hideRefFromClients = false`; the default would inject an `ExecStartPre` into `systemd.services.gitea` on this host. -- G2 adds only `nixbot/nix-eval` to our existing ruleset before first deployment; installation setup creates the App's separate `gitea-mq` ruleset and adds App bypass to ours. Both nixbot contexts must remain forge-required: `internal/github/forge.go::GetRequiredChecks` unions ruleset/classic contexts minus queue-owned contexts, and `internal/monitor/monitor.go::ResolveRequiredChecks` prefers that non-empty list over the configured fallback. No linear-history rule or classic protection is intended. +- G2 originally added only `nixbot/nix-eval` to our existing ruleset; the operator later added `nixbot/effects` so landing waits for effects, as recorded in `design.md::D2`. + Installation setup creates the App's separate `gitea-mq` ruleset and adds App bypass to ours. + All three nixbot contexts must remain forge-required: `internal/github/forge.go::GetRequiredChecks` unions ruleset/classic contexts minus queue-owned contexts, and `internal/monitor/monitor.go::ResolveRequiredChecks` prefers that non-empty list over the configured fallback. + The inactive fallback must match the authoritative list and must never weaken it; requiring effects also depends on default-branch `nixbot.toml::effects_on_pull_requests = true` and a non-empty effect set. + No linear-history rule or classic protection is intended. - No upstream issue or pull request is opened against gitea-mq anywhere in this work. - Long or output-heavy commands are captured: ` 2>&1 | tee logs/-$(date +%Y%m%d-%H%M%S).log`. - Verification of nix-managed outputs is by `nix eval` against `.#nixosConfigurations.magnetite` and by instantiation of `.#checks.x86_64-linux.nixos-magnetite.drvPath`; realization is deferred to the deployment on the host for the reason `stand-up-nixbot-on-magnetite` task 7.1 records. @@ -208,6 +212,7 @@ The generator declaration commits with Task 4; the vars entries commit as `clan requiredChecks = [ "nixbot/nix-eval" "nixbot/nix-build" + "nixbot/effects" ]; }; @@ -237,7 +242,7 @@ Verify: the three `nix eval` commands in tasks.md 4.1. - [ ] **Step 2: Confirm the environment (tasks.md 4.2)** Run: `nix eval .#nixosConfigurations.magnetite.config.systemd.services.gitea-mq.environment --apply 'e: { inherit (e) GITEA_MQ_BATCH_MAX GITEA_MQ_SKIP_QUEUE_IF_UP_TO_DATE GITEA_MQ_REQUIRED_CHECKS; label = e ? GITEA_MQ_MERGE_LABEL; }' --json` -Expected: `{"GITEA_MQ_BATCH_MAX":"20","GITEA_MQ_REQUIRED_CHECKS":"nixbot/nix-eval,nixbot/nix-build","GITEA_MQ_SKIP_QUEUE_IF_UP_TO_DATE":"true","label":false}`. +Expected: `{"GITEA_MQ_BATCH_MAX":"20","GITEA_MQ_REQUIRED_CHECKS":"nixbot/nix-eval,nixbot/nix-build,nixbot/effects","GITEA_MQ_SKIP_QUEUE_IF_UP_TO_DATE":"true","label":false}`. - [ ] **Step 3: Assertions (tasks.md 4.3)** @@ -261,8 +266,9 @@ Expected: `{"GITEA_MQ_BATCH_MAX":"20","GITEA_MQ_REQUIRED_CHECKS":"nixbot/nix-eva cfg.requiredChecks == [ "nixbot/nix-eval" "nixbot/nix-build" + "nixbot/effects" ]; - message = "services.gitea-mq.requiredChecks must be exactly nixbot/nix-eval and nixbot/nix-build per ${adr}"; + message = "services.gitea-mq.requiredChecks must be exactly nixbot/nix-eval, nixbot/nix-build, and nixbot/effects to match the authoritative ruleset without weakening the fallback per ${adr}"; } { assertion = !(config.systemd.services.gitea-mq.environment ? GITEA_MQ_MERGE_LABEL); @@ -384,7 +390,10 @@ Capture all five in `logs/rulesets-before-$(date +%Y%m%d-%H%M%S).log` and verify - [ ] **Step 2: Present the diff and stop (tasks.md 8.2; operator gate, an agent MUST NOT tick)** -Present, verbatim, and do not proceed until the operator approves or amends: +The diff and apply instructions below record the original G2 eval addition, not today's complete required set. +The operator subsequently added `nixbot/effects`; verify today's three-context set read-only under `design.md::D2`, without replaying the historical mutation or removing effects. + +The original gate required presenting the following verbatim and waiting for operator approval or amendment: ```text BEFORE ruleset 16212553 "nixbot" target branch enforcement active include ~DEFAULT_BRANCH @@ -466,8 +475,8 @@ Record every observation in verify.md with the `[operator]` and `[verified here] - [ ] **Step 1: Hostname and certificate, both services (11.1)** - [ ] **Step 2: Webhook endpoint registered by the service; accepted and rejected arms (11.2)** - [ ] **Step 3: Database and role for the dynamic user (11.3)** -- [ ] **Step 4: Two rulesets, both nixbot contexts, App bypass, no linear-history/classic protection, `allow_auto_merge` true (11.4)** -- [ ] **Step 5: V3 check-run names and the forge-derived required-check pair; fallback inactive (11.5)** +- [ ] **Step 4: Two rulesets, all three nixbot contexts, App bypass, no linear-history/classic protection, `allow_auto_merge` true (11.4)** +- [ ] **Step 5: V3 check-run names and the forge-derived three-context required set; matching fallback inactive (11.5)** - [ ] **Step 6: Four settings in the running unit's environment, including batch maximum 20 (11.6)** - [ ] **Step 7: V2 singleton original-head shortcut under `batchMax = 20`, ordinary auto-merge (11.7)** - [ ] **Step 8: Re-confirm discharged V1 at the first live stacked landing; retired V6 ref probe removed (11.8)** diff --git a/openspec/changes/stand-up-gitea-mq-on-magnetite/proposal.md b/openspec/changes/stand-up-gitea-mq-on-magnetite/proposal.md index 6ba6312d1..5315d3150 100644 --- a/openspec/changes/stand-up-gitea-mq-on-magnetite/proposal.md +++ b/openspec/changes/stand-up-gitea-mq-on-magnetite/proposal.md @@ -45,7 +45,7 @@ The 2026-09-09 ADR revision retires the orchestrator rollup onto `staging` becau **Four settings fixed by the ADR, and pinned** - From: nothing. -- To: `batchMax = 20`, `skipQueueIfUpToDate = true`, `requiredChecks = [ "nixbot/nix-eval" "nixbot/nix-build" ]`, and the merge label left at gitea-mq's default `merge-queue`; a NixOS assertion in the aspect reads the merged configuration back and fails evaluation of the host if any of the three options drifts or if any module sets `GITEA_MQ_MERGE_LABEL` on the unit. +- To: `batchMax = 20`, `skipQueueIfUpToDate = true`, `requiredChecks = [ "nixbot/nix-eval" "nixbot/nix-build" "nixbot/effects" ]`, and the merge label left at gitea-mq's default `merge-queue`; a NixOS assertion in the aspect reads the merged configuration back and fails evaluation of the host if any of the three options drifts or if any module sets `GITEA_MQ_MERGE_LABEL` on the unit. - Reason: gitea-mq's batch engine fast-forwards the target to the exact commit CI tested (`internal/batch/batch.go::Engine.HandlePass` calling `internal/github/forge.go::FastForward`), so testing up to twenty entries together preserves the tested tree the substitution argument depends on. Twenty supersedes the prior five-entry cap chosen to match Mic92/dotfiles, SBEE-Lab/infra, and mulatta/dots; `design.md::D2` preserves both decisions and their reasons. Our twice-weekly flake-update lane produces waves of 20–40 simultaneously-ready PRs (`.github/workflows/update-flake-inputs.yaml::on.schedule`) absent from those reference deployments. @@ -59,7 +59,9 @@ The 2026-09-09 ADR revision retires the orchestrator rollup onto `staging` becau These triggers and greedy selection explain the observed split: batch size emerges from arrival rate versus build time. Raising the cap raises the ceiling; it does not force larger batches, and any speedup from twenty remains a projection rather than a measured result. The up-to-date shortcut lets a single ready entry land its own head without a further build. - The configured checks are a fallback the queue consults only when the forge names none, and the merge label is not a module option, so the only drift possible there is an override, which is what the fourth assertion catches. + The configured checks are a fallback the queue consults only when the forge names none; the fallback must match the authoritative three-context ruleset so it cannot silently weaken the gate. + Requiring effects also depends on default-branch `nixbot.toml::effects_on_pull_requests = true` and a non-empty effect set (`design.md::D2`). + The merge label is not a module option, so the only drift possible there is an override, which is what the fourth assertion catches. - Impact: any later edit to one of these values, in this aspect or by `lib.mkForce` elsewhere, fails `checks.x86_64-linux.nixos-magnetite`. **A dedicated GitHub App, separate from nixbot's** @@ -68,11 +70,16 @@ The 2026-09-09 ADR revision retires the orchestrator rollup onto `staging` becau - Reason: the queue writes refs and edits rulesets, which are permissions the build service must never hold; adding them to nixbot's App would edit a registration a running service depends on. - Impact: one more registration to hold and one more credential set to rotate; registration is a human gate. -**One check context added to our ruleset, and a second ruleset created by the queue** +**Three check contexts required by our ruleset, and a second ruleset created by the queue** - From: ruleset `16212553` on `cameronraysmith/vanixiets` carries `deletion`, `non_fast_forward`, and required check `nixbot/nix-build` pinned to App `4743700`, with repository admins as its only bypass actor. -- To: the same ruleset, with `nixbot/nix-eval` added beside `nixbot/nix-build`; nothing renamed, nothing removed, no linear-history rule, and no classic branch protection. The queue's own startup setup creates a second ruleset named `gitea-mq` carrying only its own status, adds its App as a bypass actor on ours, and leaves `allow_auto_merge` on. -- Reason: `internal/github/forge.go::GetRequiredChecks` unions ruleset and classic contexts minus queue-owned contexts, and `internal/monitor/monitor.go::ResolveRequiredChecks` prefers a non-empty forge list. Both nixbot contexts must stay in ours, making the configured fallback inactive. This matches the live SBEE-Lab/infra and mulatta/dots pattern. `internal/github/setup.go::EnsureRepoSetup` adds App bypass to ours and creates its own second ruleset if absent; an existing `gitea-mq` name makes it return without repairing or activating that ruleset. -- Impact: the ruleset diff is presented for approval before it is applied, and it adds one required context and removes nothing. A pull request that once merged on `nixbot/nix-build` alone now needs both contexts, and the queue's own context once its ruleset exists. +- To: the same ruleset, with `nixbot/nix-eval` and `nixbot/effects` beside `nixbot/nix-build`; nothing renamed, nothing removed, no linear-history rule, and no classic branch protection. + The queue's own startup setup creates a second ruleset named `gitea-mq` carrying only its own status, adds its App as a bypass actor on ours, and leaves `allow_auto_merge` on. +- Reason: `internal/github/forge.go::GetRequiredChecks` unions ruleset and classic contexts minus queue-owned contexts, and `internal/monitor/monitor.go::ResolveRequiredChecks` prefers a non-empty forge list. + All three nixbot contexts must stay in ours, making the matching configured fallback inactive without permitting a weaker fallback gate. + The two-ruleset arrangement matches the live SBEE-Lab/infra and mulatta/dots pattern, not necessarily their required-check names or count. + `internal/github/setup.go::EnsureRepoSetup` adds App bypass to ours and creates its own second ruleset if absent; an existing `gitea-mq` name makes it return without repairing or activating that ruleset. +- Impact: the original G2 diff added one required context, eval, after approval; the operator subsequently added effects so landing waits for effects to conclude (`design.md::D2`). + A pull request that once merged on `nixbot/nix-build` alone now needs all three contexts, and the queue's own context once its ruleset exists. **Credentials through the fleet's primary secrets system** - From: nothing. @@ -83,7 +90,7 @@ The 2026-09-09 ADR revision retires the orchestrator rollup onto `staging` becau ## Capabilities ### New Capabilities -- `merge-queue-service` (stratum: `behavioral`): queue reachability, tested-SHA batch landing, both nixbot verdicts, a separate App identity, documented merge-authorization signals and review-blind boundary, secret handling, declarative activation, and pinned settings. The earlier orchestrator-only authorization requirement is retired; E1 review-before-signal and ordinary-PR auto-merge versus registered-stack top label are sibling procedure, with auto-merge prohibited on every stack member. +- `merge-queue-service` (stratum: `behavioral`): queue reachability, tested-SHA batch landing, all three nixbot verdicts, a separate App identity, documented merge-authorization signals and review-blind boundary, secret handling, declarative activation, and pinned settings. The earlier orchestrator-only authorization requirement is retired; E1 review-before-signal and ordinary-PR auto-merge versus registered-stack top label are sibling procedure, with auto-merge prohibited on every stack member. - `merge-queue-interface` (stratum: `interface`): the properties at the machine's interface that discharge those requirements — a distinct hostname served over TLS proxied to a loopback port, the four settings as evaluated option values guarded by an assertion, a database and role provisioned for the unit's dynamic user, credentials present only as activation-resolved systemd credentials, the forge application's permission and event set, the two rulesets governing the default branch, and the webhook endpoint the service registers for itself. Its trust boundary is stated in the capability: the installation selection, the collaborator set that bounds who can enable auto-merge, and GitHub's marking of fast-forwarded stack members as merged are all outside what this machine can assert; the shared nginx, PostgreSQL, and ACME account remain common-mode surfaces; and the assertion guards the evaluated configuration, not the running process. ### Modified Capabilities diff --git a/openspec/changes/stand-up-gitea-mq-on-magnetite/specs/merge-queue-interface/spec.md b/openspec/changes/stand-up-gitea-mq-on-magnetite/specs/merge-queue-interface/spec.md index 9d6a12442..530f35a5f 100644 --- a/openspec/changes/stand-up-gitea-mq-on-magnetite/specs/merge-queue-interface/spec.md +++ b/openspec/changes/stand-up-gitea-mq-on-magnetite/specs/merge-queue-interface/spec.md @@ -16,13 +16,15 @@ The machine SHALL serve the merge queue at one hostname of its own, distinct fro ### Requirement: The four landing settings are evaluated values guarded by an assertion -The machine's evaluated configuration SHALL carry the queue's batch maximum as `20`, its up-to-date shortcut as enabled, its configured fallback required checks as exactly `nixbot/nix-eval` and `nixbot/nix-build`, and no override of the merge label on the queue's unit, and SHALL refuse to evaluate when the merged configuration differs from those values. -The configured fallback is not the operative required set: the queue consults it only when the forge names no required check, and the ruleset requirement below keeps the forge's set non-empty, so the two settings are one invariant rather than two independent ones. +The machine's evaluated configuration SHALL carry the queue's batch maximum as `20`, its up-to-date shortcut as enabled, its configured fallback required checks as exactly `nixbot/nix-eval`, `nixbot/nix-build`, and `nixbot/effects`, and no override of the merge label on the queue's unit, and SHALL refuse to evaluate when the merged configuration differs from those values. +The configured fallback is not the operative required set: the queue consults it only when the forge names no required check, and the ruleset requirement below keeps the forge's set non-empty. +The ruleset is authoritative, and the fallback SHALL match its three required external contexts so an empty forge list cannot silently weaken the gate. +Requiring `nixbot/effects` SHALL remain coupled to default-branch `nixbot.toml::effects_on_pull_requests = true` and a non-empty effect set; an empty set returns before `effects_started` in nixbot's `nixbot/nixbot/effects_run.py::enqueue_effects`, leaving the required context unposted and a PR blocked indefinitely. #### Scenario: The evaluated options are read - **WHEN** the queue's options are read from the host's evaluated configuration -- **THEN** the batch maximum is `20`, the up-to-date shortcut is enabled, the configured fallback required checks are exactly the two build-service contexts, and the unit's environment carries no merge-label attribute +- **THEN** the batch maximum is `20`, the up-to-date shortcut is enabled, the configured fallback required checks are exactly the three build-service contexts, and the unit's environment carries no merge-label attribute #### Scenario: Another module forces a different value @@ -32,7 +34,7 @@ The configured fallback is not the operative required set: the queue consults it #### Scenario: The unit's environment is read on the host - **WHEN** the queue's unit environment is read on the host after activation -- **THEN** it carries the batch maximum `20`, the up-to-date shortcut `true`, the two contexts as the configured required-checks list, and no merge-label variable, so the queue's own default of `merge-queue` is in force +- **THEN** it carries the batch maximum `20`, the up-to-date shortcut `true`, the three contexts as the configured required-checks list, and no merge-label variable, so the queue's own default of `merge-queue` is in force ### Requirement: A database and role exist for the unit's dynamic user @@ -83,13 +85,13 @@ The target-repository installation endpoint alone cannot establish exclusivity; ### Requirement: The default branch is governed by two rulesets -The managed repository's default branch SHALL be governed by two rulesets: one maintained by the operator, requiring deletion protection, non-fast-forward protection, and both of the build service's check contexts; and one created and owned by the queue's own startup setup, requiring only the queue's own status. +The managed repository's default branch SHALL be governed by two rulesets: one maintained by the operator, requiring deletion protection, non-fast-forward protection, and all three of the build service's check contexts; and one created and owned by the queue's own startup setup, requiring only the queue's own status. Neither SHALL require linear history, no classic branch protection SHALL govern the branch, the queue's forge application SHALL be a bypass actor on the operator's ruleset, and the repository's `allow_auto_merge` setting SHALL be left as the queue sets it. #### Scenario: The rulesets are read on the forge - **WHEN** the default branch's rulesets are read through the forge's API after the queue has started -- **THEN** two rulesets govern it, the operator's naming both build-service contexts and the queue's naming only its own status pinned to the queue's application, the queue's application is a bypass actor on the operator's ruleset, no rule requires linear history, and the branch's classic protection is absent +- **THEN** two rulesets govern it, the operator's naming all three build-service contexts and the queue's naming only its own status pinned to the queue's application, the queue's application is a bypass actor on the operator's ruleset, no rule requires linear history, and the branch's classic protection is absent #### Scenario: The queue's startup setup runs @@ -103,9 +105,19 @@ Neither SHALL require linear history, no classic branch protection SHALL govern #### Scenario: A build-service context is dropped from the operator's ruleset -- **WHEN** either build-service context is removed from the operator's ruleset on the default branch +- **WHEN** any build-service context is removed from the operator's ruleset on the default branch while the forge's required list remains non-empty - **THEN** the queue's required set shrinks to whatever the forge still names, because it prefers a non-empty forge list over its configured fallback, which is the drift the behavioral requirement `The queue gates on the build service's verdicts and nothing else` names +#### Scenario: The forge names no required external context + +- **WHEN** the forge returns an empty required-check list after excluding queue-owned contexts +- **THEN** the queue uses the configured fallback containing evaluation, build, and effects contexts without weakening the landing gate + +#### Scenario: No effects context is posted + +- **WHEN** PR effects are disabled or the effect set is empty while `nixbot/effects` remains required +- **THEN** the missing context blocks the PR indefinitely rather than failing it, so the required-check list, ruleset, and effect-production configuration must change together + ### Requirement: The service registers its own webhook endpoint The machine SHALL present the queue's delivery endpoint at `/webhook/github` and the queue SHALL, at each start, patch the forge application's webhook URL and delivery secret to that endpoint and the host's current secret, so that no delivery setting is maintained by hand on the forge. @@ -141,7 +153,7 @@ This capability SHALL state what its properties guarantee and what they do not, #### Scenario: The properties above are read as a set - **WHEN** the properties of this capability are read as a set -- **THEN** what they establish is that the queue is reached at its own hostname over its own certificate through a loopback listener, that its four landing settings are fixed at evaluation and visible at runtime, that its database and role are its own, that its credentials exist only as activation-resolved systemd credentials, that its forge application holds exactly the stated set, that the default branch is governed by the operator's ruleset naming both build-service contexts and the queue's own ruleset naming only its status, that the queue maintains its own delivery endpoint, and that what runs is a consequence of the host's declared configuration +- **THEN** what they establish is that the queue is reached at its own hostname over its own certificate through a loopback listener, that its four landing settings are fixed at evaluation and visible at runtime, that its database and role are its own, that its credentials exist only as activation-resolved systemd credentials, that its forge application holds exactly the stated set, that the default branch is governed by the operator's ruleset naming all three build-service contexts and the queue's own ruleset naming only its status, that the queue maintains its own delivery endpoint, and that what runs is a consequence of the host's declared configuration #### Scenario: A guarantee about who can enqueue is sought from this capability diff --git a/openspec/changes/stand-up-gitea-mq-on-magnetite/specs/merge-queue-service/spec.md b/openspec/changes/stand-up-gitea-mq-on-magnetite/specs/merge-queue-service/spec.md index a02f3651c..0b3822cfc 100644 --- a/openspec/changes/stand-up-gitea-mq-on-magnetite/specs/merge-queue-service/spec.md +++ b/openspec/changes/stand-up-gitea-mq-on-magnetite/specs/merge-queue-service/spec.md @@ -40,24 +40,24 @@ A single queued change whose head already contains the default branch's tip MAY ### Requirement: The queue gates on the build service's verdicts and nothing else -The merge queue SHALL consider a change ready to land only when the build service's two verdicts, its evaluation verdict and its build verdict, are both passed on the commit it tests, and SHALL NOT require any other check run beyond its own, whether or not those verdicts were published before the change was queued. +The merge queue SHALL consider a change ready to land only when the build service's three verdicts, its evaluation verdict, build verdict, and effects verdict, are all passed on the commit it tests, and SHALL NOT require any other check run beyond its own, whether or not those verdicts were published before the change was queued. **Discharged by**: `merge-queue-interface` requirements `The four landing settings are evaluated values guarded by an assertion` and `The default branch is governed by two rulesets`, resting on world assumption `A25 — gitea-mq takes required checks from the forge's protection before its own list`. #### Scenario: Verdicts were published before the change was queued -- **WHEN** a single up-to-date entry's head already carries both successful build-service verdicts before authorization and the queue takes the head shortcut without a later rebuild +- **WHEN** a single up-to-date entry's head already carries all three successful build-service verdicts before authorization and the queue takes the head shortcut without a later rebuild - **THEN** the queue accepts the existing head verdicts for enqueue and landing without requiring a new check run or an arrive-after-enqueue timestamp #### Scenario: One verdict is failed -- **WHEN** a tested batch fails either required build-service verdict +- **WHEN** a tested batch fails any required build-service verdict - **THEN** the queue withholds that batch's landing and bisects a multi-entry batch, or ejects a failing singleton with a comment naming the failed verdict; any surviving subset must pass on its tested SHA before landing #### Scenario: The forge's own protection names one verdict only -- **WHEN** the repository's forge-side protection on the default branch comes to name one of the build service's two verdicts and not the other -- **THEN** the queue gates on that single verdict, because it prefers the forge's list whenever that list is non-empty, which is why both verdicts are required in our own ruleset rather than left to the queue's configured fallback +- **WHEN** the repository's forge-side protection on the default branch comes to name one of the build service's three verdicts and neither of the others +- **THEN** the queue gates on that single verdict, because it prefers the forge's list whenever that list is non-empty, which is why all three verdicts are required in our own ruleset rather than left to the queue's configured fallback ### Requirement: The queue acts under its own identity diff --git a/openspec/changes/stand-up-gitea-mq-on-magnetite/specs/world-assumptions/spec.md b/openspec/changes/stand-up-gitea-mq-on-magnetite/specs/world-assumptions/spec.md index 050890f6c..c978943c0 100644 --- a/openspec/changes/stand-up-gitea-mq-on-magnetite/specs/world-assumptions/spec.md +++ b/openspec/changes/stand-up-gitea-mq-on-magnetite/specs/world-assumptions/spec.md @@ -42,13 +42,13 @@ Any requirement whose discharge depends on this fact SHALL name it explicitly, a ### Requirement: A25 — gitea-mq takes required checks from the forge's protection before its own list It is true of gitea-mq at the revision this fleet pins, independent of what this fleet builds, that the set of checks it requires on a target branch is the forge's own required-status-check list from rulesets and classic protection with the queue's own contexts removed, and that its configured list is consulted only when that forge list is empty (`monitor.go::ResolveRequiredChecks`, `forge.go::GetRequiredChecks`). -A forge-side requirement for any single build-service context therefore replaces the configured pair rather than adding to it, and a forge-side requirement for both makes the forge the operative source with the configured pair never consulted. +A forge-side requirement for any single build-service context therefore replaces the configured three-context set rather than adding to it, and a forge-side requirement for all three makes the forge the operative source with the matching configured fallback never consulted. Any requirement whose discharge depends on this fact SHALL name it explicitly, and SHALL be treated as losing its discharge once this assumption's violation condition below is observed. #### Scenario: The resolution order changes - **WHEN** a pinned revision of gitea-mq merges the forge's list with its configured list, prefers its configured list, or drops the forge list entirely -- **THEN** this assumption is void, and the `merge-queue-service` requirement `The queue gates on the build service's verdicts and nothing else` and the `merge-queue-interface` requirement `The default branch is governed by two rulesets` lose the argument that keeping both build-service contexts in the operator's ruleset makes exactly that pair the queue's required set +- **THEN** this assumption is void, and the `merge-queue-service` requirement `The queue gates on the build service's verdicts and nothing else` and the `merge-queue-interface` requirement `The default branch is governed by two rulesets` lose the argument that keeping all three build-service contexts in the operator's ruleset makes exactly that set the queue's required set ### Requirement: A26 — the batch engine advances the target to the exact tested commit diff --git a/openspec/changes/stand-up-gitea-mq-on-magnetite/tasks.md b/openspec/changes/stand-up-gitea-mq-on-magnetite/tasks.md index b1873c883..b0bd436cb 100644 --- a/openspec/changes/stand-up-gitea-mq-on-magnetite/tasks.md +++ b/openspec/changes/stand-up-gitea-mq-on-magnetite/tasks.md @@ -23,9 +23,9 @@ R1/R2 source filtering and cache warming and R15/R16 authorization procedure rem ## 4. First-party aspect - [x] 4.1 Write `modules/nixos/gitea-mq.nix` defining `flake.modules.nixos.gitea-mq` with `services.gitea-mq` enabled, `github.appId` from task 1.2, `github.privateKeyFile` and `github.webhookSecretFile` from the two generators, `github.repos = [ "cameronraysmith/vanixiets" ]`, `externalUrl = "https://mq.scientistexperience.net"`, `listenAddr = "127.0.0.1:8092"`, `hideRefFromClients = false`, `databaseUrl` at its default, `services.postgresql.ensureDatabases = [ "gitea-mq" ]` and `ensureUsers = [ { name = "gitea-mq"; ensureDBOwnership = true; } ]`, and `services.nginx.virtualHosts."mq.scientistexperience.net"` with `forceSSL`, `enableACME`, and `locations."/".proxyPass = "http://127.0.0.1:8092"` — verify: `nix eval .#nixosConfigurations.magnetite.config.services.gitea-mq --apply 's: { inherit (s) externalUrl listenAddr hideRefFromClients databaseUrl; repos = s.github.repos; appId = s.github.appId; }' --json` returns those values, `nix eval .#nixosConfigurations.magnetite.config.services.postgresql.ensureDatabases --json` contains `gitea-mq`, and `nix eval '.#nixosConfigurations.magnetite.config.services.nginx.virtualHosts."mq.scientistexperience.net".locations."/".proxyPass'` returns the loopback address -- [ ] 4.2 Set the four landing settings — `batchMax = 20`, `skipQueueIfUpToDate = true`, `requiredChecks = [ "nixbot/nix-eval" "nixbot/nix-build" ]`, and no assignment to the merge label anywhere — verify: `nix eval .#nixosConfigurations.magnetite.config.systemd.services.gitea-mq.environment --apply 'e: { inherit (e) GITEA_MQ_BATCH_MAX GITEA_MQ_SKIP_QUEUE_IF_UP_TO_DATE GITEA_MQ_REQUIRED_CHECKS; label = e ? GITEA_MQ_MERGE_LABEL; }' --json` returns `{"GITEA_MQ_BATCH_MAX":"20","GITEA_MQ_SKIP_QUEUE_IF_UP_TO_DATE":"true","GITEA_MQ_REQUIRED_CHECKS":"nixbot/nix-eval,nixbot/nix-build","label":false}`, and `rg GITEA_MQ_MERGE_LABEL modules/` finds only the assertion -- [ ] 4.3 Add `assertions` to the aspect that read `config.services.gitea-mq.batchMax == 20`, `.skipQueueIfUpToDate == true`, `.requiredChecks == [ "nixbot/nix-eval" "nixbot/nix-build" ]`, and `!(config.systemd.services.gitea-mq.environment ? GITEA_MQ_MERGE_LABEL)`, each with a message naming the setting and ADR R11 — verify: `nix eval .#checks.x86_64-linux.nixos-magnetite.drvPath` succeeds as-is; then, in a separate authorized scratch workspace, adding `services.gitea-mq.batchMax = lib.mkForce 1;` to the host makes the same command fail with the assertion's message requiring 20, and adding `systemd.services.gitea-mq.environment.GITEA_MQ_MERGE_LABEL = "x";` fails with the label assertion's message; both failures captured and scratch edits reverted. The previous completion predates the twenty-entry assertion, so this negative-control verification remains reopened; do not perform it in the shared correction session. -- [ ] 4.4 Give the aspect file a header documenting its two generators, the peer-authentication coupling between the unit name and the role name, the loopback port and why 8080 is not used, `hideRefFromClients` and why, and the two-ruleset/forge-derived-check invariant — verify: the header names each item, including G2's single added check and setup's creation of a second ruleset, and `just lint` passes inside the dev shell +- [ ] 4.2 Set the four landing settings — `batchMax = 20`, `skipQueueIfUpToDate = true`, `requiredChecks = [ "nixbot/nix-eval" "nixbot/nix-build" "nixbot/effects" ]`, and no assignment to the merge label anywhere — verify: `nix eval .#nixosConfigurations.magnetite.config.systemd.services.gitea-mq.environment --apply 'e: { inherit (e) GITEA_MQ_BATCH_MAX GITEA_MQ_SKIP_QUEUE_IF_UP_TO_DATE GITEA_MQ_REQUIRED_CHECKS; label = e ? GITEA_MQ_MERGE_LABEL; }' --json` returns `{"GITEA_MQ_BATCH_MAX":"20","GITEA_MQ_SKIP_QUEUE_IF_UP_TO_DATE":"true","GITEA_MQ_REQUIRED_CHECKS":"nixbot/nix-eval,nixbot/nix-build,nixbot/effects","label":false}`, and `rg GITEA_MQ_MERGE_LABEL modules/` finds only the assertion +- [ ] 4.3 Add `assertions` to the aspect that read `config.services.gitea-mq.batchMax == 20`, `.skipQueueIfUpToDate == true`, `.requiredChecks == [ "nixbot/nix-eval" "nixbot/nix-build" "nixbot/effects" ]`, and `!(config.systemd.services.gitea-mq.environment ? GITEA_MQ_MERGE_LABEL)`, each with a message naming the setting and ADR R11 — verify: `nix eval .#checks.x86_64-linux.nixos-magnetite.drvPath` succeeds as-is; then, in a separate authorized scratch workspace, adding `services.gitea-mq.batchMax = lib.mkForce 1;` to the host makes the same command fail with the assertion's message requiring 20, and adding `systemd.services.gitea-mq.environment.GITEA_MQ_MERGE_LABEL = "x";` fails with the label assertion's message; both failures captured and scratch edits reverted. The previous completion predates the twenty-entry assertion, so this negative-control verification remains reopened; do not perform it in the shared correction session. +- [ ] 4.4 Give the aspect file a header documenting its two generators, the peer-authentication coupling between the unit name and the role name, the loopback port and why 8080 is not used, `hideRefFromClients` and why, and the two-ruleset/forge-derived-check invariant — verify: the header names each item, including G2's original eval addition, the operator's later effects addition, why the inactive fallback must match all three contexts, and setup's creation of a second ruleset, and `just lint` passes inside the dev shell ## 5. Host composition @@ -44,6 +44,9 @@ R1/R2 source filtering and cache warming and R15/R16 authorization procedure rem ## 8. Rulesets +The G2 diff in tasks 8.2/8.3 records the original eval addition, not today's complete required set. +The operator subsequently added `nixbot/effects`; current read-only verification must match all three contexts under `design.md::D2` without replaying that historical mutation or removing effects. + - [ ] 8.1 Read the current state of `cameronraysmith/vanixiets` and record it in verify.md — verify: `gh api /repos/cameronraysmith/vanixiets/rulesets`, `gh api /repos/cameronraysmith/vanixiets/rulesets/16212553`, `gh api /repos/cameronraysmith/vanixiets/branches/main/protection`, `gh api /repos/cameronraysmith/vanixiets --jq '{allow_auto_merge,allow_merge_commit,allow_squash_merge,allow_rebase_merge}'`, and `gh api /repos/cameronraysmith/vanixiets/collaborators --jq '.[].login'` are captured verbatim - [ ] 8.2 [G2 — operator gate; an agent MUST NOT tick this box] Present the per-repository ruleset diff for approval and stop: before, ruleset `16212553` `nixbot` on `~DEFAULT_BRANCH` with `deletion`, `non_fast_forward`, and `required_status_checks [nixbot/nix-build @ 4743700]`; after, the same ruleset with only `nixbot/nix-eval @ 4743700` added to that list. Preserve every other field, rule, name, and bypass actor; remove nothing, add no linear-history rule, and do not pre-create classic protection. Show the full captured before and proposed after bodies and the reverse diff removing only the added context. Installation setup creates the separate App-owned `gitea-mq` ruleset and adds App bypass to ours; this is distinct from the operator diff. Leave `allow_auto_merge` true. Wait for operator approval or amendment and record the stop; if current state differs from task 8.1's expected baseline, ask rather than broaden the mutation. - [ ] 8.3 After approval, apply only the added `nixbot/nix-eval @ 4743700` context to ruleset `16212553` and record the result — verify: a read-back of `gh api /repos/cameronraysmith/vanixiets/rulesets/16212553` differs from the before body only by that context, retaining `nixbot/nix-build`, `deletion`, `non_fast_forward`, the `nixbot` name, parameters, and bypass actors. Do not rename it or swap checks. A disabled pre-created `gitea-mq` ruleset is optional, solely to choose when the queue context starts blocking, and is not part of G2's required edit; if chosen, record operator-approved activation timing because `internal/github/setup.go::EnsureRepoSetup` returns early for that name. @@ -62,9 +65,12 @@ R1/R2 source filtering and cache warming and R15/R16 authorization procedure rem - [ ] 11.1 Verify the queue answers at its own hostname over an accepted connection and the build service still does — verify: `curl -o /dev/null -w '%{http_code} ssl_verify=%{ssl_verify_result}\n' https://mq.scientistexperience.net/` returns `200 ssl_verify=0` with a certificate issued for that hostname, and the same request to `https://nixbot.scientistexperience.net/` returns `200 ssl_verify=0` - [ ] 11.2 Verify the service registered its own webhook endpoint and that deliveries are authenticated both ways — verify: `gh api /app/hook/config` under the App's JWT, or the App's settings page, shows URL `https://mq.scientistexperience.net/webhook/github`; a redelivery from the App's advanced settings returns `2xx` and appears in `journalctl -u gitea-mq.service`; and an unsigned `curl -X POST https://mq.scientistexperience.net/webhook/github -d '{}'` returns `401` or `403`, with both status codes recorded in verify.md - [ ] 11.3 Verify the database and role exist for the dynamic user — verify: on the host `sudo -u postgres psql -c '\l'` lists `gitea-mq` owned by `gitea-mq`, `psql -c '\du'` shows the role without superuser or createdb, and `systemctl show gitea-mq.service -p User -p DynamicUser` reports `User=gitea-mq DynamicUser=yes` -- [ ] 11.4 Verify setup's two-ruleset result — verify: read `gh api /repos/cameronraysmith/vanixiets/rulesets` and each ruleset body; ours retains `deletion`, `non_fast_forward`, both nixbot contexts pinned to `4743700`, and App integration bypass; the separate active `gitea-mq` ruleset requires only `gitea-mq` pinned to the queue App, with App/admin bypass. No `required_linear_history` exists, `gh api /repos/cameronraysmith/vanixiets/branches/main/protection` returns 404, and `gh api /repos/cameronraysmith/vanixiets --jq .allow_auto_merge` is `true`. Setup may create its own ruleset and add bypass to ours; require no setup permission warnings. If a disabled queue ruleset was pre-created, verify its approved activation explicitly. -- [ ] 11.5 Verify V3 and the forge-derived required-check invariant — verify: on a pull request whose tip nixbot has built, `gh api /repos/cameronraysmith/vanixiets/commits//check-runs --jq '.check_runs[].name'` includes the exact names `nixbot/nix-eval` and `nixbot/nix-build`; read the ruleset bodies and classic-protection response to establish that `internal/github/forge.go::GetRequiredChecks` returns exactly that pair after excluding queue-owned contexts. `internal/monitor/monitor.go::ResolveRequiredChecks` prefers this non-empty forge list, so `GITEA_MQ_REQUIRED_CHECKS` does not fire. Corroborate through runtime logs if available; log/dashboard visibility of the resolved list is unverified and must not be assumed. -- [ ] 11.6 Verify the four landing settings in the running unit's environment — verify: `systemctl show gitea-mq.service -p Environment` on the host carries `GITEA_MQ_BATCH_MAX=20`, `GITEA_MQ_SKIP_QUEUE_IF_UP_TO_DATE=true`, `GITEA_MQ_REQUIRED_CHECKS=nixbot/nix-eval,nixbot/nix-build`, and no `GITEA_MQ_MERGE_LABEL` -- [ ] 11.7 Verify V2 and the singleton shortcut end to end under `batchMax = 20` — verify: choose one ordinary trunk PR with an up-to-date head and both nixbot checks green before authorization, apply the sibling E1 policy, then enable native auto-merge with no other queued entry. Record head SHA, tested SHA, and landing SHA equality, GitHub `merged_at`, queue success status, no batch ref created, and elapsed signal-to-landing time. Do not infer no merge commits from parent count: the original head may already contain them. If target movement or comparison failure causes a batch-branch rebuild, record that path rather than claiming the shortcut (`internal/batch/batch.go::{Engine.headIfUpToDate,Engine.HandlePass}`). +- [ ] 11.4 Verify setup's two-ruleset result — verify: read `gh api /repos/cameronraysmith/vanixiets/rulesets` and each ruleset body; ours retains `deletion`, `non_fast_forward`, all three nixbot contexts pinned to `4743700`, and App integration bypass; the separate active `gitea-mq` ruleset requires only `gitea-mq` pinned to the queue App, with App/admin bypass. No `required_linear_history` exists, `gh api /repos/cameronraysmith/vanixiets/branches/main/protection` returns 404, and `gh api /repos/cameronraysmith/vanixiets --jq .allow_auto_merge` is `true`. Setup may create its own ruleset and add bypass to ours; require no setup permission warnings. If a disabled queue ruleset was pre-created, verify its approved activation explicitly. +- [ ] 11.5 Verify V3 and the forge-derived required-check invariant — verify: on a pull request whose tip nixbot has built and run effects for, `gh api /repos/cameronraysmith/vanixiets/commits//check-runs --jq '.check_runs[].name'` includes the exact names `nixbot/nix-eval`, `nixbot/nix-build`, and `nixbot/effects`; read the ruleset bodies and classic-protection response to establish that `internal/github/forge.go::GetRequiredChecks` returns exactly that three-context set after excluding queue-owned contexts. + `internal/monitor/monitor.go::ResolveRequiredChecks` prefers this non-empty forge list, so `GITEA_MQ_REQUIRED_CHECKS` does not fire but must match it to avoid a weaker gate when the forge list is empty. + Confirm default-branch `nixbot.toml::effects_on_pull_requests = true` and a non-empty effect set, because an unposted required effects context blocks indefinitely (`design.md::D2`). + Corroborate through runtime logs if available; log/dashboard visibility of the resolved list is unverified and must not be assumed. +- [ ] 11.6 Verify the four landing settings in the running unit's environment — verify: `systemctl show gitea-mq.service -p Environment` on the host carries `GITEA_MQ_BATCH_MAX=20`, `GITEA_MQ_SKIP_QUEUE_IF_UP_TO_DATE=true`, `GITEA_MQ_REQUIRED_CHECKS=nixbot/nix-eval,nixbot/nix-build,nixbot/effects`, and no `GITEA_MQ_MERGE_LABEL` +- [ ] 11.7 Verify V2 and the singleton shortcut end to end under `batchMax = 20` — verify: choose one ordinary trunk PR with an up-to-date head and all three nixbot checks green before authorization, apply the sibling E1 policy, then enable native auto-merge with no other queued entry. Record head SHA, tested SHA, and landing SHA equality, GitHub `merged_at`, queue success status, no batch ref created, and elapsed signal-to-landing time. Do not infer no merge commits from parent count: the original head may already contain them. If target movement or comparison failure causes a batch-branch rebuild, record that path rather than claiming the shortcut (`internal/batch/batch.go::{Engine.headIfUpToDate,Engine.HandlePass}`). - [ ] 11.8 Re-confirm discharged V1 at the first live stacked landing, replacing the retired V6 landing-ref probe — verify: after native registration and head-ancestry checks and authorization under the sibling R15/R16 procedure, label only the topmost intended PR `merge-queue` and ensure every member has auto-merge disabled. Retain each member's head SHA and `merged_at`, observe GitHub retargeting/marking, and check subsequent mergify-cli sync recognizes every landed member as merged. V1 is discharged by operator confirmation of this mechanism and empirical non-stack Mic92/dotfiles #5887–#5890; this is re-confirmation, not a promotion blocker. The queue posts completion and runs `Engine.ensureMergedOrClose` only for actual entries, not lower members (`internal/batch/batch.go::Engine.HandlePass`). - [ ] 11.9 Verify the rollback path is available — verify: in a separate authorized scratch workspace, removing `gitea-mq` from magnetite's aspect list makes `nix eval .#checks.x86_64-linux.nixos-magnetite.drvPath` succeed with a different derivation path, then revert the scratch edit. Record the G2 reverse diff separately from the operator-approved disabling of the App-owned queue gate when stopping the service; preserve our nixbot build check and protection rules. No rollback mutation is performed by this correction session.