Skip to content

ci: land main through quest land - #137

Merged
kixelated merged 6 commits into
mainfrom
quest/m1/land
Oct 28, 2026
Merged

kixelated merged 6 commits into
mainfrom
quest/m1/land

Conversation

@kixelated

@kixelated kixelated commented Oct 28, 2026 •

Copy link
Copy Markdown
Collaborator

Problem

Today each PR merges once it is green against the main it last saw, so two PRs that pass alone can break main together. #176 and #177 did: together they left lang's dyns.ts snapshot stale, and main went red at 1af6db6a. Landing should test the combination before main moves.

quest/m1/land.md: main should land PRs only through quest land, as one tested batch, so the merge button cannot land around it. Today main has no ruleset or branch protection, and check.yml still carries a merge_group trigger for a merge queue this personal repository cannot use.

Approach

  • .github/workflows/land.yml: the three jobs from kixelated/quest's README, set up like kixelated/quest#236:
    • Resolve builds quest from main's flake pin, outside the checkout, so a queued PR cannot change it.
    • Check runs Check's steps (just check && just test in the dev shell) on the chain, on the same ubuntu-24.04-arm runner. It holds only a read token and no persisted credentials. Its timeout is 90 minutes, enough for a few bisection runs of the ~12 minute suite.
    • Land pushes with a token from the land App, minted from the land environment (vars.LAND_APP_CLIENT_ID, secrets.LAND_APP_PRIVATE_KEY). Then it re-dispatches itself when the queue changed.
  • .github/land.test.ts, run by just test with Bun: parses every *.yml and *.yaml workflow and fails if any job other than land.yml's land names an environment (any environment, since an expression can spell land without the word) or calls another repository's reusable workflow, which could name it.
  • check.yml drops merge_group. It also drops push: main: main only fast-forwards to a tree the gate already checked. It gains workflow_dispatch so an admin can check main after an emergency bypass push.
  • CONTRIBUTING.md: PRs land only through quest land, and the admin bypass is for emergencies. The squash body is the PR's commit messages (the gate builds GitHub's squash format), not the PR description.
  • quest/m1/land.md is rewritten to what remains: the maintainer's setup and a live landing.

Change from the quest's plan

The plan (2026-10-26) named GitHub Actions as the ruleset's bypass actor, guarded by a test refusing contents: write outside the land job. That no longer works. On 2026-10-27 GitHub refused GitHub Actions (Integration 15368) as a bypass actor on a personal repository (kixelated/quest) and on an organization repository, both with "must be part of the ruleset source or owner organization". Even where GitHub allows it, GITHUB_TOKEN cannot get workflows permission, so it could not land a PR that changes .github/workflows. This PR follows kixelated/quest's newer setup instead: an App bypass, with a test that guards the environment holding the App's key.

Impact

  • No public API change.
  • Landing changes for contributors once the ruleset is in place: queue with quest merge <pr> <token> --land <sha> (/quest-merge does this).

Alternatives

  • GitHub Actions bypass: rejected, for the reasons above.
  • Keep Check on push to main: it would re-run ~12 minutes of CPU on every landing, on a tree the gate already tested. Until the gate is live, a broken main still shows on every PR's Check, which runs on the PR merged with main.

Decisions

Settled without the maintainer, since none changes this diff:

  • Land order: this PR lands the way PRs land today, since the gate needs land.yml on main first. Until the setup below exists, land.yml stops at resolve (it refuses a base that does not require quest land), so nothing lands through it early.
  • Setup is the quest's remaining work, not a blocker here: the App, environment, and ruleset are account settings, recorded in the rewritten quest. Which App holds the bypass does not change land.yml: either way it reads LAND_APP_CLIENT_ID and LAND_APP_PRIVATE_KEY.
  • The guard is a Bun script in .github/ run by just test, since rust2 has no TypeScript test harness at the root. Unlike kixelated/quest's line-based test/workflows.test.ts, it parses the YAML, per Codex's security finding.

Maintainer setup after merge

  1. App. Recommended: install kixelated/quest's existing land App on kixelated/rust2, one key to rotate across the maintainer's repositories. The alternative is a separate App with the same permissions, to keep each repository's blast radius separate. Either way it needs Contents (read and write), Workflows (read and write), and Metadata (read), and no webhook, Checks, or Statuses permission.
  2. Environment. Create land on kixelated/rust2 with deployment branches restricted to main, the variable LAND_APP_CLIENT_ID, and the secret LAND_APP_PRIVATE_KEY. Do this before the ruleset: kixelated/quest's first live run failed at the App token step because its land environment had no client ID yet.
  3. Ruleset. A new active ruleset on main that restricts deletions, blocks force pushes, and requires two status checks: Check from GitHub Actions (so the gate ejects a red PR before batching it), and quest land pinned to the land App, which never reports it. Bypass actors: the land App and the Repository admin role. Add no other rule types: quest land resolve fails on any it does not uphold, and these are all the gate needs.
  4. The PR that deletes quest/m1/land.md lands through the gate as the live check.

