Skip to content

feat(pr-pipeline): add a per-repo red-main remedy key - #8948

Merged
kyle-sexton merged 1 commit into
mainfrom
feat/red-main-remedy
Oct 22, 2026
Merged

kyle-sexton merged 1 commit into
mainfrom
feat/red-main-remedy

Conversation

@kyle-sexton

Copy link
Copy Markdown
Member

No related issue: owner decision W-51 (2026-10-21)

Summary

Adds red-main.remedy, a per-repository setting for what happens after a post-merge failure: rollback (the default, which keeps the current revert rule) or fail-forward. The report is still report-only.

Fix

  • red-main.mjs: remedy joins RED_MAIN_DEFAULTS (rollback) and VALID (rollback or fail-forward; any other value falls back to the default). Under fail-forward, decide() returns human with one reason, "the remedy is fail-forward: a person or a fix pull request repairs main".
  • report.mjs: plannedRevert runs only under rollback, so a fail-forward report has no planned revert.
  • renderReport: the culprit line and the "A person decides because:" reasons used to render only when there was a revert, which would have dropped the remedy reason. They now render whenever a culprit was decided, and only the revert block requires a revert.
  • Marker grammar is unchanged. decision=human already matches MARKER_PATTERN, and since fail-forward never produces decision=auto-revert, those decisions count toward neither the daily cap nor "already decided".
  • Schema (pr-pipeline.schema.json): adds remedy with an enum and a default. pr-pipeline.yaml sets remedy: rollback with a comment.
  • README: adds a remedy row to the Red main key table. The deferred "post-merge failure remedy" bullet now says it is the per-repository red-main.remedy key. No ADRs were edited.

Note: docs/conventions/pr-pipeline.yaml is a lane-power file, so a human merges this PR.

Verification

  • New tests failed before the implementation (4 of 39 failed: the two-sided fallback, the default/valid values, fail-forward decide, and the fail-forward report). After the implementation: node --test red-main.test.mjs report.test.mjs workflow.test.mjs passes 49/49.
  • node --test .github/actions/resolve-config/live-config.test.mjs resolve-config.test.mjs: 102/102.
  • scripts/check-convention-yaml.sh --check: 5 files validate against their schemas.
  • scripts/check-affected.sh: 9 passed, 4 skipped; the lefthook pre-push check-affected passed.
  • scripts/check-line-citations.py: clean.

Related

  • Red main rule: docs/conventions/pr-pipeline/README.md#red-main.
  • #8699: the open items to fix before live reverts. Not changed here.

🤖 Generated with Claude Code

red-main.remedy is rollback (default, the current revert rule) or
fail-forward, under which the report plans no revert: the decision is
human, the reason names the remedy, and a person or a fix pull request
repairs main. Owner decision W-51 (2026-10-21).

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

chatgpt-codex-connector Bot commented Oct 22, 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-22T01:23:08.898213Z c540480 Draft marked ready
ℹ️ 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.

@github-actions

Copy link
Copy Markdown
Contributor

claude-security-review reviewed c540480f45e34664092f6bdc2cd7e17b719edc51: 0 finding(s), 0 as inline comments.

Security review of PR #8948 at c540480

No security findings.

The PR adds an optional red-main.remedy key, rollback (default) or fail-forward, to the red-main report: the YAML, the JSON schema, the validator and default in red-main.mjs, the README, and report.mjs.

  • Direction of the change: the new key can only narrow what the bot does. Under fail-forward, decide() returns human before any guard is evaluated, and report.mjs sets revert = null. The default stays rollback, so repositories that never set the key keep today's behavior. Nothing is widened, and no gate, cap or budget changes.
  • Input validation: the validator accepts only the exact strings rollback and fail-forward, and anything else falls back to rollback. The value is only compared with ===. It never reaches a shell, a path, a template or an API call. The report prints a fixed reason string, not the configured value.
  • Config source: the config is read from the base checkout through redMainFromBase, not from the PR head, so a PR cannot change its own remedy.
  • Report change: renderReport now prints the culprit and the "a person decides because" reasons whenever a decision exists, and prints the revert block only when a revert plan exists. The content is the same culprit SHA, number and merge time the report already printed.
  • Revert execution: report.mjs only posts a comment, and the diff touches no revert-execution code. If an old resolver returned no remedy, the report would show an auto-revert decision with no plan, but that is a correctness edge, not a security one.
  • Instruction-surface deletions: none. The README edit replaces a "deferred, undecided" bullet with a pointer to the new key. It removes no guardrail.
  • Plugin manifests: no plugin.json or marketplace.json is touched, so there are no manifest <field> lines.
  • Actions hardening: no workflow files changed.
  • Instruction exception register: I did not consult it, since no standing instruction was removed.

@github-actions

Copy link
Copy Markdown
Contributor

claude-review reviewed c540480f45e34664092f6bdc2cd7e17b719edc51: 0 finding(s), 0 as inline comments.

No findings. I read the full diff, decide, renderReport and report() in full, and the resolve-config path that supplies config. I did not run the tests; the PR body's counts (49/49, 102/102) are author-claimed and unverified by me.

  • Config path: report() always builds config with resolveRedMain, so config.remedy is always rollback or fail-forward. The config.remedy === "rollback" check in report.mjs therefore cannot null the revert by accident, even when loadConfig returns a partial leaf.
  • remedy placement: redMainFromBase passes the red-main leaf through unvalidated, and the schema, the default YAML and the README row all agree with RED_MAIN_DEFAULTS and VALID.
  • Counting and dedup: the fail-forward branch in decide returns before any guard, so a fail-forward run never writes decision=auto-revert. Its report therefore adds nothing to the daily-cap and "already decided" counts, which read only that marker value.
  • renderReport restructure: the culprit line and the reasons now render whenever a culprit was decided, and the revert block renders only when revert is set. Under rollback a culprit always comes with a revert, so that output is unchanged.
  • Tests: the new tests cover the fallback for an invalid value, the default, decide under fail-forward, and the end-to-end report with no revert block. They exercise the changed branches.
  • Changelog fragment: bump: minor fits an added optional key.
  • Docs: the README row and the "Not settled yet" edit match the code.

Manifest checks: the diff touches no plugin.json or marketplace.json, so none apply. The PR has no closing issue; the body gives "No related issue: owner decision W-51", so there are no acceptance criteria to check.

Merged via the queue into main with commit a98d212 Oct 22, 2026
60 checks passed
@kyle-sexton
kyle-sexton deleted the feat/red-main-remedy branch October 22, 2026 01:26
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