Checks

  • just check and just test passed locally on the merged head 494f104; after the guard rewrite, CI runs them.
  • quest check --base origin/main --branch quest/m1/land and actionlint are clean.
  • The guard's fixtures cover the scalar, mapping, flow, and expression forms and remote reusable workflows, and it fails on a .yaml workflow naming an expression environment.
  • The live run waits for the setup above.

Follow-ups

  • kixelated/quest's test/workflows.test.ts has the same gaps (expressions, flow mappings, remote reusable workflows); worth the same parse there.
  • The live verification is the rewritten quest.

(Written by Claude Opus 5.5)

🤖 Generated with Claude Code

kixelated and others added 2 commits October 27, 2026 17:13
Quest-Claim: 4ab8057807151749
The Land workflow batches PRs queued with `quest merge --land`, checks the
chain with Check's steps once, and fast-forwards main to the passing prefix.
It pushes with a land App's token from the `land` environment; a test keeps
every other job out of that environment. Check drops its merge_group and
push-to-main triggers.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@kixelated kixelated changed the title Land main through quest land ci: land main through quest land Oct 28, 2026
@kixelated
kixelated marked this pull request as ready for review October 28, 2026 16:40
@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Oct 28, 2026 •

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review ✅ Completed 2026-10-28T17:06:49.438527Z a8c7966 New commits
🔒 Security Review ✅ Completed 2026-10-28T17:12:25.660468Z a8c7966 New commits

Security findings

Advisory findings (1)

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@kixelated kixelated left a comment

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Automated review by review (OpenAI)

Reviewed commit: 494f104485dc32545127ca42d9415f38a531c9c8.

No actionable bugs found in the changed workflow, guard, and contributor instructions. The resolve/check/push separation is a reasonable direction: queued code runs without the landing credential, while the existing pinned quest CLI supplies batching rather than duplicating that logic here.

Verification limits: static GitHub review only; I did not run Nix, lint, tests, or a live landing. The App/environment/ruleset setup and end-to-end enforcement remain unverified, as documented in the remaining quest. The text-based environment guard is a regression check for the repository's current YAML conventions, not a general security boundary.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛡️ Codex Security Review · Automatically triggered

Here are some automated security review suggestions for this pull request.

Reviewed commit: 494f104485

ℹ️ About Codex security reviews in GitHub

This is an experimental Codex feature. Security reviews are triggered when:

  • You comment "@codex security review"
  • A regular code review gets triggered (for example, "@codex review" or when a PR is opened), and you’re opted in so security review runs alongside code review

Once complete, Codex will leave suggestions, or a comment if no findings are found.

Comment thread .github/land.test.sh Resolved
kixelated and others added 2 commits October 28, 2026 09:51
The line scan read only `*.yml` and could miss an environment spelled through
an expression, a flow mapping, or another repository's reusable workflow.
Parse every `*.yml` and `*.yaml` workflow and allow no environment and no
remote reusable workflow outside land.yml's land job.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

@kixelated kixelated left a comment

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Automated review by review (OpenAI)

Reviewed commit: 1349259388552cac8034c3652dd9bdfd759b8a02, compared with my previous review of 494f104485dc32545127ca42d9415f38a531c9c8.

Fixed: the independent security finding is addressed in .github/land.test.ts:11–15,30–34: both workflow extensions are scanned and parsed, any environment reference is rejected outside the designated job, and remote reusable workflows are included in the guard. The new fixtures exercise the scalar, mapping, flow and expression cases. This is a fix to an existing finding, not a new one.

No new actionable bugs found. Replacing line matching with the existing Bun runtime's YAML parser is a simpler, more reliable direction than extending the awk grammar; just test now invokes that guard.

Verification: static review of this update, its fixtures and test wiring only; I did not execute Bun/Nix, the suite or a live landing. External App/environment/ruleset configuration remains unverified. This guard still does not replace review of changes to the guard itself or the privileged landing job.

@kixelated

Copy link
Copy Markdown
Collaborator Author

Landing a8c7966 (squash, pinned to the reviewed head).

  • Settled: setup (App, land environment, ruleset) is the quest's remaining work and doesn't change this diff. The PR body recommends reusing kixelated/quest's land App and creating the environment before the ruleset.
  • Fixed Codex's P1 security finding: the guard now parses every *.yml/*.yaml workflow and allows no environment and no remote reusable workflow outside land.yml's land job.
  • Merged main twice, cleanly. Check is green on the head, and Codex gave it a 👍.

(Written by Claude Opus 5.5)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant